From fa86d0967ac98fff08023e12f9b17a8d9e3eab6b Mon Sep 17 00:00:00 2001
From: sirpkt <sirpkt@apache.org>
Date: Thu, 18 Dec 2014 16:16:06 +0900
Subject: [PATCH] Fix data missing bug in order by null first

---
 .../apache/tajo/engine/utils/TupleUtil.java   | 18 ++++++++----
 .../tajo/engine/query/TestSortQuery.java      | 28 +++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java b/tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java
index 3bb1b5ba44..027da50f28 100644
--- a/tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java
+++ b/tajo-core/src/main/java/org/apache/tajo/engine/utils/TupleUtil.java
@@ -97,13 +97,19 @@ public static void setMaxRangeIfNull(SortSpec[] sortSpecs, Schema sortSchema,
         continue;
       }
       if (columnStat.hasNullValue()) {
-        int rangeIndex = sortSpecs[i].isAscending() ? ranges.length - 1 : 0;
-        VTuple rangeTuple = sortSpecs[i].isAscending() ? (VTuple) ranges[rangeIndex].getEnd() : (VTuple) ranges[rangeIndex].getStart();
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Set null into range: " + col.getQualifiedName() + ", previous tuple is " + rangeTuple);
+        if (sortSpecs[i].isNullFirst()) {
+          int rangeIndex = 0;
+          VTuple rangeTuple = (VTuple) ranges[rangeIndex].getStart();
+          rangeTuple.put(i, NullDatum.get());
+        } else {
+          int rangeIndex = sortSpecs[i].isAscending() ? ranges.length - 1 : 0;
+          VTuple rangeTuple = sortSpecs[i].isAscending() ? (VTuple) ranges[rangeIndex].getEnd() : (VTuple) ranges[rangeIndex].getStart();
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Set null into range: " + col.getQualifiedName() + ", previous tuple is " + rangeTuple);
+          }
+          rangeTuple.put(i, NullDatum.get());
+          LOG.info("Set null into range: " + col.getQualifiedName() + ", current tuple is " + rangeTuple);
         }
-        rangeTuple.put(i, NullDatum.get());
-        LOG.info("Set null into range: " + col.getQualifiedName() + ", current tuple is " + rangeTuple);
       }
       i++;
     }
diff --git a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java b/tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
index 21a2cc0c6b..9c911f9930 100644
--- a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
+++ b/tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java
@@ -236,6 +236,34 @@ public final void testSortOnNullColumn2() throws Exception {
     }
   }
 
+  @Test
+  public final void testSortOnNullColumn3() throws Exception {
+    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");
+    }
+  }
+
   @Test
   public final void testSortOnUnicodeTextAsc() throws Exception {
     try {