Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-279

Improving the query_executor page of web UI

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: Web UI
    • Labels:
      None

      Description

      The current implementation has the following issues.

      • If there are too many results, the result view occupies too much space in the current implementation. Furthermore, memory overflow might be occurred in the browsers.
      • The maximal number of results is limited as 1000 rows.
      1. TAJO-279_5.patch
        13 kB
        Wan Heo
      2. TAJO-279_4.patch
        13 kB
        Wan Heo
      3. TAJO-279_3.patch
        13 kB
        Wan Heo
      4. screenshot_3.png
        397 kB
        Wan Heo
      5. TAJO-279_3.patch
        1.01 MB
        Wan Heo
      6. TAJO-279_2.patch
        1.01 MB
        Wan Heo
      7. screenshot_2.png
        98 kB
        Wan Heo
      8. TAJO-279.patch
        58 kB
        Wan Heo
      9. screenshot.png
        229 kB
        Wan Heo

        Activity

        Hide
        hyunsik Hyunsik Choi added a comment -

        Wan Heo,

        Nice work. Thanks!

        Show
        hyunsik Hyunsik Choi added a comment - Wan Heo, Nice work. Thanks!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-trunk-postcommit #569 (See https://builds.apache.org/job/Tajo-trunk-postcommit/569/)
        TAJO-279: Improving the query_executor page of web UI. (Wan Heo via jihoon) (jihoonson: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=87437f1718e5e55ca4eb4517fcb3a00707cbc5a9)

        • tajo-core/tajo-core-backend/src/main/resources/webapps/admin/getCSV.jsp
        • tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query_executor.jsp
        • tajo-core/tajo-core-backend/src/main/resources/webapps/admin/WEB-INF/jetty-web.xml
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/webapp/QueryExecutorServlet.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-trunk-postcommit #569 (See https://builds.apache.org/job/Tajo-trunk-postcommit/569/ ) TAJO-279 : Improving the query_executor page of web UI. (Wan Heo via jihoon) (jihoonson: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=87437f1718e5e55ca4eb4517fcb3a00707cbc5a9 ) tajo-core/tajo-core-backend/src/main/resources/webapps/admin/getCSV.jsp tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query_executor.jsp tajo-core/tajo-core-backend/src/main/resources/webapps/admin/WEB-INF/jetty-web.xml tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/webapp/QueryExecutorServlet.java
        Hide
        jihoonson Jihoon Son added a comment -

        Thanks for your contribution.
        I've just committed it.

        Show
        jihoonson Jihoon Son added a comment - Thanks for your contribution. I've just committed it.
        Hide
        WanHeo Wan Heo added a comment -

        I'm sorry to bother you. I fixed the bug that adds comma last of each line.

        Show
        WanHeo Wan Heo added a comment - I'm sorry to bother you. I fixed the bug that adds comma last of each line.
        Hide
        jihoonson Jihoon Son added a comment -

        When I download the result as a csv file, an extra comma is appended to each end of the line.
        Would you fix this bug, please?

        Show
        jihoonson Jihoon Son added a comment - When I download the result as a csv file, an extra comma is appended to each end of the line. Would you fix this bug, please?
        Hide
        jihoonson Jihoon Son added a comment - - edited

        +1 for the latest patch.
        I'll commit this patch.
        Thanks for your contribution.

        Show
        jihoonson Jihoon Son added a comment - - edited +1 for the latest patch. I'll commit this patch. Thanks for your contribution.
        Hide
        WanHeo Wan Heo added a comment -

        Oh, It's my mistake. I fixed the bug. So, "Download to CSV" will work well.

        Show
        WanHeo Wan Heo added a comment - Oh, It's my mistake. I fixed the bug. So, "Download to CSV" will work well.
        Hide
        hyunsik Hyunsik Choi added a comment -

        Until the latest patch, 'download csv' button works well. But, in the latest patch, download button doesn't work. Could you check this bug?

        Show
        hyunsik Hyunsik Choi added a comment - Until the latest patch, 'download csv' button works well. But, in the latest patch, download button doesn't work. Could you check this bug?
        Hide
        hyunsik Hyunsik Choi added a comment -

        +1

        I reviewed your latest patch. The patch looks good for me. Even there are some unused imports, I'll fix them before commit this. Thank you for your contribution!

        Show
        hyunsik Hyunsik Choi added a comment - +1 I reviewed your latest patch. The patch looks good for me. Even there are some unused imports, I'll fix them before commit this. Thank you for your contribution!
        Hide
        WanHeo Wan Heo added a comment -

        I appreciate for your kind comment. I agree with you. So, I uploaded the patch that the graph viewer is removed.

        Show
        WanHeo Wan Heo added a comment - I appreciate for your kind comment. I agree with you. So, I uploaded the patch that the graph viewer is removed.
        Hide
        hyunsik Hyunsik Choi added a comment -

        If other guys don't disagree my suggestion and If you are ok, could you upload the new patch without the graph viewer? If you do, that would be great for me.

        Show
        hyunsik Hyunsik Choi added a comment - If other guys don't disagree my suggestion and If you are ok, could you upload the new patch without the graph viewer? If you do, that would be great for me.
        Hide
        hyunsik Hyunsik Choi added a comment -

        Thank you for your contribution. I'm reviewing your patch. In overall, the patch looks good for me. But, I don't agree with some of the features that the patch provides.

        Tajo is a data warehouse system rather than BI solution. So, I think that graph viewer is not fit to Tajo system. I would like to suggest that graph viewer should be an individual application using Tajo.

        Show
        hyunsik Hyunsik Choi added a comment - Thank you for your contribution. I'm reviewing your patch. In overall, the patch looks good for me. But, I don't agree with some of the features that the patch provides. Tajo is a data warehouse system rather than BI solution. So, I think that graph viewer is not fit to Tajo system. I would like to suggest that graph viewer should be an individual application using Tajo.
        Hide
        jihoonson Jihoon Son added a comment -

        When the number of result tuples is large (about 60K), my web browser was down during drawing a chart.
        In my opinion, however, it is ok because charts are useful for the sufficiently small data.

        Show
        jihoonson Jihoon Son added a comment - When the number of result tuples is large (about 60K), my web browser was down during drawing a chart. In my opinion, however, it is ok because charts are useful for the sufficiently small data.
        Hide
        WanHeo Wan Heo added a comment - - edited

        Thanks to your comment. I uploaded the patch after rebasing.

        Show
        WanHeo Wan Heo added a comment - - edited Thanks to your comment. I uploaded the patch after rebasing.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I'm sorry for the late. The patch looks stale. Could you rebase the patch against the recent revision?

        Show
        hyunsik Hyunsik Choi added a comment - I'm sorry for the late. The patch looks stale. Could you rebase the patch against the recent revision?
        Hide
        WanHeo Wan Heo added a comment -

        In this patch, I added the following features.

        • Add "Paging" to the result view.
          • Users can set the number of displayed rows per page.
        • Add "Limiting size" of query results.
          • Users can set the result size. Users can see the results of the configured size.
        • Add "CSV export" to the result view.
          • Users can download the results in CSV file format.
        • Add "Graph function".
          • The results can be shown as a graph.
          • Bar, Pie and Line types are supported in this patch.
        Show
        WanHeo Wan Heo added a comment - In this patch, I added the following features. Add "Paging" to the result view. Users can set the number of displayed rows per page. Add "Limiting size" of query results. Users can set the result size. Users can see the results of the configured size. Add "CSV export" to the result view. Users can download the results in CSV file format. Add "Graph function". The results can be shown as a graph. Bar, Pie and Line types are supported in this patch.
        Hide
        WanHeo Wan Heo added a comment -

        I uploaded the second patch and screenshot.

        Show
        WanHeo Wan Heo added a comment - I uploaded the second patch and screenshot.
        Hide
        hyunsik Hyunsik Choi added a comment -

        The patch looks great for me. Thank you for nice contribution. There are a couple of issues that we need to discuss.

        Firstly, I wonder whether 'Download to CSV feature' reads data on HDFS or only outputs the data of HTML page? Unless the output table is explicitly given, the result data stored on HDFS will be automatically removed after a while. If Download feature is based on HDFS, it is only available for a while. Next and previous buttons also have the same problem. So, I think that Tajo web UI should show some part whose volume can be shown in the web page in the case where output data is too large.

        Second, google js works only online. However, The environments where Tajo usually is used have local networks that cannot access to Internet. So, the chart included in the patch won't work in those environments.

        When it comes to code, there are some bugs. 'Download feature' leads to wrong results. I've just experienced the number of results are fewer than that of the correct result. I'm expecting that the first row is omitted.

        In addition, two jsapi.js files are included in the patch.

        #	tajo-core/tajo-core-backend/src/main/resources/webapps/static/js/jsapi.js
        #	tajo-core/tajo-core-backend/src/main/resources/webapps/static/jsapi.js
        

        Thank you for your contribution.

        Show
        hyunsik Hyunsik Choi added a comment - The patch looks great for me. Thank you for nice contribution. There are a couple of issues that we need to discuss. Firstly, I wonder whether 'Download to CSV feature' reads data on HDFS or only outputs the data of HTML page? Unless the output table is explicitly given, the result data stored on HDFS will be automatically removed after a while. If Download feature is based on HDFS, it is only available for a while. Next and previous buttons also have the same problem. So, I think that Tajo web UI should show some part whose volume can be shown in the web page in the case where output data is too large. Second, google js works only online. However, The environments where Tajo usually is used have local networks that cannot access to Internet. So, the chart included in the patch won't work in those environments. When it comes to code, there are some bugs. 'Download feature' leads to wrong results. I've just experienced the number of results are fewer than that of the correct result. I'm expecting that the first row is omitted. In addition, two jsapi.js files are included in the patch. # tajo-core/tajo-core-backend/src/main/resources/webapps/ static /js/jsapi.js # tajo-core/tajo-core-backend/src/main/resources/webapps/ static /jsapi.js Thank you for your contribution.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I've assigned it to you.

        Show
        hyunsik Hyunsik Choi added a comment - I've assigned it to you.
        Hide
        WanHeo Wan Heo added a comment -

        I attached a patch.

        Show
        WanHeo Wan Heo added a comment - I attached a patch.
        Hide
        WanHeo Wan Heo added a comment -

        For your convenience, I attached a screenshot.

        Show
        WanHeo Wan Heo added a comment - For your convenience, I attached a screenshot.

          People

          • Assignee:
            WanHeo Wan Heo
            Reporter:
            WanHeo Wan Heo
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development