PHOENIX-4884 Update INSTR to handle literals and non-literals in either 
function argument

INSTR previously only handled arguments of the form non-literal and literal, 
but the documentation
doesn't clearly state this. We can support all variants though.


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/e83c6147
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/e83c6147
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/e83c6147

Branch: refs/heads/4.x-cdh5.15
Commit: e83c6147e5696b34d76de7ae16ab2233bda864ae
Parents: cb69793
Author: Josh Elser <els...@apache.org>
Authored: Fri Aug 31 15:59:47 2018 +0100
Committer: Pedro Boado <pbo...@apache.org>
Committed: Wed Oct 17 22:49:38 2018 +0100

----------------------------------------------------------------------
 .../apache/phoenix/end2end/InstrFunctionIT.java | 35 +++++++++
 .../expression/function/InstrFunction.java      | 78 +++++++++++++-------
 .../expression/function/InstrFunctionTest.java  | 44 +++++++----
 3 files changed, 114 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/e83c6147/phoenix-core/src/it/java/org/apache/phoenix/end2end/InstrFunctionIT.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InstrFunctionIT.java 
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InstrFunctionIT.java
index 270b1ec..bc86980 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InstrFunctionIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InstrFunctionIT.java
@@ -131,4 +131,39 @@ public class InstrFunctionIT extends 
ParallelStatsDisabledIT {
         testInstrFilter(conn, queryToExecute,"abcdefghijkl");
     }
 
+    @Test
+    public void testNonLiteralExpression() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        String tableName = generateUniqueName();
+        initTable(conn, tableName, "ASC", "asdf", "sdf");
+        // Should be able to use INSTR with a non-literal expression as the 
2nd argument
+        String query = "SELECT INSTR(name, substr) FROM " + tableName;
+        testInstr(conn, query, 2);
+    }
+
+    @Test
+    public void testNonLiteralSourceExpression() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        String tableName = generateUniqueName();
+        initTable(conn, tableName, "ASC", "asdf", "sdf");
+        // Using the function inside the SELECT will test client-side.
+        String query = "SELECT INSTR('asdf', 'sdf') FROM " + tableName;
+        testInstr(conn, query, 2);
+        query = "SELECT INSTR('asdf', substr) FROM " + tableName;
+        testInstr(conn, query, 2);
+        query = "SELECT INSTR('qwerty', 'sdf') FROM " + tableName;
+        testInstr(conn, query, 0);
+        query = "SELECT INSTR('qwerty', substr) FROM " + tableName;
+        testInstr(conn, query, 0);
+        // Test the built-in function in a where clause to make sure
+        // it works server-side (and not just client-side).
+        query = "SELECT name FROM " + tableName + " WHERE INSTR(name, substr) 
= 2";
+        testInstrFilter(conn, query, "asdf");
+        query = "SELECT name FROM " + tableName + " WHERE INSTR(name, 'sdf') = 
2";
+        testInstrFilter(conn, query, "asdf");
+        query = "SELECT name FROM " + tableName + " WHERE INSTR('asdf', 
substr) = 2";
+        testInstrFilter(conn, query, "asdf");
+        query = "SELECT name FROM " + tableName + " WHERE INSTR('asdf', 'sdf') 
= 2";
+        testInstrFilter(conn, query, "asdf");
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e83c6147/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
index 7a002f8..e6b4c16 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InstrFunction.java
@@ -30,7 +30,6 @@ import org.apache.phoenix.schema.tuple.Tuple;
 import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.schema.types.PInteger;
 import org.apache.phoenix.schema.types.PVarchar;
-import org.apache.phoenix.util.ByteUtil;
 
 @BuiltInFunction(name=InstrFunction.NAME, args={
         @Argument(allowedTypes={ PVarchar.class }),
@@ -38,8 +37,9 @@ import org.apache.phoenix.util.ByteUtil;
 public class InstrFunction extends ScalarFunction{
     
     public static final String NAME = "INSTR";
-    
-    private String strToSearch = null;
+
+    private String literalSourceStr = null;
+    private String literalSearchStr = null;
     
     public InstrFunction() { }
     
@@ -49,40 +49,62 @@ public class InstrFunction extends ScalarFunction{
     }
     
     private void init() {
-        Expression strToSearchExpression = getChildren().get(1);
-        if (strToSearchExpression instanceof LiteralExpression) {
-            Object strToSearchValue = ((LiteralExpression) 
strToSearchExpression).getValue();
-            if (strToSearchValue != null) {
-                this.strToSearch = strToSearchValue.toString();
-            }
+        literalSourceStr = maybeExtractLiteralString(getChildren().get(0));
+        literalSearchStr = maybeExtractLiteralString(getChildren().get(1));
+    }
+
+    /**
+     * Extracts the string-representation of {@code expr} only if {@code expr} 
is a
+     * non-null {@link LiteralExpression}.
+     *
+     * @param expr An Expression.
+     * @return The string value for the expression or null
+     */
+    private String maybeExtractLiteralString(Expression expr) {
+        if (expr instanceof LiteralExpression) {
+            // Whether the value is null or non-null, we can give it back 
right away
+            return (String) ((LiteralExpression) expr).getValue();
         }
+        return null;
     }
         
     
     @Override
     public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
-        Expression child = getChildren().get(0);
-        
-        if (!child.evaluate(tuple, ptr)) {
-            return false;
-        }
-        
-        if (ptr.getLength() == 0) {
-            ptr.set(ByteUtil.EMPTY_BYTE_ARRAY);
-            return true;
+        String sourceStr = literalSourceStr;
+        if (sourceStr == null) {
+            Expression child = getChildren().get(0);
+
+            if (!child.evaluate(tuple, ptr)) {
+                return false;
+            }
+
+            // We need something non-empty to search against
+            if (ptr.getLength() == 0) {
+              return true;
+            }
+
+            sourceStr = (String) PVarchar.INSTANCE.toObject(ptr, 
child.getSortOrder());
         }
-        
-        int position;
-        //Logic for Empty string search
-        if (strToSearch == null){
-            position = 0;
-            ptr.set(PInteger.INSTANCE.toBytes(position));
-            return true;
+
+        String searchStr = literalSearchStr;
+        // A literal was not provided, try to evaluate the expression to a 
literal
+        if (searchStr == null){
+            Expression child = getChildren().get(1);
+
+            if (!child.evaluate(tuple, ptr)) {
+              return false;
+            }
+
+            // A null (or zero-length) search string
+            if (ptr.getLength() == 0) {
+              return true;
+            }
+            
+            searchStr = (String) PVarchar.INSTANCE.toObject(ptr, 
child.getSortOrder());
         }
-        
-        String sourceStr = (String) PVarchar.INSTANCE.toObject(ptr, 
getChildren().get(0).getSortOrder());
 
-        position = sourceStr.indexOf(strToSearch) + 1;
+        int position = sourceStr.indexOf(searchStr) + 1;
         ptr.set(PInteger.INSTANCE.toBytes(position));
         return true;
     }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e83c6147/phoenix-core/src/test/java/org/apache/phoenix/expression/function/InstrFunctionTest.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/InstrFunctionTest.java
 
b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/InstrFunctionTest.java
index 359d772..6fd16ec 100644
--- 
a/phoenix-core/src/test/java/org/apache/phoenix/expression/function/InstrFunctionTest.java
+++ 
b/phoenix-core/src/test/java/org/apache/phoenix/expression/function/InstrFunctionTest.java
@@ -17,6 +17,8 @@
  */
 package org.apache.phoenix.expression.function;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.sql.SQLException;
@@ -32,18 +34,27 @@ import org.apache.phoenix.schema.types.PVarchar;
 import org.junit.Test;
 
 public class InstrFunctionTest {
+
+    private static Object evaluateExpression(String value, PDataType<?> 
dataType, String strToSearch, SortOrder order) throws SQLException {
+      Expression inputArg = 
LiteralExpression.newConstant(value,dataType,order);
+
+      Expression strToSearchExp = 
LiteralExpression.newConstant(strToSearch,dataType);
+      List<Expression> expressions = 
Arrays.<Expression>asList(inputArg,strToSearchExp);
+      Expression instrFunction = new InstrFunction(expressions);
+      ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+      instrFunction.evaluate(null,ptr);
+      return instrFunction.getDataType().toObject(ptr);
+    }
     
-    public static void inputExpression(String value, PDataType dataType, 
String strToSearch,Integer expected, SortOrder order) throws SQLException{
-        Expression inputArg = 
LiteralExpression.newConstant(value,dataType,order);
-        
-        Expression strToSearchExp = 
LiteralExpression.newConstant(strToSearch,dataType);
-        List<Expression> expressions = 
Arrays.<Expression>asList(inputArg,strToSearchExp);
-        Expression instrFunction = new InstrFunction(expressions);
-        ImmutableBytesWritable ptr = new ImmutableBytesWritable();
-        instrFunction.evaluate(null,ptr);
-        Integer result = (Integer) instrFunction.getDataType().toObject(ptr);
-        assertTrue(result.compareTo(expected) == 0);
-        
+    public static void inputExpression(String value, PDataType<?> dataType, 
String strToSearch,Integer expected, SortOrder order) throws SQLException {
+        Object obj = evaluateExpression(value, dataType, strToSearch, order);
+        assertNotNull("Result was unexpectedly null", obj);
+        assertTrue(((Integer) obj).compareTo(expected) == 0);
+    }
+
+    public static void inputNullExpression(String value, PDataType<?> 
dataType, String strToSearch, SortOrder order) throws SQLException {
+        Object obj = evaluateExpression(value, dataType, strToSearch, order);
+        assertNull("Result was unexpectedly non-null", obj);
     }
     
     
@@ -72,10 +83,13 @@ public class InstrFunctionTest {
         inputExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, " ", 6, 
SortOrder.ASC);
         
         inputExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, " ", 6, 
SortOrder.DESC);
-        
-        inputExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, "", 0, 
SortOrder.ASC);
-        
-        inputExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, "", 0, 
SortOrder.DESC);
+
+        // Phoenix can't represent empty strings, so an empty or null search 
string should return null
+        // See PHOENIX-4884 for more chatter.
+        inputNullExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, "", 
SortOrder.ASC);
+        inputNullExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, "", 
SortOrder.DESC);
+        inputNullExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, null, 
SortOrder.ASC);
+        inputNullExpression("ABCDE FGHiJKL",PVarchar.INSTANCE, null, 
SortOrder.DESC);
         
         inputExpression("ABCDEABC",PVarchar.INSTANCE, "ABC", 1, SortOrder.ASC);
         

Reply via email to