You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/01/23 17:40:34 UTC
[GitHub] [hive] jcamachor opened a new pull request #887: HIVE-22746
jcamachor opened a new pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] asfgit closed pull request #887: HIVE-22746
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] jcamachor commented on a change in pull request #887:
HIVE-22746
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887#discussion_r370946051
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprFactory.java
##########
@@ -0,0 +1,361 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.ql.parse.type;
+
+import java.math.BigDecimal;
+import java.time.ZoneId;
+import java.util.List;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.UDFArgumentException;
+import org.apache.hadoop.hive.ql.optimizer.calcite.CalciteSubquerySemanticException;
+import org.apache.hadoop.hive.ql.parse.ASTNode;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.SubqueryType;
+import org.apache.hadoop.hive.ql.udf.generic.GenericUDF;
+import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
+import org.apache.hive.common.util.DateUtils;
+
+/**
+ * Generic expressions factory. Currently, the only implementation produces
+ * Hive {@link ExprNodeDesc}.
+ */
+public abstract class ExprFactory<T> {
+
+ static final BigDecimal NANOS_PER_SEC_BD =
+ new BigDecimal(DateUtils.NANOS_PER_SEC);
+
+ /**
+ * Returns whether the input is an instance of the expression class.
+ */
+ protected abstract boolean isExprInstance(Object o);
+
+ /**
+ * Generates an expression from the input column. This may not necessarily
+ * be a column expression, e.g., if the column is a constant.
+ */
+ protected abstract T toExpr(ColumnInfo colInfo);
+
+ /* FIELD REFERENCES */
+ /**
+ * Returns whether the input object is a column reference expression.
+ */
+ protected abstract boolean isColumnRefExpr(Object o);
+
+ /**
+ * Creates column expression.
+ */
+ protected abstract T createColumnRefExpr(ColumnInfo colInfo);
+
+ /**
+ * Returns column name referenced by a column expression.
+ */
+ protected abstract String getColumnName(T expr);
+
+ /* CONSTANT EXPRESSIONS */
+ /**
+ * Returns whether the input expression is a constant expression.
+ */
+ protected abstract boolean isConstantExpr(Object o);
+
+ /**
+ * Returns whether all input expressions are constant expressions.
+ */
+ protected boolean isAllConstants(List<T> exprs) {
+ for (T expr : exprs) {
+ if (!isConstantExpr(expr)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Returns whether the input expression is a struct of
+ * constant expressions (all of them).
+ */
+ protected abstract boolean isConstantStruct(T expr);
+
+ /**
+ * Creates a null constant expression with void type.
+ */
+ protected abstract T createNullConstantExpr();
+
+ /**
+ * Creates a boolean constant expression from input value.
+ */
+ protected abstract T createBooleanConstantExpr(String value);
+
+ /**
+ * Creates a bigint constant expression from input value.
+ */
+ protected abstract T createBigintConstantExpr(String value);
+
+ /**
+ * Creates a int constant expression from input value.
+ */
+ protected abstract T createIntConstantExpr(String value);
+
+ /**
+ * Creates a smallint constant expression from input value.
+ */
+ protected abstract T createSmallintConstantExpr(String value);
+
+ /**
+ * Creates a tinyint constant expression from input value.
+ */
+ protected abstract T createTinyintConstantExpr(String value);
+
+ /**
+ * Creates a float constant expression from input value.
+ */
+ protected abstract T createFloatConstantExpr(String value);
+
+ /**
+ * Creates a double constant expression from input value.
+ */
+ protected abstract T createDoubleConstantExpr(String value);
+
+ /**
+ * Creates a decimal constant expression from input value.
+ * If the constant created from the input value is null, we return:
+ * 1) a constant expression containing null value if allowNullValueConstantExpr is true, or
+ * 2) null if allowNullValueConstantExpr is false.
+ */
+ protected abstract T createDecimalConstantExpr(String value, boolean allowNullValueConstantExpr);
+
+ /**
+ * Creates a string constant expression from input value.
+ */
+ protected abstract T createStringConstantExpr(String value);
+
+ /**
+ * Creates a date constant expression from input value.
+ */
+ protected abstract T createDateConstantExpr(String value);
+
+ /**
+ * Creates a timestamp constant expression from input value.
+ */
+ protected abstract T createTimestampConstantExpr(String value);
+
+ /**
+ * Creates a timestamp with local time zone constant expression from input value.
+ * ZoneId is the local time zone.
+ */
+ protected abstract T createTimestampLocalTimeZoneConstantExpr(String value, ZoneId zoneId);
+
+ /**
+ * Creates a interval year-month constant expression from input value.
+ */
+ protected abstract T createIntervalYearMonthConstantExpr(String value);
+
+ /**
+ * Creates a interval day-time constant expression from input value.
+ */
+ protected abstract T createIntervalDayTimeConstantExpr(String value);
+
+ /**
+ * Creates a interval year constant expression from input value.
+ */
+ protected abstract T createIntervalYearConstantExpr(String value);
+
+ /**
+ * Creates a interval month constant expression from input value.
+ */
+ protected abstract T createIntervalMonthConstantExpr(String value);
+
+ /**
+ * Creates a interval day constant expression from input value.
+ */
+ protected abstract T createIntervalDayConstantExpr(String value);
+
+ /**
+ * Creates a interval hour constant expression from input value.
+ */
+ protected abstract T createIntervalHourConstantExpr(String value);
+
+ /**
+ * Creates a interval minute constant expression from input value.
+ */
+ protected abstract T createIntervalMinuteConstantExpr(String value);
+
+ /**
+ * Creates a interval second constant expression from input value.
+ */
+ protected abstract T createIntervalSecondConstantExpr(String value);
+
+ /**
+ * Default generator for constant expression when type cannot be inferred
+ * from input query.
+ */
+ protected T createConstantExpr(String value) {
+ // The expression can be any one of Double, Long and Integer. We
+ // try to parse the expression in that order to ensure that the
+ // most specific type is used for conversion.
+ T result = null;
+ T result2 = null;
+ try {
+ result = createDoubleConstantExpr(value);
+ if (value != null && !value.toLowerCase().contains("e")) {
+ result2 = createDecimalConstantExpr(value, false);
Review comment:
What you describe above is the correct behavior: this code is already in Hive in `TypeCheckProcFactory`.
If an exception is thrown for decimal, we consider the constant to be a double. If an exception is not thrown for decimal, we try to parse as bigint and int. If any of those succeed, we will use those. But if none of them do, we will use the decimal.
If we want to refactor this crazy logic, I would rather create a new JIRA for that instead of doing it in this JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] jcamachor commented on a change in pull request #887:
HIVE-22746
Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887#discussion_r370945831
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java
##########
@@ -1001,17 +1002,10 @@ public static boolean isSame(List<ExprNodeDesc> first, List<ExprNodeDesc> second
// Given an expression this method figures out if the type for the expression belongs to string group
// e.g. (String, Char, Varchar etc)
public static boolean isStringType(ExprNodeDesc expr) {
- TypeInfo typeInfo = expr.getTypeInfo();
- if (typeInfo.getCategory() == ObjectInspector.Category.PRIMITIVE) {
- PrimitiveObjectInspector.PrimitiveCategory primitiveCategory = ((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory();
- if (PrimitiveObjectInspectorUtils.getPrimitiveGrouping(primitiveCategory) ==
- PrimitiveObjectInspectorUtils.PrimitiveGrouping.STRING_GROUP) {
- return true;
- }
- }
- return false;
+ return TypeCheckProcFactory.isStringType(expr.getTypeInfo());
Review comment:
It was not called anymore, I have just removed it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] miklosgergely commented on a change in pull request #887:
HIVE-22746
Posted by GitBox <gi...@apache.org>.
miklosgergely commented on a change in pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887#discussion_r370714925
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprFactory.java
##########
@@ -0,0 +1,361 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.ql.parse.type;
+
+import java.math.BigDecimal;
+import java.time.ZoneId;
+import java.util.List;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.UDFArgumentException;
+import org.apache.hadoop.hive.ql.optimizer.calcite.CalciteSubquerySemanticException;
+import org.apache.hadoop.hive.ql.parse.ASTNode;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.SubqueryType;
+import org.apache.hadoop.hive.ql.udf.generic.GenericUDF;
+import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
+import org.apache.hive.common.util.DateUtils;
+
+/**
+ * Generic expressions factory. Currently, the only implementation produces
+ * Hive {@link ExprNodeDesc}.
+ */
+public abstract class ExprFactory<T> {
+
+ static final BigDecimal NANOS_PER_SEC_BD =
+ new BigDecimal(DateUtils.NANOS_PER_SEC);
+
+ /**
+ * Returns whether the input is an instance of the expression class.
+ */
+ protected abstract boolean isExprInstance(Object o);
+
+ /**
+ * Generates an expression from the input column. This may not necessarily
+ * be a column expression, e.g., if the column is a constant.
+ */
+ protected abstract T toExpr(ColumnInfo colInfo);
+
+ /* FIELD REFERENCES */
+ /**
+ * Returns whether the input object is a column reference expression.
+ */
+ protected abstract boolean isColumnRefExpr(Object o);
+
+ /**
+ * Creates column expression.
+ */
+ protected abstract T createColumnRefExpr(ColumnInfo colInfo);
+
+ /**
+ * Returns column name referenced by a column expression.
+ */
+ protected abstract String getColumnName(T expr);
+
+ /* CONSTANT EXPRESSIONS */
+ /**
+ * Returns whether the input expression is a constant expression.
+ */
+ protected abstract boolean isConstantExpr(Object o);
+
+ /**
+ * Returns whether all input expressions are constant expressions.
+ */
+ protected boolean isAllConstants(List<T> exprs) {
+ for (T expr : exprs) {
+ if (!isConstantExpr(expr)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Returns whether the input expression is a struct of
+ * constant expressions (all of them).
+ */
+ protected abstract boolean isConstantStruct(T expr);
+
+ /**
+ * Creates a null constant expression with void type.
+ */
+ protected abstract T createNullConstantExpr();
+
+ /**
+ * Creates a boolean constant expression from input value.
+ */
+ protected abstract T createBooleanConstantExpr(String value);
+
+ /**
+ * Creates a bigint constant expression from input value.
+ */
+ protected abstract T createBigintConstantExpr(String value);
+
+ /**
+ * Creates a int constant expression from input value.
+ */
+ protected abstract T createIntConstantExpr(String value);
+
+ /**
+ * Creates a smallint constant expression from input value.
+ */
+ protected abstract T createSmallintConstantExpr(String value);
+
+ /**
+ * Creates a tinyint constant expression from input value.
+ */
+ protected abstract T createTinyintConstantExpr(String value);
+
+ /**
+ * Creates a float constant expression from input value.
+ */
+ protected abstract T createFloatConstantExpr(String value);
+
+ /**
+ * Creates a double constant expression from input value.
+ */
+ protected abstract T createDoubleConstantExpr(String value);
+
+ /**
+ * Creates a decimal constant expression from input value.
+ * If the constant created from the input value is null, we return:
+ * 1) a constant expression containing null value if allowNullValueConstantExpr is true, or
+ * 2) null if allowNullValueConstantExpr is false.
+ */
+ protected abstract T createDecimalConstantExpr(String value, boolean allowNullValueConstantExpr);
+
+ /**
+ * Creates a string constant expression from input value.
+ */
+ protected abstract T createStringConstantExpr(String value);
+
+ /**
+ * Creates a date constant expression from input value.
+ */
+ protected abstract T createDateConstantExpr(String value);
+
+ /**
+ * Creates a timestamp constant expression from input value.
+ */
+ protected abstract T createTimestampConstantExpr(String value);
+
+ /**
+ * Creates a timestamp with local time zone constant expression from input value.
+ * ZoneId is the local time zone.
+ */
+ protected abstract T createTimestampLocalTimeZoneConstantExpr(String value, ZoneId zoneId);
+
+ /**
+ * Creates a interval year-month constant expression from input value.
+ */
+ protected abstract T createIntervalYearMonthConstantExpr(String value);
+
+ /**
+ * Creates a interval day-time constant expression from input value.
+ */
+ protected abstract T createIntervalDayTimeConstantExpr(String value);
+
+ /**
+ * Creates a interval year constant expression from input value.
+ */
+ protected abstract T createIntervalYearConstantExpr(String value);
+
+ /**
+ * Creates a interval month constant expression from input value.
+ */
+ protected abstract T createIntervalMonthConstantExpr(String value);
+
+ /**
+ * Creates a interval day constant expression from input value.
+ */
+ protected abstract T createIntervalDayConstantExpr(String value);
+
+ /**
+ * Creates a interval hour constant expression from input value.
+ */
+ protected abstract T createIntervalHourConstantExpr(String value);
+
+ /**
+ * Creates a interval minute constant expression from input value.
+ */
+ protected abstract T createIntervalMinuteConstantExpr(String value);
+
+ /**
+ * Creates a interval second constant expression from input value.
+ */
+ protected abstract T createIntervalSecondConstantExpr(String value);
+
+ /**
+ * Default generator for constant expression when type cannot be inferred
+ * from input query.
+ */
+ protected T createConstantExpr(String value) {
+ // The expression can be any one of Double, Long and Integer. We
+ // try to parse the expression in that order to ensure that the
+ // most specific type is used for conversion.
+ T result = null;
+ T result2 = null;
+ try {
+ result = createDoubleConstantExpr(value);
+ if (value != null && !value.toLowerCase().contains("e")) {
+ result2 = createDecimalConstantExpr(value, false);
Review comment:
If createDecimalConstantExpr throws a NumberFormatException then the function won't try to parse the value as Bigint and Int, so I think it should be surrounded with try/catch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] miklosgergely commented on a change in pull request #887:
HIVE-22746
Posted by GitBox <gi...@apache.org>.
miklosgergely commented on a change in pull request #887: HIVE-22746
URL: https://github.com/apache/hive/pull/887#discussion_r370708452
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java
##########
@@ -1001,17 +1002,10 @@ public static boolean isSame(List<ExprNodeDesc> first, List<ExprNodeDesc> second
// Given an expression this method figures out if the type for the expression belongs to string group
// e.g. (String, Char, Varchar etc)
public static boolean isStringType(ExprNodeDesc expr) {
- TypeInfo typeInfo = expr.getTypeInfo();
- if (typeInfo.getCategory() == ObjectInspector.Category.PRIMITIVE) {
- PrimitiveObjectInspector.PrimitiveCategory primitiveCategory = ((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory();
- if (PrimitiveObjectInspectorUtils.getPrimitiveGrouping(primitiveCategory) ==
- PrimitiveObjectInspectorUtils.PrimitiveGrouping.STRING_GROUP) {
- return true;
- }
- }
- return false;
+ return TypeCheckProcFactory.isStringType(expr.getTypeInfo());
Review comment:
ExprNodeDescUtils.isStringType is only redirecting to TypeCheckProcFactory.isStringType, therefore it can be removed, and it's callers may call TypeCheckProcFactory.isStringType directly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org