Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.1
    • Component/s: SQL Shell, Web UI
    • Labels:
      None

      Description

      Tajo shows users function list in two places(CLI,Web) in only alphabetical `function name` and emunerical(not alphabetical) `result type` order. This issue propose to display them in full alphabetical `function name`, `type`, `result type', 'argument type' order.

      tsql

      default> \df sum
       Name            | Result type     | Argument types        | Description                                   | Type      
      -----------------+-----------------+-----------------------+-----------------------------------------------+-----------
       sum             | INT8            | int4                  | the sum of a set of numbers                   | AGGREGATIO
       sum             | INT8            | int8                  | the sum of a set of numbers                   | AGGREGATIO
       sum             | INT8            | int8                  | the sum of a distinct and non-null values     | DISTINCT_A
       sum             | INT8            | int4                  | the sum of a distinct and non-null values     | DISTINCT_A
       sum             | FLOAT8          | float8                | the sum of a distinct and non-null values     | DISTINCT_A
       sum             | FLOAT8          | float4                | the sum of a distinct and non-null values     | DISTINCT_A
       sum             | FLOAT8          | float8                | the sum of a set of numbers                   | AGGREGATIO
       sum             | FLOAT8          | float4                | the sum of a set of numbers                   | AGGREGATIO
      
      default> \df sum
       Name            | Result type     | Argument types        | Description                                   | Type      
      -----------------+-----------------+-----------------------+-----------------------------------------------+-----------
       sum             | float8          | float4                | the sum of a set of numbers                   | aggregatio
       sum             | float8          | float8                | the sum of a set of numbers                   | aggregatio
       sum             | int8            | int4                  | the sum of a set of numbers                   | aggregatio
       sum             | int8            | int8                  | the sum of a set of numbers                   | aggregatio
       sum             | float8          | float4                | the sum of a distinct and non-null values     | distinct_a
       sum             | float8          | float8                | the sum of a distinct and non-null values     | distinct_a
       sum             | int8            | int4                  | the sum of a distinct and non-null values     | distinct_a
       sum             | int8            | int8                  | the sum of a distinct and non-null values     | distinct_a
      

      `http://localhost:26080/functions.jsp` will change the result in the same order.

      1. after_lag.png
        137 kB
        Dongjoon Hyun
      2. before_lag.png
        145 kB
        Dongjoon Hyun
      3. TAJO-1452.Hyun.150327.0.patch.txt
        8 kB
        Dongjoon Hyun

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dongjoon-hyun opened a pull request:

        https://github.com/apache/tajo/pull/469

        TAJO-1452: Improve function listing order

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1452

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/469.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #469


        commit 2a6e2044746cbb3a174faea6f31fb788d72ea332
        Author: Dongjoon Hyun <dongjoon@apache.org>
        Date: 2015-03-27T04:24:37Z

        TAJO-1452: Improve function listing order


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dongjoon-hyun opened a pull request: https://github.com/apache/tajo/pull/469 TAJO-1452 : Improve function listing order You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1452 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/469.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #469 commit 2a6e2044746cbb3a174faea6f31fb788d72ea332 Author: Dongjoon Hyun <dongjoon@apache.org> Date: 2015-03-27T04:24:37Z TAJO-1452 : Improve function listing order
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun closed the pull request at:

        https://github.com/apache/tajo/pull/469

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun closed the pull request at: https://github.com/apache/tajo/pull/469
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dongjoon-hyun opened a pull request:

        https://github.com/apache/tajo/pull/470

        TAJO-1452: Improve function listing order

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1452

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/470.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #470


        commit 2a6e2044746cbb3a174faea6f31fb788d72ea332
        Author: Dongjoon Hyun <dongjoon@apache.org>
        Date: 2015-03-27T04:24:37Z

        TAJO-1452: Improve function listing order


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dongjoon-hyun opened a pull request: https://github.com/apache/tajo/pull/470 TAJO-1452 : Improve function listing order You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1452 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/470.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #470 commit 2a6e2044746cbb3a174faea6f31fb788d72ea332 Author: Dongjoon Hyun <dongjoon@apache.org> Date: 2015-03-27T04:24:37Z TAJO-1452 : Improve function listing order
        Hide
        sirpkt Keuntae Park added a comment -

        Cool feature, Dongjoon Hyun!

        I think it would look better if functions of the same name and the same return type are listed together.
        For example, like followings:

         
        default> \df sum
         Name            | Result type     | Argument types        | Description                                   | Type      
        -----------------+-----------------+-----------------------+-----------------------------------------------+-----------
         sum             | float8          | float4, float8        | the sum of a set of numbers                   | aggregatio
        ...
        

        How about this idea?

        Show
        sirpkt Keuntae Park added a comment - Cool feature, Dongjoon Hyun ! I think it would look better if functions of the same name and the same return type are listed together. For example, like followings: default > \df sum Name | Result type | Argument types | Description | Type -----------------+-----------------+-----------------------+-----------------------------------------------+----------- sum | float8 | float4, float8 | the sum of a set of numbers | aggregatio ... How about this idea?
        Hide
        sirpkt Keuntae Park added a comment -

        Hmm, this may be confused with the function with two arguments.
        However, there can be better way to express multiple types

        Show
        sirpkt Keuntae Park added a comment - Hmm, this may be confused with the function with two arguments. However, there can be better way to express multiple types
        Hide
        sirpkt Keuntae Park added a comment -

        It is just one suggestion for more fancy view of function lists
        because I sometimes feel that current function list is too messy with functions of the same name.
        I really like your issue

        Show
        sirpkt Keuntae Park added a comment - It is just one suggestion for more fancy view of function lists because I sometimes feel that current function list is too messy with functions of the same name. I really like your issue
        Hide
        dongjoon Dongjoon Hyun added a comment -

        Thank you, Keuntae Park. I had tried to group the functions as you suggest, but it was not easy due to multiple input functions as you mentioned.
        So, I designed like this up to now. However, there can be many improvement ideas.
        I will welcome any comments in addition to yours. Thank you again.

        Warmly,
        Dongjoon

        Show
        dongjoon Dongjoon Hyun added a comment - Thank you, Keuntae Park . I had tried to group the functions as you suggest, but it was not easy due to multiple input functions as you mentioned. So, I designed like this up to now. However, there can be many improvement ideas. I will welcome any comments in addition to yours. Thank you again. Warmly, Dongjoon
        Hide
        dongjoon Dongjoon Hyun added a comment -

        I added `before` and `after` images in Tajo Master WebUI.

        Show
        dongjoon Dongjoon Hyun added a comment - I added `before` and `after` images in Tajo Master WebUI.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12707685/TAJO-1452.Hyun.150327.0.patch.txt
        against master revision release-0.9.0-rc0-222-gb1e174e.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-cli tajo-core.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/663//testReport/
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/663//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707685/TAJO-1452.Hyun.150327.0.patch.txt against master revision release-0.9.0-rc0-222-gb1e174e. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-catalog/tajo-catalog-common tajo-cli tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/663//testReport/ Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/663//console This message is automatically generated.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12707695/before_lag.png
        against master revision release-0.9.0-rc0-222-gb1e174e.

        -1 patch. The patch command could not apply the patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/665//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707695/before_lag.png against master revision release-0.9.0-rc0-222-gb1e174e. -1 patch. The patch command could not apply the patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/665//console This message is automatically generated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-88878730

        Rebased.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-88878730 Rebased.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-89479972

        Hi @dongjoon-hyun,

        Unfortunately, I don't have enough time to review this patch. Nevertheless, I greatly appreciate your recent contributions.

        Besides, I'm going to give you some useful suggestion. You may close the previous PR (https://github.com/apache/tajo/pull/469) and open this PR in order to rebase or update the patch.

        Actually, you don't need to close and open new PR for each update. You can update an existing PR by just pushing some changes to the branch used for the existing PR. I hope that my advice would be helpful to your contribution.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-89479972 Hi @dongjoon-hyun, Unfortunately, I don't have enough time to review this patch. Nevertheless, I greatly appreciate your recent contributions. Besides, I'm going to give you some useful suggestion. You may close the previous PR ( https://github.com/apache/tajo/pull/469 ) and open this PR in order to rebase or update the patch. Actually, you don't need to close and open new PR for each update. You can update an existing PR by just pushing some changes to the branch used for the existing PR. I hope that my advice would be helpful to your contribution.
        Hide
        dongjoon Dongjoon Hyun added a comment -

        Thank you, Hyunsik! I will update the previous PR from now.

        Show
        dongjoon Dongjoon Hyun added a comment - Thank you, Hyunsik! I will update the previous PR from now.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-89547239

        Thank you, @hyunsik ! As I mentioned in Jira, I will update the previous PR from now.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-89547239 Thank you, @hyunsik ! As I mentioned in Jira, I will update the previous PR from now.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-89548002

        Travis CI error looks temporal error in Travis CI system. I tested with the same command with Travis CI, and the pass result file is in the following link.

        https://app.box.com/s/d6pz3ocmigno6dzfrcy3eqoz44215lj7

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-89548002 Travis CI error looks temporal error in Travis CI system. I tested with the same command with Travis CI, and the pass result file is in the following link. https://app.box.com/s/d6pz3ocmigno6dzfrcy3eqoz44215lj7
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-94593765

        Rebased.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-94593765 Rebased.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/470#issuecomment-100129101

        +1
        The change looks straightforward and reasonable. I'll commit it shortly.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/470#issuecomment-100129101 +1 The change looks straightforward and reasonable. I'll commit it shortly.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/470

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/470
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed to master as well as 0.10.1 branch. Thank you for your contribution.

        Show
        hyunsik Hyunsik Choi added a comment - committed to master as well as 0.10.1 branch. Thank you for your contribution.
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #338 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/338/)
        TAJO-1452: Improve function listing order (Contributed Dongjoon Hyun, Committed by hyunsik) (hyunsik: rev 1baf8dce6521fcab041c2e94b1ba5eb016611bb2)

        • tajo-core/src/main/java/org/apache/tajo/util/JSPUtil.java
        • CHANGES
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/DescFunctionCommand.java
        • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionSignature.java
        • tajo-core/src/main/resources/webapps/admin/functions.jsp
        • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionUtil.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #338 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/338/ ) TAJO-1452 : Improve function listing order (Contributed Dongjoon Hyun, Committed by hyunsik) (hyunsik: rev 1baf8dce6521fcab041c2e94b1ba5eb016611bb2) tajo-core/src/main/java/org/apache/tajo/util/JSPUtil.java CHANGES tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/DescFunctionCommand.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionSignature.java tajo-core/src/main/resources/webapps/admin/functions.jsp tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionUtil.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #697 (See https://builds.apache.org/job/Tajo-master-build/697/)
        TAJO-1452: Improve function listing order (Contributed Dongjoon Hyun, Committed by hyunsik) (hyunsik: rev 1baf8dce6521fcab041c2e94b1ba5eb016611bb2)

        • tajo-core/src/main/java/org/apache/tajo/util/JSPUtil.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/DescFunctionCommand.java
        • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionSignature.java
        • tajo-core/src/main/resources/webapps/admin/functions.jsp
        • CHANGES
        • tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionUtil.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #697 (See https://builds.apache.org/job/Tajo-master-build/697/ ) TAJO-1452 : Improve function listing order (Contributed Dongjoon Hyun, Committed by hyunsik) (hyunsik: rev 1baf8dce6521fcab041c2e94b1ba5eb016611bb2) tajo-core/src/main/java/org/apache/tajo/util/JSPUtil.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/commands/DescFunctionCommand.java tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionSignature.java tajo-core/src/main/resources/webapps/admin/functions.jsp CHANGES tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/function/FunctionUtil.java

          People

          • Assignee:
            dongjoon Dongjoon Hyun
            Reporter:
            dongjoon Dongjoon Hyun
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development