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

ORDER BY with NULL FIRST misses some data

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None

      Description

      Even with TAJO-904 and TAJO-1239, ORDER BY NULL FIRST misses some data.
      Below test code doesn't pass.
      It seems that this problem comes from misbehaving setMaxRangeIfNull() method in TupleUtil.java, which places NULL in the range by only considering the existence of ASC/DESC.

       
          KeyValueSet tableOptions = new KeyValueSet();
          tableOptions.set(StorageConstants.TEXT_DELIMITER, StorageConstants.DEFAULT_FIELD_DELIMITER);
          tableOptions.set(StorageConstants.TEXT_NULL, "\\\\N");
      
          Schema schema = new Schema();
          schema.addColumn("id", Type.INT4);
          schema.addColumn("name", Type.TEXT);
          String[] data = new String[]{ "1|111", "2|\\N", "3|333" };
          TajoTestingCluster.createTable("table11", schema, tableOptions, data, 1);
      
          try {
            ResultSet res = executeString("select * from table11 order by name null first");
            String ascExpected = "id,name\n" +
                "-------------------------------\n" +
                "2,null\n" +
                "1,111\n" +
                "3,333\n";
      
            assertEquals(ascExpected, resultSetToString(res));
            res.close();
      
          } finally {
            executeString("DROP TABLE table11 PURGE");
          }
      

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/307#issuecomment-67583202

        Thank you for nice finding. After I test it on real cluster, I'll give reviews.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/307#issuecomment-67583202 Thank you for nice finding. After I test it on real cluster, I'll give reviews.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/307#issuecomment-68916077

        I'm sorry for late review. I tested various cases. It seems to work correctly.

        BTW, I found another bug, I don't think that your patch causes this problem. But, you may have interested in this problem.

        https://issues.apache.org/jira/browse/TAJO-1283

        I'll finish your review by tomorrow.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/307#issuecomment-68916077 I'm sorry for late review. I tested various cases. It seems to work correctly. BTW, I found another bug, I don't think that your patch causes this problem. But, you may have interested in this problem. https://issues.apache.org/jira/browse/TAJO-1283 I'll finish your review by tomorrow.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/307#issuecomment-70046306

        @sirpkt

        +1 LGTM

        I'm sorry for late review. I missed the review for few days. Ship it. Thank you for your contribution.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/307#issuecomment-70046306 @sirpkt +1 LGTM I'm sorry for late review. I missed the review for few days. Ship it. Thank you for your contribution.
        Hide
        sirpkt Keuntae Park added a comment -

        I just committed the patch as I got +1.

        Thank you for the review, Hyunsik Choi.

        Show
        sirpkt Keuntae Park added a comment - I just committed the patch as I got +1. Thank you for the review, Hyunsik Choi .
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #559 (See https://builds.apache.org/job/Tajo-master-build/559/)
        TAJO-1257: ORDER BY with NULL FIRST misses some data. (Keuntae Park) (sirpkt: rev 4d1006d56b7761500e16afe7be7826422850555a)

        • tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java
        • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #559 (See https://builds.apache.org/job/Tajo-master-build/559/ ) TAJO-1257 : ORDER BY with NULL FIRST misses some data. (Keuntae Park) (sirpkt: rev 4d1006d56b7761500e16afe7be7826422850555a) tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java CHANGES
        Hide
        hudson Hudson added a comment -

        ABORTED: Integrated in Tajo-master-CODEGEN-build #198 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/198/)
        TAJO-1257: ORDER BY with NULL FIRST misses some data. (Keuntae Park) (sirpkt: rev 4d1006d56b7761500e16afe7be7826422850555a)

        • tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java
        Show
        hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-CODEGEN-build #198 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/198/ ) TAJO-1257 : ORDER BY with NULL FIRST misses some data. (Keuntae Park) (sirpkt: rev 4d1006d56b7761500e16afe7be7826422850555a) tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java CHANGES tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java

          People

          • Assignee:
            sirpkt Keuntae Park
            Reporter:
            sirpkt Keuntae Park
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development