diff --git ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java index 4246d68..79aff15 100644 --- ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java +++ ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java @@ -84,7 +84,8 @@ INVALID_PATH(10027, "Invalid path"), ILLEGAL_PATH(10028, "Path is not legal"), INVALID_NUMERICAL_CONSTANT(10029, "Invalid numerical constant"), - INVALID_ARRAYINDEX_CONSTANT(10030, "Non-constant expressions for array indexes not supported"), + INVALID_ARRAYINDEX_TYPE(10030, + "Not proper type for index of ARRAY. Currently, only integer type is supported"), INVALID_MAPINDEX_CONSTANT(10031, "Non-constant expression for map indexes not supported"), INVALID_MAPINDEX_TYPE(10032, "MAP key type does not match index expression type"), NON_COLLECTION_TYPE(10033, "[] not valid on non-collection types"), diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java index c503bbb..80b7420 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java @@ -903,15 +903,15 @@ public static TypeInfo getCommonClassForUnionAll(TypeInfo a, TypeInfo b) { (PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b,PrimitiveCategory.STRING); } - if (FunctionRegistry.implicitConvertable(a, b)) { + if (FunctionRegistry.implicitConvertible(a, b)) { return getTypeInfoForPrimitiveCategory((PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b, pcB); } - if (FunctionRegistry.implicitConvertable(b, a)) { + if (FunctionRegistry.implicitConvertible(b, a)) { return getTypeInfoForPrimitiveCategory((PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b, pcA); } for (PrimitiveCategory t : numericTypeList) { - if (FunctionRegistry.implicitConvertable(pcA, t) - && FunctionRegistry.implicitConvertable(pcB, t)) { + if (FunctionRegistry.implicitConvertible(pcA, t) + && FunctionRegistry.implicitConvertible(pcB, t)) { return getTypeInfoForPrimitiveCategory((PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b, t); } } @@ -955,8 +955,8 @@ public static TypeInfo getCommonClassForComparison(TypeInfo a, TypeInfo b) { } for (PrimitiveCategory t : numericTypeList) { - if (FunctionRegistry.implicitConvertable(pcA, t) - && FunctionRegistry.implicitConvertable(pcB, t)) { + if (FunctionRegistry.implicitConvertible(pcA, t) + && FunctionRegistry.implicitConvertible(pcB, t)) { return getTypeInfoForPrimitiveCategory((PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b, t); } } @@ -1007,7 +1007,7 @@ public static TypeInfo getCommonClass(TypeInfo a, TypeInfo b) { return getTypeInfoForPrimitiveCategory((PrimitiveTypeInfo)a, (PrimitiveTypeInfo)b, commonCat); } - public static boolean implicitConvertable(PrimitiveCategory from, PrimitiveCategory to) { + public static boolean implicitConvertible(PrimitiveCategory from, PrimitiveCategory to) { if (from == to) { return true; } @@ -1058,7 +1058,7 @@ public static boolean implicitConvertable(PrimitiveCategory from, PrimitiveCateg * Returns whether it is possible to implicitly convert an object of Class * from to Class to. */ - public static boolean implicitConvertable(TypeInfo from, TypeInfo to) { + public static boolean implicitConvertible(TypeInfo from, TypeInfo to) { if (from.equals(to)) { return true; } @@ -1067,9 +1067,9 @@ public static boolean implicitConvertable(TypeInfo from, TypeInfo to) { // 2 TypeInfos from the same qualified type (varchar, decimal) should still be // seen as equivalent. if (from.getCategory() == Category.PRIMITIVE && to.getCategory() == Category.PRIMITIVE) { - return implicitConvertable( - ((PrimitiveTypeInfo)from).getPrimitiveCategory(), - ((PrimitiveTypeInfo)to).getPrimitiveCategory()); + return implicitConvertible( + ((PrimitiveTypeInfo) from).getPrimitiveCategory(), + ((PrimitiveTypeInfo) to).getPrimitiveCategory()); } return false; } @@ -1305,7 +1305,7 @@ public static int matchCost(TypeInfo argumentPassed, // but there is a conversion cost. return 1; } - if (!exact && implicitConvertable(argumentPassed, argumentAccepted)) { + if (!exact && implicitConvertible(argumentPassed, argumentAccepted)) { return 1; } diff --git ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java index e44f5ae..5c5589a 100644 --- ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java +++ ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java @@ -751,12 +751,10 @@ static ExprNodeDesc getXpathOrFuncExprNodeDesc(ASTNode expr, if (myt.getCategory() == Category.LIST) { // Only allow integer index for now - if (!(children.get(1) instanceof ExprNodeConstantDesc) - || !(((ExprNodeConstantDesc) children.get(1)).getTypeInfo() - .equals(TypeInfoFactory.intTypeInfo))) { + if (!FunctionRegistry.implicitConvertible(children.get(1).getTypeInfo(), + TypeInfoFactory.intTypeInfo)) { throw new SemanticException(SemanticAnalyzer.generateErrorMessage( - expr, - ErrorMsg.INVALID_ARRAYINDEX_CONSTANT.getMsg())); + expr, ErrorMsg.INVALID_ARRAYINDEX_TYPE.getMsg())); } // Calculate TypeInfo @@ -764,14 +762,8 @@ static ExprNodeDesc getXpathOrFuncExprNodeDesc(ASTNode expr, desc = new ExprNodeGenericFuncDesc(t, FunctionRegistry .getGenericUDFForIndex(), children); } else if (myt.getCategory() == Category.MAP) { - // Only allow constant map key for now - if (!(children.get(1) instanceof ExprNodeConstantDesc)) { - throw new SemanticException(SemanticAnalyzer.generateErrorMessage( - expr, - ErrorMsg.INVALID_MAPINDEX_CONSTANT.getMsg())); - } - if (!(((ExprNodeConstantDesc) children.get(1)).getTypeInfo() - .equals(((MapTypeInfo) myt).getMapKeyTypeInfo()))) { + if (!FunctionRegistry.implicitConvertible(children.get(1).getTypeInfo(), + ((MapTypeInfo) myt).getMapKeyTypeInfo())) { throw new SemanticException(ErrorMsg.INVALID_MAPINDEX_TYPE .getMsg(expr)); } diff --git ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFIndex.java ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFIndex.java index 5911f2c..bdb2361 100644 --- ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFIndex.java +++ ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFIndex.java @@ -26,9 +26,11 @@ import org.apache.hadoop.hive.serde2.objectinspector.ListObjectInspector; import org.apache.hadoop.hive.serde2.objectinspector.MapObjectInspector; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector; +import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorConverters; import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector.Category; -import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorUtils; +import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory; +import org.apache.hadoop.io.IntWritable; /** * GenericUDFIndex. @@ -36,11 +38,10 @@ */ @Description(name = "index", value = "_FUNC_(a, n) - Returns the n-th element of a ") public class GenericUDFIndex extends GenericUDF { + private transient MapObjectInspector mapOI; - private boolean mapKeyPreferWritable; private transient ListObjectInspector listOI; - private transient PrimitiveObjectInspector indexOI; - private transient ObjectInspector returnOI; + private transient ObjectInspectorConverters.Converter converter; @Override public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumentException { @@ -66,21 +67,22 @@ public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumen } // index has to be a primitive - if (arguments[1] instanceof PrimitiveObjectInspector) { - indexOI = (PrimitiveObjectInspector) arguments[1]; - } else { + if (!(arguments[1] instanceof PrimitiveObjectInspector)) { throw new UDFArgumentTypeException(1, "Primitive Type is expected but " + arguments[1].getTypeName() + "\" is found"); } - + PrimitiveObjectInspector inputOI = (PrimitiveObjectInspector) arguments[1]; + ObjectInspector returnOI; + ObjectInspector indexOI; if (mapOI != null) { + indexOI = ObjectInspectorConverters.getConvertedOI( + inputOI, mapOI.getMapKeyObjectInspector()); returnOI = mapOI.getMapValueObjectInspector(); - ObjectInspector keyOI = mapOI.getMapKeyObjectInspector(); - mapKeyPreferWritable = ((PrimitiveObjectInspector) keyOI) - .preferWritable(); } else { + indexOI = PrimitiveObjectInspectorFactory.writableIntObjectInspector; returnOI = listOI.getListElementObjectInspector(); } + converter = ObjectInspectorConverters.getConverter(inputOI, indexOI); return returnOI; } @@ -88,35 +90,16 @@ public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumen @Override public Object evaluate(DeferredObject[] arguments) throws HiveException { assert (arguments.length == 2); - Object main = arguments[0].get(); Object index = arguments[1].get(); + Object indexObject = converter.convert(index); + if (indexObject == null) { + return null; + } if (mapOI != null) { - - Object indexObject; - if (mapKeyPreferWritable) { - indexObject = indexOI.getPrimitiveWritableObject(index); - } else { - indexObject = indexOI.getPrimitiveJavaObject(index); - } - return mapOI.getMapValueElement(main, indexObject); - - } else { - - assert (listOI != null); - int intIndex = 0; - try { - intIndex = PrimitiveObjectInspectorUtils.getInt(index, indexOI); - } catch (NullPointerException e) { - // If index is null, we should return null. - return null; - } catch (NumberFormatException e) { - // If index is not a number, we should return null. - return null; - } - return listOI.getListElement(main, intIndex); - + return mapOI.getMapValueElement(arguments[0].get(), indexObject); } + return listOI.getListElement(arguments[0].get(), ((IntWritable)indexObject).get()); } @Override diff --git ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java index d7d2a34..f2e8113 100644 --- ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java +++ ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java @@ -20,7 +20,6 @@ import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -80,7 +79,7 @@ protected void setUp() { } private void implicit(TypeInfo a, TypeInfo b, boolean convertible) { - assertEquals(convertible, FunctionRegistry.implicitConvertable(a,b)); + assertEquals(convertible, FunctionRegistry.implicitConvertible(a, b)); } public void testImplicitConversion() { diff --git ql/src/test/queries/clientpositive/array_map_access_nonconstant.q ql/src/test/queries/clientpositive/array_map_access_nonconstant.q new file mode 100644 index 0000000..49c1f54 --- /dev/null +++ ql/src/test/queries/clientpositive/array_map_access_nonconstant.q @@ -0,0 +1,15 @@ +set hive.fetch.task.conversion=more; + +create table array_table (array array, index int ); +insert into table array_table select array('first', 'second', 'third'), key%3 from src tablesample (4 rows); + +explain +select index, array[index] from array_table; +select index, array[index] from array_table; + +create table map_table (data map, key int ); +insert into table map_table select map('1','one','2','two','3','three'), cast((key%3+1) as int) from src tablesample (4 rows); + +explain +select key, data[key] from map_table; +select key, data[key] from map_table; diff --git ql/src/test/queries/negative/invalid_list_index.q ql/src/test/queries/negative/invalid_list_index.q deleted file mode 100644 index c40f079..0000000 --- ql/src/test/queries/negative/invalid_list_index.q +++ /dev/null @@ -1,2 +0,0 @@ -FROM src_thrift -INSERT OVERWRITE TABLE dest1 SELECT src_thrift.lint[0], src_thrift.lstring['abc'] diff --git ql/src/test/queries/negative/invalid_list_index2.q ql/src/test/queries/negative/invalid_list_index2.q deleted file mode 100644 index 99d0b3d..0000000 --- ql/src/test/queries/negative/invalid_list_index2.q +++ /dev/null @@ -1,2 +0,0 @@ -FROM src_thrift -INSERT OVERWRITE TABLE dest1 SELECT src_thrift.lint[0], src_thrift.lstring[1 + 2] diff --git ql/src/test/queries/negative/invalid_map_index2.q ql/src/test/queries/negative/invalid_map_index2.q deleted file mode 100644 index 5828f07..0000000 --- ql/src/test/queries/negative/invalid_map_index2.q +++ /dev/null @@ -1,2 +0,0 @@ -FROM src_thrift -INSERT OVERWRITE TABLE dest1 SELECT src_thrift.lint[0], src_thrift.mstringstring[concat('abc', 'abc')] diff --git ql/src/test/results/clientpositive/array_map_access_nonconstant.q.out ql/src/test/results/clientpositive/array_map_access_nonconstant.q.out new file mode 100644 index 0000000..d8b88c6 --- /dev/null +++ ql/src/test/results/clientpositive/array_map_access_nonconstant.q.out @@ -0,0 +1,106 @@ +PREHOOK: query: create table array_table (array array, index int ) +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@array_table +POSTHOOK: query: create table array_table (array array, index int ) +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@array_table +PREHOOK: query: insert into table array_table select array('first', 'second', 'third'), key%3 from src tablesample (4 rows) +PREHOOK: type: QUERY +PREHOOK: Input: default@src +PREHOOK: Output: default@array_table +POSTHOOK: query: insert into table array_table select array('first', 'second', 'third'), key%3 from src tablesample (4 rows) +POSTHOOK: type: QUERY +POSTHOOK: Input: default@src +POSTHOOK: Output: default@array_table +POSTHOOK: Lineage: array_table.array EXPRESSION [] +POSTHOOK: Lineage: array_table.index EXPRESSION [(src)src.FieldSchema(name:key, type:string, comment:default), ] +PREHOOK: query: explain +select index, array[index] from array_table +PREHOOK: type: QUERY +POSTHOOK: query: explain +select index, array[index] from array_table +POSTHOOK: type: QUERY +STAGE DEPENDENCIES: + Stage-0 is a root stage + +STAGE PLANS: + Stage: Stage-0 + Fetch Operator + limit: -1 + Processor Tree: + TableScan + alias: array_table + Statistics: Num rows: 4 Data size: 80 Basic stats: COMPLETE Column stats: NONE + Select Operator + expressions: index (type: int), array[index] (type: string) + outputColumnNames: _col0, _col1 + Statistics: Num rows: 4 Data size: 80 Basic stats: COMPLETE Column stats: NONE + ListSink + +PREHOOK: query: select index, array[index] from array_table +PREHOOK: type: QUERY +PREHOOK: Input: default@array_table +#### A masked pattern was here #### +POSTHOOK: query: select index, array[index] from array_table +POSTHOOK: type: QUERY +POSTHOOK: Input: default@array_table +#### A masked pattern was here #### +1 second +2 third +2 third +0 first +PREHOOK: query: create table map_table (data map, key int ) +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@map_table +POSTHOOK: query: create table map_table (data map, key int ) +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@map_table +PREHOOK: query: insert into table map_table select map('1','one','2','two','3','three'), cast((key%3+1) as int) from src tablesample (4 rows) +PREHOOK: type: QUERY +PREHOOK: Input: default@src +PREHOOK: Output: default@map_table +POSTHOOK: query: insert into table map_table select map('1','one','2','two','3','three'), cast((key%3+1) as int) from src tablesample (4 rows) +POSTHOOK: type: QUERY +POSTHOOK: Input: default@src +POSTHOOK: Output: default@map_table +POSTHOOK: Lineage: map_table.data EXPRESSION [] +POSTHOOK: Lineage: map_table.key EXPRESSION [(src)src.FieldSchema(name:key, type:string, comment:default), ] +PREHOOK: query: explain +select key, data[key] from map_table +PREHOOK: type: QUERY +POSTHOOK: query: explain +select key, data[key] from map_table +POSTHOOK: type: QUERY +STAGE DEPENDENCIES: + Stage-0 is a root stage + +STAGE PLANS: + Stage: Stage-0 + Fetch Operator + limit: -1 + Processor Tree: + TableScan + alias: map_table + Statistics: Num rows: 4 Data size: 84 Basic stats: COMPLETE Column stats: NONE + Select Operator + expressions: key (type: int), data[key] (type: string) + outputColumnNames: _col0, _col1 + Statistics: Num rows: 4 Data size: 84 Basic stats: COMPLETE Column stats: NONE + ListSink + +PREHOOK: query: select key, data[key] from map_table +PREHOOK: type: QUERY +PREHOOK: Input: default@map_table +#### A masked pattern was here #### +POSTHOOK: query: select key, data[key] from map_table +POSTHOOK: type: QUERY +POSTHOOK: Input: default@map_table +#### A masked pattern was here #### +2 two +3 three +3 three +1 one diff --git ql/src/test/results/compiler/errors/invalid_list_index.q.out ql/src/test/results/compiler/errors/invalid_list_index.q.out deleted file mode 100644 index a4179cd..0000000 --- ql/src/test/results/compiler/errors/invalid_list_index.q.out +++ /dev/null @@ -1,2 +0,0 @@ -Semantic Exception: -2:74 Non-constant expressions for array indexes not supported. Error encountered near token ''abc'' \ No newline at end of file diff --git ql/src/test/results/compiler/errors/invalid_list_index2.q.out ql/src/test/results/compiler/errors/invalid_list_index2.q.out deleted file mode 100644 index aaa9455..0000000 --- ql/src/test/results/compiler/errors/invalid_list_index2.q.out +++ /dev/null @@ -1,2 +0,0 @@ -Semantic Exception: -2:74 Non-constant expressions for array indexes not supported. Error encountered near token '2' \ No newline at end of file diff --git ql/src/test/results/compiler/errors/invalid_map_index2.q.out ql/src/test/results/compiler/errors/invalid_map_index2.q.out deleted file mode 100644 index edc9bda..0000000 --- ql/src/test/results/compiler/errors/invalid_map_index2.q.out +++ /dev/null @@ -1,2 +0,0 @@ -Semantic Exception: -2:80 Non-constant expression for map indexes not supported. Error encountered near token ''abc'' \ No newline at end of file