You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by sa...@apache.org on 2016/11/04 22:13:59 UTC
[32/50] [abbrv] phoenix git commit: PHOENIX-3422 PhoenixQueryBuilder
doesn't make value string correctly for char(/varchar) column type.
PHOENIX-3422 PhoenixQueryBuilder doesn't make value string correctly for char(/varchar) column type.
Signed-off-by: Sergey Soldatov <ss...@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 <kj...@gmail.com>
Authored: Fri Oct 28 17:13:23 2016 +0900
Committer: Sergey Soldatov <ss...@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));
+ }
+}