Repository: phoenix Updated Branches: refs/heads/5.x-HBase-2.0 81784909f -> 6e7b7d4ca
PHOENIX-4624 COLLATION_KEY function cannot handle null values (Shehzaad Nakhoda) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/6e7b7d4c Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/6e7b7d4c Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/6e7b7d4c Branch: refs/heads/5.x-HBase-2.0 Commit: 6e7b7d4ca7fe0cf7b8e6213a2644758c0096be2e Parents: 8178490 Author: James Taylor <jtay...@salesforce.com> Authored: Mon Mar 5 13:44:09 2018 -0800 Committer: James Taylor <jtay...@salesforce.com> Committed: Mon Mar 5 13:50:35 2018 -0800 ---------------------------------------------------------------------- .../phoenix/end2end/CollationKeyFunctionIT.java | 37 ++++++++++++++------ .../function/CollationKeyFunction.java | 10 ++++++ .../function/CollationKeyFunctionTest.java | 7 ++++ 3 files changed, 44 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/6e7b7d4c/phoenix-core/src/it/java/org/apache/phoenix/end2end/CollationKeyFunctionIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CollationKeyFunctionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CollationKeyFunctionIT.java index efbab64..9f9e4d1 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CollationKeyFunctionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CollationKeyFunctionIT.java @@ -41,7 +41,10 @@ public class CollationKeyFunctionIT extends ParallelStatsDisabledIT { // (0-6) chinese characters "\u963f", "\u55c4", "\u963e", "\u554a", "\u4ec8", "\u3d9a", "\u9f51", // (7-13) western characters, some with accent - "a", "b", "ä", "A", "a", "ä", "A" }; + "a", "b", "ä", "A", "a", "ä", "A", + // null for null-input tests + null + }; @Before public void initAndPopulateTable() throws Exception { @@ -69,27 +72,27 @@ public class CollationKeyFunctionIT extends ParallelStatsDisabledIT { @Test public void testZhSort() throws Exception { - queryWithCollKeyDefaultArgsWithExpectedOrder("zh", 0, 6, new Integer[] { 3, 0, 1, 6, 5, 4, 2 }); + queryWithCollKeyDefaultArgsWithExpectedOrder("zh", false, 0, 6, new Integer[] { 3, 0, 1, 6, 5, 4, 2 }); } @Test public void testZhTwSort() throws Exception { - queryWithCollKeyDefaultArgsWithExpectedOrder("zh_TW", 0, 6, new Integer[] { 0, 3, 4, 1, 5, 2, 6 }); + queryWithCollKeyDefaultArgsWithExpectedOrder("zh_TW", false, 0, 6, new Integer[] { 0, 3, 4, 1, 5, 2, 6 }); } @Test public void testZhTwStrokeSort() throws Exception { - queryWithCollKeyDefaultArgsWithExpectedOrder("zh_TW_STROKE", 0, 6, new Integer[] { 4, 2, 0, 3, 1, 6, 5 }); + queryWithCollKeyDefaultArgsWithExpectedOrder("zh_TW_STROKE", false, 0, 6, new Integer[] { 4, 2, 0, 3, 1, 6, 5 }); } @Test public void testZhStrokeSort() throws Exception { - queryWithCollKeyDefaultArgsWithExpectedOrder("zh__STROKE", 0, 6, new Integer[] { 0, 1, 3, 4, 6, 2, 5 }); + queryWithCollKeyDefaultArgsWithExpectedOrder("zh__STROKE", false, 0, 6, new Integer[] { 0, 1, 3, 4, 6, 2, 5 }); } @Test public void testZhPinyinSort() throws Exception { - queryWithCollKeyDefaultArgsWithExpectedOrder("zh__PINYIN", 0, 6, new Integer[] { 0, 1, 3, 4, 6, 2, 5 }); + queryWithCollKeyDefaultArgsWithExpectedOrder("zh__PINYIN", false, 0, 6, new Integer[] { 0, 1, 3, 4, 6, 2, 5 }); } @Test @@ -120,6 +123,18 @@ public class CollationKeyFunctionIT extends ParallelStatsDisabledIT { queryWithCollKeyWithStrengthWithExpectedOrder("en", Collator.TERTIARY, true, 7, 13, new Integer[] { 8, 12, 9, 13, 10, 11, 7 }); } + + // Null before anything else when doing ascending sort + @Test + public void testSortWithNullInputAsc() throws Exception { + queryWithCollKeyDefaultArgsWithExpectedOrder("en", false, 13, 14, new Integer[] {14, 13}); + } + + // Null before anything else when doing descending sort (same behavior when doing order by without collation_key) + @Test + public void testSortWithNullInputDesc() throws Exception { + queryWithCollKeyDefaultArgsWithExpectedOrder("en", true, 13, 14, new Integer[] {14, 13}); + } /** @@ -132,14 +147,16 @@ public class CollationKeyFunctionIT extends ParallelStatsDisabledIT { * This is the same as the ID column * @throws SQLException */ - private void queryWithCollKeyDefaultArgsWithExpectedOrder(String localeString, Integer beginIndex, Integer endIndex, + private void queryWithCollKeyDefaultArgsWithExpectedOrder(String localeString, boolean isDescending, Integer beginIndex, Integer endIndex, Integer[] expectedIndexOrder) throws Exception { + String sortOrder = isDescending ? "DESC" : ""; + String query = String.format( - "SELECT id, data FROM %s WHERE ID BETWEEN %d AND %d ORDER BY COLLATION_KEY(data, '%s')", tableName, - beginIndex, endIndex, localeString); + "SELECT id, data FROM %s WHERE ID BETWEEN %d AND %d ORDER BY COLLATION_KEY(data, '%s') %s", tableName, + beginIndex, endIndex, localeString, sortOrder); queryWithExpectedOrder(query, expectedIndexOrder); } - + /** * Same as above, except the upperCase collator argument is set to true */ http://git-wip-us.apache.org/repos/asf/phoenix/blob/6e7b7d4c/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CollationKeyFunction.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CollationKeyFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CollationKeyFunction.java index 827f70a..6adfb7e 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CollationKeyFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/CollationKeyFunction.java @@ -118,6 +118,11 @@ public class CollationKeyFunction extends ScalarFunction { if (LOG.isTraceEnabled()) { LOG.trace("CollationKey inputString: " + inputString); } + + if (inputString == null) { + return true; + } + byte[] collationKeyByteArray = collator.getCollationKey(inputString).toByteArray(); if (LOG.isTraceEnabled()) { @@ -196,4 +201,9 @@ public class CollationKeyFunction extends ScalarFunction { // this method. return type.cast(((LiteralExpression) expression).getValue()); } + + @Override + public boolean isNullable() { + return getChildren().get(0).isNullable(); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/6e7b7d4c/phoenix-core/src/test/java/org/apache/phoenix/expression/function/CollationKeyFunctionTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/CollationKeyFunctionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/CollationKeyFunctionTest.java index f57a937..950875b 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/CollationKeyFunctionTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/CollationKeyFunctionTest.java @@ -18,6 +18,7 @@ package org.apache.phoenix.expression.function; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; import java.text.Collator; @@ -119,6 +120,12 @@ public class CollationKeyFunctionTest { public void testUpperCaseCollationKeyBytes() throws Exception { testCollationKeysEqual(new String[] { "abcdef", "ABCDEF", "aBcDeF" }, "en", Boolean.TRUE, null, null); } + + @Test + public void testNullCollationKey() throws Exception { + List<ByteArrayAndInteger> collationKeys = calculateCollationKeys(new String[] { null }, "en", null, null, null); + assertNull(collationKeys.get(0).byteArray); + } @Test public void testEqualCollationKeysForPrimaryStrength() throws Exception {
