TAJO-1239 ORDER BY with null column desc miss some data. (Hyoungjun Kim via hyunsik)
Closes #299 Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/c7dd002a Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/c7dd002a Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/c7dd002a Branch: refs/heads/index_support Commit: c7dd002a6e12b569e830396f84a32c832bf1e783 Parents: 06ed5dc Author: Hyunsik Choi <[email protected]> Authored: Fri Dec 12 15:56:19 2014 +0900 Committer: Hyunsik Choi <[email protected]> Committed: Fri Dec 12 15:56:19 2014 +0900 ---------------------------------------------------------------------- CHANGES | 3 + .../apache/tajo/engine/query/TestSortQuery.java | 39 +++++++++ .../tajo/storage/BaseTupleComparator.java | 4 +- .../tajo/storage/TestTupleComparator.java | 89 +++++++++++++++++++- 4 files changed, 131 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/c7dd002a/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 39499aa..d758459 100644 --- a/CHANGES +++ b/CHANGES @@ -109,6 +109,9 @@ Release 0.9.1 - unreleased BUG FIXES + TAJO-1239 ORDER BY with null column desc miss some data. + (Hyoungjun Kim via hyunsik) + TAJO-1244: tajo.worker.tmpdir.locations should use a validator for a list of paths. (hyunsik) http://git-wip-us.apache.org/repos/asf/tajo/blob/c7dd002a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestSortQuery.java ---------------------------------------------------------------------- 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 48a1464..21a2cc0 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 @@ -34,6 +34,8 @@ import org.junit.experimental.categories.Category; import java.sql.ResultSet; import java.util.TimeZone; +import static org.junit.Assert.assertEquals; + @Category(IntegrationTest.class) public class TestSortQuery extends QueryTestCaseBase { @@ -198,6 +200,43 @@ public class TestSortQuery extends QueryTestCaseBase { } @Test + public final void testSortOnNullColumn2() 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 asc"); + String ascExpected = "id,name\n" + + "-------------------------------\n" + + "1,111\n" + + "3,333\n" + + "2,null\n"; + + assertEquals(ascExpected, resultSetToString(res)); + res.close(); + + res = executeString("select * from table11 order by name desc"); + String descExpected = "id,name\n" + + "-------------------------------\n" + + "2,null\n" + + "3,333\n" + + "1,111\n"; + + assertEquals(descExpected, resultSetToString(res)); + res.close(); + } finally { + executeString("DROP TABLE table11 PURGE"); + } + } + + @Test public final void testSortOnUnicodeTextAsc() throws Exception { try { testingCluster.setAllTajoDaemonConfValue(ConfVars.$TEST_MIN_TASK_NUM.varname, "2"); http://git-wip-us.apache.org/repos/asf/tajo/blob/c7dd002a/tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BaseTupleComparator.java ---------------------------------------------------------------------- diff --git a/tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BaseTupleComparator.java b/tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BaseTupleComparator.java index b829f60..cba9ee0 100644 --- a/tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BaseTupleComparator.java +++ b/tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/BaseTupleComparator.java @@ -120,8 +120,8 @@ public class BaseTupleComparator extends TupleComparator implements ProtoObject< } else if (right.isNull()) { compVal = -1; } - if (nullFirsts[i]) { - if (compVal != 0) { + if (compVal != 0) { + if ((nullFirsts[i] && asc[i]) || (!nullFirsts[i] && !asc[i])) { compVal *= -1; } } http://git-wip-us.apache.org/repos/asf/tajo/blob/c7dd002a/tajo-storage/tajo-storage-common/src/test/java/org/apache/tajo/storage/TestTupleComparator.java ---------------------------------------------------------------------- diff --git a/tajo-storage/tajo-storage-common/src/test/java/org/apache/tajo/storage/TestTupleComparator.java b/tajo-storage/tajo-storage-common/src/test/java/org/apache/tajo/storage/TestTupleComparator.java index 639ca04..5e94531 100644 --- a/tajo-storage/tajo-storage-common/src/test/java/org/apache/tajo/storage/TestTupleComparator.java +++ b/tajo-storage/tajo-storage-common/src/test/java/org/apache/tajo/storage/TestTupleComparator.java @@ -21,12 +21,15 @@ package org.apache.tajo.storage; import org.apache.tajo.catalog.Schema; import org.apache.tajo.catalog.SortSpec; import org.apache.tajo.common.TajoDataTypes.Type; -import org.apache.tajo.datum.Datum; -import org.apache.tajo.datum.DatumFactory; +import org.apache.tajo.datum.*; import org.junit.After; import org.junit.Before; import org.junit.Test; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + import static org.junit.Assert.assertEquals; public class TestTupleComparator { @@ -74,4 +77,86 @@ public class TestTupleComparator { assertEquals(-1, tc.compare(tuple1, tuple2)); assertEquals(1, tc.compare(tuple2, tuple1)); } + + @Test + public void testNullFirst() throws Exception { + Schema schema = new Schema(); + schema.addColumn("id", Type.INT4); + schema.addColumn("name", Type.TEXT); + + VTuple tuple1 = new VTuple(2); + tuple1.put(0, new Int4Datum(1)); + tuple1.put(1, new TextDatum("111")); + + VTuple nullTuple = new VTuple(2); + nullTuple.put(0, new Int4Datum(2)); + nullTuple.put(1, NullDatum.get()); + + VTuple tuple3 = new VTuple(2); + tuple3.put(0, new Int4Datum(3)); + tuple3.put(1, new TextDatum("333")); + + List<Tuple> tupleList = new ArrayList<Tuple>(); + tupleList.add(tuple1); + tupleList.add(nullTuple); + tupleList.add(tuple3); + + SortSpec sortSpecAsc = new SortSpec(schema.getColumn(1), true, true); + BaseTupleComparator ascComp = new BaseTupleComparator(schema, new SortSpec[]{sortSpecAsc}); + Collections.sort(tupleList, ascComp); + + assertEquals(nullTuple, tupleList.get(0)); + assertEquals(tuple1, tupleList.get(1)); + assertEquals(tuple3, tupleList.get(2)); + + SortSpec sortSpecDesc = new SortSpec(schema.getColumn(1), false, true); + BaseTupleComparator descComp = new BaseTupleComparator(schema, new SortSpec[]{sortSpecDesc}); + Collections.sort(tupleList, descComp); + + assertEquals(tuple3, tupleList.get(0)); + assertEquals(tuple1, tupleList.get(1)); + assertEquals(nullTuple, tupleList.get(2)); + } + + @Test + public void testNullLast() throws Exception { + Schema schema = new Schema(); + schema.addColumn("id", Type.INT4); + schema.addColumn("name", Type.TEXT); + + VTuple tuple1 = new VTuple(2); + tuple1.put(0, new Int4Datum(1)); + tuple1.put(1, new TextDatum("111")); + + VTuple nullTuple = new VTuple(2); + nullTuple.put(0, new Int4Datum(2)); + nullTuple.put(1, NullDatum.get()); + + VTuple tuple3 = new VTuple(2); + tuple3.put(0, new Int4Datum(3)); + tuple3.put(1, new TextDatum("333")); + + List<Tuple> tupleList = new ArrayList<Tuple>(); + tupleList.add(tuple1); + tupleList.add(nullTuple); + tupleList.add(tuple3); + + SortSpec sortSpecAsc = new SortSpec(schema.getColumn(1), true, false); + BaseTupleComparator ascComp = new BaseTupleComparator(schema, new SortSpec[]{sortSpecAsc}); + + Collections.sort(tupleList, ascComp); + + assertEquals(tuple1, tupleList.get(0)); + assertEquals(tuple3, tupleList.get(1)); + assertEquals(nullTuple, tupleList.get(2)); + + SortSpec sortSpecDesc = new SortSpec(schema.getColumn(1), false, false); + BaseTupleComparator descComp = new BaseTupleComparator(schema, new SortSpec[]{sortSpecDesc}); + + Collections.sort(tupleList, descComp); + + assertEquals(nullTuple, tupleList.get(0)); + assertEquals(tuple3, tupleList.get(1)); + assertEquals(tuple1, tupleList.get(2)); + } }
