diff --git ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java index aef46da..533a3ab 100644 --- ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java +++ ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java @@ -35,7 +35,9 @@ import org.apache.hadoop.hive.ql.session.SessionState.LogHelper; import org.apache.hadoop.hive.ql.udf.generic.GenericUDF; import org.apache.hadoop.hive.ql.udf.generic.GenericUDFBaseCompare; +import org.apache.hadoop.hive.ql.udf.generic.GenericUDFBetween; import org.apache.hadoop.hive.ql.udf.generic.GenericUDFBridge; +import org.apache.hadoop.hive.ql.udf.generic.GenericUDFIn; import org.apache.hadoop.hive.ql.udf.generic.GenericUDFMacro; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils; @@ -62,7 +64,7 @@ * exactly what we want. */ private GenericUDF genericUDF; - private List chidren; + private List children; private transient String funcText; /** * This class uses a writableObjectInspector rather than a TypeInfo to store @@ -72,7 +74,7 @@ //Is this an expression that should perform a comparison for sorted searches private boolean isSortedExpr; - public ExprNodeGenericFuncDesc() {; + public ExprNodeGenericFuncDesc() { } /* If the function has an explicit name like func(args) then call a @@ -94,7 +96,7 @@ public ExprNodeGenericFuncDesc(ObjectInspector oi, GenericUDF genericUDF, ObjectInspectorUtils.getWritableObjectInspector(oi); assert (genericUDF != null); this.genericUDF = genericUDF; - this.chidren = children; + this.children = children; this.funcText = funcText; } @@ -123,12 +125,12 @@ public void setGenericUDF(GenericUDF genericUDF) { } public void setChildren(List children) { - chidren = children; + this.children = children; } @Override public List getChildren() { - return chidren; + return children; } @Override @@ -142,12 +144,12 @@ public String toString() { sb.append(" "); } sb.append("("); - if (chidren != null) { - for (int i = 0; i < chidren.size(); i++) { + if (children != null) { + for (int i = 0; i < children.size(); i++) { if (i > 0) { sb.append(", "); } - sb.append(chidren.get(i)); + sb.append(children.get(i)); } } sb.append(")"); @@ -157,9 +159,9 @@ public String toString() { @Override public String getExprString() { // Get the children expr strings - String[] childrenExprStrings = new String[chidren.size()]; + String[] childrenExprStrings = new String[children.size()]; for (int i = 0; i < childrenExprStrings.length; i++) { - childrenExprStrings[i] = chidren.get(i).getExprString(); + childrenExprStrings[i] = children.get(i).getExprString(); } return genericUDF.getDisplayString(childrenExprStrings); @@ -168,10 +170,10 @@ public String getExprString() { @Override public List getCols() { List colList = new ArrayList(); - if (chidren != null) { + if (children != null) { int pos = 0; - while (pos < chidren.size()) { - List colCh = chidren.get(pos).getCols(); + while (pos < children.size()) { + List colCh = children.get(pos).getCols(); colList = Utilities.mergeUniqElems(colList, colCh); pos++; } @@ -182,8 +184,8 @@ public String getExprString() { @Override public ExprNodeDesc clone() { - List cloneCh = new ArrayList(chidren.size()); - for (ExprNodeDesc ch : chidren) { + List cloneCh = new ArrayList(children.size()); + for (ExprNodeDesc ch : children) { cloneCh.add(ch.clone()); } ExprNodeGenericFuncDesc clone = new ExprNodeGenericFuncDesc(typeInfo, @@ -192,6 +194,31 @@ public ExprNodeDesc clone() { } /** + * Check the two types are allowed to be cast. + * + * @throws UDFArgumentException + */ + private static void checkTypeSafety(TypeInfo oiTypeInfo0, TypeInfo oiTypeInfo1) + throws UDFArgumentException { + SessionState ss = SessionState.get(); + Configuration conf = (ss != null) ? ss.getConf() : new Configuration(); + LogHelper console = new LogHelper(LOG); + + // For now, if a bigint is going to be cast to a double throw an error or warning + if ((oiTypeInfo0.equals(TypeInfoFactory.stringTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.longTypeInfo)) || + (oiTypeInfo0.equals(TypeInfoFactory.longTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.stringTypeInfo))) { + String error = StrictChecks.checkTypeSafety(conf); + if (error != null) throw new UDFArgumentException(error); + console.printError("WARNING: Comparing a bigint and a string may result in a loss of precision."); + } else if ((oiTypeInfo0.equals(TypeInfoFactory.doubleTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.longTypeInfo)) || + (oiTypeInfo0.equals(TypeInfoFactory.longTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.doubleTypeInfo))) { + String error = StrictChecks.checkTypeSafety(conf); + if (error != null) throw new UDFArgumentException(error); + console.printError("WARNING: Comparing a bigint and a double may result in a loss of precision."); + } + } + + /** * Create a ExprNodeGenericFuncDesc based on the genericUDFClass and the * children parameters. If the function has an explicit name, the * newInstance method should be passed the function name in the funcText @@ -207,30 +234,20 @@ public static ExprNodeGenericFuncDesc newInstance(GenericUDF genericUDF, childrenOIs[i] = children.get(i).getWritableObjectInspector(); } - // Check if a bigint is implicitely cast to a double as part of a comparison + // Check if a bigint is implicitly cast to a double as part of a comparison // Perform the check here instead of in GenericUDFBaseCompare to guarantee it is only run once per operator if (genericUDF instanceof GenericUDFBaseCompare && children.size() == 2) { - + checkTypeSafety(children.get(0).getTypeInfo(), children.get(1).getTypeInfo()); + } else if (genericUDF instanceof GenericUDFIn) { TypeInfo oiTypeInfo0 = children.get(0).getTypeInfo(); - TypeInfo oiTypeInfo1 = children.get(1).getTypeInfo(); - - SessionState ss = SessionState.get(); - Configuration conf = (ss != null) ? ss.getConf() : new Configuration(); - - LogHelper console = new LogHelper(LOG); - - // For now, if a bigint is going to be cast to a double throw an error or warning - if ((oiTypeInfo0.equals(TypeInfoFactory.stringTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.longTypeInfo)) || - (oiTypeInfo0.equals(TypeInfoFactory.longTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.stringTypeInfo))) { - String error = StrictChecks.checkTypeSafety(conf); - if (error != null) throw new UDFArgumentException(error); - console.printError("WARNING: Comparing a bigint and a string may result in a loss of precision."); - } else if ((oiTypeInfo0.equals(TypeInfoFactory.doubleTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.longTypeInfo)) || - (oiTypeInfo0.equals(TypeInfoFactory.longTypeInfo) && oiTypeInfo1.equals(TypeInfoFactory.doubleTypeInfo))) { - String error = StrictChecks.checkTypeSafety(conf); - if (error != null) throw new UDFArgumentException(error); - console.printError("WARNING: Comparing a bigint and a double may result in a loss of precision."); + for (int i = 1; i < children.size(); i++) { + checkTypeSafety(oiTypeInfo0, children.get(i).getTypeInfo()); } + } else if (genericUDF instanceof GenericUDFBetween) { + // First argument for BETWEEN is a boolean value, skipping. + TypeInfo oiTypeInfo0 = children.get(1).getTypeInfo(); + checkTypeSafety(oiTypeInfo0, children.get(2).getTypeInfo()); + checkTypeSafety(oiTypeInfo0, children.get(3).getTypeInfo()); } ObjectInspector oi = genericUDF.initializeAndFoldConstants(childrenOIs); @@ -298,12 +315,12 @@ public boolean isSame(Object o) { } } - if (chidren.size() != dest.getChildren().size()) { + if (children.size() != dest.getChildren().size()) { return false; } - for (int pos = 0; pos < chidren.size(); pos++) { - if (!chidren.get(pos).isSame(dest.getChildren().get(pos))) { + for (int pos = 0; pos < children.size(); pos++) { + if (!children.get(pos).isSame(dest.getChildren().get(pos))) { return false; } } @@ -316,7 +333,7 @@ public int hashCode() { int superHashCode = super.hashCode(); HashCodeBuilder builder = new HashCodeBuilder(); builder.appendSuper(superHashCode); - builder.append(chidren); + builder.append(children); return builder.toHashCode(); } diff --git ql/src/test/queries/clientnegative/compare_double_bigint_between.q ql/src/test/queries/clientnegative/compare_double_bigint_between.q new file mode 100644 index 0000000..200cd57 --- /dev/null +++ ql/src/test/queries/clientnegative/compare_double_bigint_between.q @@ -0,0 +1,7 @@ +set hive.strict.checks.bucketing=false; + +set hive.mapred.mode=strict; + +-- This should fail until we fix the issue with precision when casting a bigint to a double + +select * from src where cast(1 as bigint) BETWEEN cast(1 as double) and cast(2 as double) limit 10; diff --git ql/src/test/queries/clientnegative/compare_double_bigint_in.q ql/src/test/queries/clientnegative/compare_double_bigint_in.q new file mode 100644 index 0000000..d67da34 --- /dev/null +++ ql/src/test/queries/clientnegative/compare_double_bigint_in.q @@ -0,0 +1,7 @@ +set hive.strict.checks.bucketing=false; + +set hive.mapred.mode=strict; + +-- This should fail until we fix the issue with precision when casting a bigint to a double + +select * from src where cast(1 as bigint) IN (cast(1 as double), cast(2 as double)) limit 10; diff --git ql/src/test/queries/clientnegative/compare_string_bigint_between.q ql/src/test/queries/clientnegative/compare_string_bigint_between.q new file mode 100644 index 0000000..0c8e42e --- /dev/null +++ ql/src/test/queries/clientnegative/compare_string_bigint_between.q @@ -0,0 +1,7 @@ +set hive.strict.checks.bucketing=false; + +set hive.mapred.mode=strict; + +--This should fail until we fix the issue with precision when casting a bigint to a double + +select * from src where cast(1 as bigint) BETWEEN '1' and '2' limit 10; diff --git ql/src/test/queries/clientnegative/compare_string_bigint_in.q ql/src/test/queries/clientnegative/compare_string_bigint_in.q new file mode 100644 index 0000000..9b7de87 --- /dev/null +++ ql/src/test/queries/clientnegative/compare_string_bigint_in.q @@ -0,0 +1,7 @@ +set hive.strict.checks.bucketing=false; + +set hive.mapred.mode=strict; + +--This should fail until we fix the issue with precision when casting a bigint to a double + +select * from src where cast(1 as bigint) IN ('1', '2') limit 10; diff --git ql/src/test/results/clientnegative/compare_double_bigint_between.q.out ql/src/test/results/clientnegative/compare_double_bigint_between.q.out new file mode 100644 index 0000000..00525ce --- /dev/null +++ ql/src/test/results/clientnegative/compare_double_bigint_between.q.out @@ -0,0 +1 @@ +FAILED: SemanticException Line 0:-1 Wrong arguments '2': Unsafe compares between different types are disabled for safety reasons. If you know what you are doing, please sethive.strict.checks.type.safety to false and that hive.mapred.mode is not set to 'strict' to proceed. Note that if you may get errors or incorrect results if you make a mistake while using some of the unsafe features. diff --git ql/src/test/results/clientnegative/compare_double_bigint_in.q.out ql/src/test/results/clientnegative/compare_double_bigint_in.q.out new file mode 100644 index 0000000..00525ce --- /dev/null +++ ql/src/test/results/clientnegative/compare_double_bigint_in.q.out @@ -0,0 +1 @@ +FAILED: SemanticException Line 0:-1 Wrong arguments '2': Unsafe compares between different types are disabled for safety reasons. If you know what you are doing, please sethive.strict.checks.type.safety to false and that hive.mapred.mode is not set to 'strict' to proceed. Note that if you may get errors or incorrect results if you make a mistake while using some of the unsafe features. diff --git ql/src/test/results/clientnegative/compare_string_bigint_between.q.out ql/src/test/results/clientnegative/compare_string_bigint_between.q.out new file mode 100644 index 0000000..b14ffa4 --- /dev/null +++ ql/src/test/results/clientnegative/compare_string_bigint_between.q.out @@ -0,0 +1 @@ +FAILED: SemanticException Line 0:-1 Wrong arguments ''2'': Unsafe compares between different types are disabled for safety reasons. If you know what you are doing, please sethive.strict.checks.type.safety to false and that hive.mapred.mode is not set to 'strict' to proceed. Note that if you may get errors or incorrect results if you make a mistake while using some of the unsafe features. diff --git ql/src/test/results/clientnegative/compare_string_bigint_in.q.out ql/src/test/results/clientnegative/compare_string_bigint_in.q.out new file mode 100644 index 0000000..b14ffa4 --- /dev/null +++ ql/src/test/results/clientnegative/compare_string_bigint_in.q.out @@ -0,0 +1 @@ +FAILED: SemanticException Line 0:-1 Wrong arguments ''2'': Unsafe compares between different types are disabled for safety reasons. If you know what you are doing, please sethive.strict.checks.type.safety to false and that hive.mapred.mode is not set to 'strict' to proceed. Note that if you may get errors or incorrect results if you make a mistake while using some of the unsafe features.