phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sama...@apache.org
Subject [32/50] [abbrv] phoenix git commit: PHOENIX-3422 PhoenixQueryBuilder doesn't make value string correctly for char(/varchar) column type.
Date Fri, 04 Nov 2016 22:13:59 GMT
PHOENIX-3422 PhoenixQueryBuilder doesn't make value string correctly for char(/varchar) column
type.

Signed-off-by: Sergey Soldatov <ssa@apache.org>


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

Branch: refs/heads/encodecolumns2
Commit: a225f5ffe773dde7a7efc1ada1d6dbda9d667cdf
Parents: cf70820
Author: Jeongdae Kim <kjd9306@gmail.com>
Authored: Fri Oct 28 17:13:23 2016 +0900
Committer: Sergey Soldatov <ssa@apache.org>
Committed: Wed Nov 2 12:58:46 2016 -0700

----------------------------------------------------------------------
 phoenix-hive/pom.xml                            |   7 +-
 .../phoenix/hive/query/PhoenixQueryBuilder.java | 129 ++++++++++---------
 .../hive/util/PhoenixStorageHandlerUtil.java    |   4 +-
 .../hive/query/PhoenixQueryBuilderTest.java     |  87 +++++++++++++
 4 files changed, 163 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/a225f5ff/phoenix-hive/pom.xml
----------------------------------------------------------------------
diff --git a/phoenix-hive/pom.xml b/phoenix-hive/pom.xml
index 250db49..c36e737 100644
--- a/phoenix-hive/pom.xml
+++ b/phoenix-hive/pom.xml
@@ -110,7 +110,12 @@
       <artifactId>hadoop-minicluster</artifactId>
       <scope>test</scope>
     </dependency>
-
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-all</artifactId>
+      <version>${mockito-all.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

http://git-wip-us.apache.org/repos/asf/phoenix/blob/a225f5ff/phoenix-hive/src/main/java/org/apache/phoenix/hive/query/PhoenixQueryBuilder.java
----------------------------------------------------------------------
diff --git a/phoenix-hive/src/main/java/org/apache/phoenix/hive/query/PhoenixQueryBuilder.java
b/phoenix-hive/src/main/java/org/apache/phoenix/hive/query/PhoenixQueryBuilder.java
index 8e3a972..a38814d 100644
--- a/phoenix-hive/src/main/java/org/apache/phoenix/hive/query/PhoenixQueryBuilder.java
+++ b/phoenix-hive/src/main/java/org/apache/phoenix/hive/query/PhoenixQueryBuilder.java
@@ -19,7 +19,9 @@ package org.apache.phoenix.hive.query;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Joiner;
+import com.google.common.base.Predicate;
 import com.google.common.base.Splitter;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
@@ -31,12 +33,9 @@ import org.apache.phoenix.hive.ql.index.IndexSearchCondition;
 import org.apache.phoenix.hive.util.PhoenixStorageHandlerUtil;
 import org.apache.phoenix.hive.util.PhoenixUtil;
 
+import javax.annotation.Nullable;
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -662,17 +661,17 @@ public class PhoenixQueryBuilder {
                     comparisonOp);
 
             if (comparisonOp.endsWith("UDFOPEqual")) {        // column = 1
-                appendCondition(sql, " = ", typeName, constantValues[0]);
+                sql.append(" = ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("UDFOPEqualOrGreaterThan")) {    // column >=
1
-                appendCondition(sql, " >= ", typeName, constantValues[0]);
+                sql.append(" >= ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("UDFOPGreaterThan")) {        // column >
1
-                appendCondition(sql, " > ", typeName, constantValues[0]);
+                sql.append(" > ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("UDFOPEqualOrLessThan")) {    // column <=
1
-                appendCondition(sql, " <= ", typeName, constantValues[0]);
+                sql.append(" <= ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("UDFOPLessThan")) {    // column < 1
-                appendCondition(sql, " < ", typeName, constantValues[0]);
+                sql.append(" < ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("UDFOPNotEqual")) {    // column != 1
-                appendCondition(sql, " != ", typeName, constantValues[0]);
+                sql.append(" != ").append(createConstantString(typeName, constantValues[0]));
             } else if (comparisonOp.endsWith("GenericUDFBetween")) {
                 appendBetweenCondition(jobConf, sql, condition.isNot(), typeName, constantValues);
             } else if (comparisonOp.endsWith("GenericUDFIn")) {
@@ -687,44 +686,16 @@ public class PhoenixQueryBuilder {
         return conditionColumnList;
     }
 
-    protected void appendCondition(StringBuilder sql, String comparisonOp, String typeName,
-                                   String conditionValue) {
-        if (serdeConstants.STRING_TYPE_NAME.equals(typeName)) {
-            sql.append(comparisonOp).append("'").append(conditionValue).append("'");
-        } else if (serdeConstants.DATE_TYPE_NAME.equals(typeName)) {
-            sql.append(comparisonOp).append("to_date('").append(conditionValue).append("')");
-        } else if (serdeConstants.TIMESTAMP_TYPE_NAME.equals(typeName)) {
-            sql.append(comparisonOp).append("to_timestamp('").append(conditionValue).append("')");
-        } else {
-            sql.append(comparisonOp).append(conditionValue);
-        }
-    }
-
     protected void appendBetweenCondition(JobConf jobConf, StringBuilder sql, boolean isNot,
                                           String typeName, String[] conditionValues) throws
             IOException {
-        if (isNot) {
-            sql.append(" not between ");
-        } else {
-            sql.append(" between ");
-        }
-
         try {
-            Arrays.sort(PhoenixStorageHandlerUtil.toTypedValues(jobConf, typeName,
-                    conditionValues));
-
-            if (serdeConstants.STRING_TYPE_NAME.equals(typeName)) {
-                sql.append("'").append(conditionValues[0]).append("'").append(" and ").append
-                        ("'").append(conditionValues[1]).append("'");
-            } else if (serdeConstants.DATE_TYPE_NAME.equals(typeName)) {
-                sql.append("to_date('").append(conditionValues[0]).append("')").append("
and ")
-                        .append("to_date('").append(conditionValues[1]).append("')");
-            } else if (serdeConstants.TIMESTAMP_TYPE_NAME.equals(typeName)) {
-                sql.append("to_timestamp('").append(conditionValues[0]).append("')").append("
and" +
-                        " ").append("to_timestamp('").append(conditionValues[1]).append("')");
-            } else {
-                sql.append(conditionValues[0]).append(" and ").append(conditionValues[1]);
-            }
+            Object[] typedValues = PhoenixStorageHandlerUtil.toTypedValues(jobConf, typeName,
conditionValues);
+            Arrays.sort(typedValues);
+
+            appendIfNot(isNot, sql).append(" between ")
+                    .append(Joiner.on(" and ").join(createConstantString(typeName, typedValues[0]),
+                    createConstantString(typeName, typedValues[1])));
         } catch (Exception e) {
             throw new IOException(e);
         }
@@ -732,29 +703,63 @@ public class PhoenixQueryBuilder {
 
     protected void appendInCondition(StringBuilder sql, boolean isNot, String typeName, String[]
             conditionValues) {
-        if (isNot) {
-            sql.append(" not in (");
-        } else {
-            sql.append(" in (");
+        List<Object> wrappedConstants = Lists.newArrayListWithCapacity(conditionValues.length);
+        for (String conditionValue : conditionValues) {
+            wrappedConstants.add(createConstantString(typeName, conditionValue));
         }
 
-        for (String conditionValue : conditionValues) {
-            if (serdeConstants.STRING_TYPE_NAME.equals(typeName)) {
-                sql.append("'").append(conditionValue).append("'");
-            } else if (serdeConstants.DATE_TYPE_NAME.equals(typeName)) {
-                sql.append("to_date('").append(conditionValue).append("')");
-            } else if (serdeConstants.TIMESTAMP_TYPE_NAME.equals(typeName)) {
-                sql.append("to_timestamp('").append(conditionValue).append("')");
-            } else {
-                sql.append(conditionValue);
-            }
+        appendIfNot(isNot, sql)
+                .append(" in (")
+                .append(Joiner.on(", ").join(wrappedConstants))
+                .append(")");
+    }
 
-            sql.append(", ");
+    private StringBuilder appendIfNot(boolean isNot, StringBuilder sb) {
+        return isNot ? sb.append(" not") : sb;
+    }
+
+    private static class ConstantStringWrapper {
+        private List<String> types;
+        private String prefix;
+        private String postfix;
+
+        ConstantStringWrapper(String type, String prefix, String postfix) {
+            this(Lists.newArrayList(type), prefix, postfix);
         }
 
-        sql.delete(sql.length() - 2, sql.length());
+        ConstantStringWrapper(List<String> types, String prefix, String postfix) {
+            this.types = types;
+            this.prefix = prefix;
+            this.postfix = postfix;
+        }
 
-        sql.append(")");
+        public Object apply(final String typeName, Object value) {
+            return Iterables.any(types, new Predicate<String>() {
+
+                @Override
+                public boolean apply(@Nullable String type) {
+                    return typeName.startsWith(type);
+                }
+            }) ? prefix + value + postfix : value;
+        }
     }
 
+    private static final String SINGLE_QUOTATION = "'";
+    private static List<ConstantStringWrapper> WRAPPERS = Lists.newArrayList(
+            new ConstantStringWrapper(Lists.newArrayList(
+                    serdeConstants.STRING_TYPE_NAME, serdeConstants.CHAR_TYPE_NAME,
+                    serdeConstants.VARCHAR_TYPE_NAME, serdeConstants.DATE_TYPE_NAME,
+                    serdeConstants.TIMESTAMP_TYPE_NAME
+            ), SINGLE_QUOTATION, SINGLE_QUOTATION),
+            new ConstantStringWrapper(serdeConstants.DATE_TYPE_NAME, "to_date(", ")"),
+            new ConstantStringWrapper(serdeConstants.TIMESTAMP_TYPE_NAME, "to_timestamp(",
")")
+    );
+
+    private Object createConstantString(String typeName, Object value) {
+        for (ConstantStringWrapper wrapper : WRAPPERS) {
+            value = wrapper.apply(typeName, value);
+        }
+
+        return value;
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/a225f5ff/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
b/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
index 1313fdb..0dd1134 100644
--- a/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
+++ b/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixStorageHandlerUtil.java
@@ -76,7 +76,9 @@ public class PhoenixStorageHandlerUtil {
         DateFormat df = null;
 
         for (int i = 0, limit = values.length; i < limit; i++) {
-            if (serdeConstants.STRING_TYPE_NAME.equals(typeName)) {
+            if (serdeConstants.STRING_TYPE_NAME.equals(typeName) ||
+                    typeName.startsWith(serdeConstants.CHAR_TYPE_NAME) ||
+                    typeName.startsWith(serdeConstants.VARCHAR_TYPE_NAME)) {
                 results[i] = values[i];
             } else if (serdeConstants.INT_TYPE_NAME.equals(typeName)) {
                 results[i] = new Integer(values[i]);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/a225f5ff/phoenix-hive/src/test/java/org/apache/phoenix/hive/query/PhoenixQueryBuilderTest.java
----------------------------------------------------------------------
diff --git a/phoenix-hive/src/test/java/org/apache/phoenix/hive/query/PhoenixQueryBuilderTest.java
b/phoenix-hive/src/test/java/org/apache/phoenix/hive/query/PhoenixQueryBuilderTest.java
new file mode 100644
index 0000000..7f1a7c3
--- /dev/null
+++ b/phoenix-hive/src/test/java/org/apache/phoenix/hive/query/PhoenixQueryBuilderTest.java
@@ -0,0 +1,87 @@
+package org.apache.phoenix.hive.query;
+
+import com.google.common.collect.Lists;
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.hadoop.hive.ql.plan.ExprNodeColumnDesc;
+import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
+import org.apache.hadoop.mapred.JobConf;
+import org.apache.phoenix.hive.ql.index.IndexSearchCondition;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixQueryBuilderTest {
+    private IndexSearchCondition mockedIndexSearchCondition(String comparisionOp,
+                                                            Object constantValue,
+                                                            Object[] constantValues,
+                                                            String columnName,
+                                                            String typeString,
+                                                            boolean isNot) {
+        IndexSearchCondition condition = mock(IndexSearchCondition.class);
+        when(condition.getComparisonOp()).thenReturn(comparisionOp);
+
+        ExprNodeConstantDesc constantDesc = mock(ExprNodeConstantDesc.class);
+        when(constantDesc.getValue()).thenReturn(constantValue);
+        when(condition.getConstantDesc()).thenReturn(constantDesc);
+
+        ExprNodeColumnDesc columnDesc = mock(ExprNodeColumnDesc.class);
+        when(columnDesc.getColumn()).thenReturn(columnName);
+        when(columnDesc.getTypeString()).thenReturn(typeString);
+        when(condition.getColumnDesc()).thenReturn(columnDesc);
+
+
+        if (ArrayUtils.isNotEmpty(constantValues)) {
+            ExprNodeConstantDesc[] constantDescs = new ExprNodeConstantDesc[constantValues.length];
+            for (int i = 0; i < constantDescs.length; i++) {
+                constantDescs[i] = mock(ExprNodeConstantDesc.class);
+                when(condition.getConstantDesc(i)).thenReturn(constantDescs[i]);
+                when(constantDescs[i].getValue()).thenReturn(constantValues[i]);
+            }
+            when(condition.getConstantDescs()).thenReturn(constantDescs);
+        }
+
+        when(condition.isNot()).thenReturn(isNot);
+
+        return condition;
+    }
+
+    @Test
+    public void testBuildQueryWithCharColumns() throws IOException {
+        final String tableName = "TEST_TABLE";
+        final String COLUMN_CHAR = "Column_Char";
+        final String COLUMN_VARCHAR = "Column_VChar";
+        final String expectedQueryPrefix = "select /*+ NO_CACHE  */ " + COLUMN_CHAR + ","
+ COLUMN_VARCHAR +
+                " from TEST_TABLE where ";
+
+        JobConf jobConf = new JobConf();
+        List<String> readColumnList = Lists.newArrayList(COLUMN_CHAR, COLUMN_VARCHAR);
+        List<IndexSearchCondition> searchConditions = Lists.newArrayList(
+            mockedIndexSearchCondition("GenericUDFOPEqual", "CHAR_VALUE", null, COLUMN_CHAR,
"char(10)", false),
+            mockedIndexSearchCondition("GenericUDFOPEqual", "CHAR_VALUE2", null, COLUMN_VARCHAR,
"varchar(10)", false)
+        );
+
+        assertEquals(expectedQueryPrefix + "Column_Char = 'CHAR_VALUE' and Column_VChar =
'CHAR_VALUE2'",
+                PhoenixQueryBuilder.getInstance().buildQuery(jobConf, tableName, readColumnList,
searchConditions));
+
+        searchConditions = Lists.newArrayList(
+                mockedIndexSearchCondition("GenericUDFIn", null,
+                        new Object[]{"CHAR1", "CHAR2", "CHAR3"}, COLUMN_CHAR, "char(10)",
false)
+        );
+
+        assertEquals(expectedQueryPrefix + "Column_Char in ('CHAR1', 'CHAR2', 'CHAR3')",
+                PhoenixQueryBuilder.getInstance().buildQuery(jobConf, tableName, readColumnList,
searchConditions));
+
+        searchConditions = Lists.newArrayList(
+                mockedIndexSearchCondition("GenericUDFBetween", null,
+                        new Object[]{"CHAR1", "CHAR2"}, COLUMN_CHAR, "char(10)", false)
+        );
+
+        assertEquals(expectedQueryPrefix + "Column_Char between 'CHAR1' and 'CHAR2'",
+                PhoenixQueryBuilder.getInstance().buildQuery(jobConf, tableName, readColumnList,
searchConditions));
+    }
+}


Mime
View raw message