You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by el...@apache.org on 2018/09/11 02:42:30 UTC
[2/4] phoenix git commit: PHOENIX-4884 Update INSTR to handle
literals and non-literals in either function argument
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/3b03e1b0
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/3b03e1b0
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/3b03e1b0
Branch: refs/heads/4.x-HBase-1.3
Commit: 3b03e1b0bddaf3b52637187ff556fbb39b1dddef
Parents: 012bb31
Author: Josh Elser <el...@apache.org>
Authored: Fri Aug 31 10:59:47 2018 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Mon Sep 10 22:14:05 2018 -0400
----------------------------------------------------------------------
.../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/3b03e1b0/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/3b03e1b0/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/3b03e1b0/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);