----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2078/#review2142 -----------------------------------------------------------
Everything looks great, this will be really helpful! Just few inline comments: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/ConstantFoldableGenericUDF.java <https://reviews.apache.org/r/2078/#comment4959> Should code here make sure this value is actually constant? I.e. create a deep copy of maps/lists, with Collections.unmodifiableMap/List wrapper around them? (or maybe it is better/cleaner to make StandardConstantList/MapObjectInspector create such a copy inside, and make code here return inspector.getWritableConstantValue instead. That will make this safe for all usages below, like getConstantObjectInspector. This will always be one-time only performance hit, since we are dealing with constant OI's, so it shouldn't matter) ql/src/java/org/apache/hadoop/hive/ql/udf/generic/ConstantFoldableGenericUDF.java <https://reviews.apache.org/r/2078/#comment4958> This might be useful as a top-level class? - Igor On 2011-09-27 23:03:09, Jonathan Chang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2078/ > ----------------------------------------------------------- > > (Updated 2011-09-27 23:03:09) > > > Review request for hive, Carl Steinbach, John Sichi, and Igor Kabiljo. > > > Summary > ------- > > ConstantObjectInspectors were introduced in a previous patch so that UDF's > could have access to constant values at initialize time. This diff adds the > following improvements: > > 1.) Extends this functionality beyond primitive types to List/Map types. > 2.) Fixes a bug in ExprNodeGenericFuncDesc where the constant value would not > propagate beyond one level of evaluation. > 3.) Adds a helper class to make functions constant-aware. By inheriting from > this class they return constant OIs if all their arguments are constant, and > they cache the result so that computation is only done during initialize(). > Some functions such as ARRAY/IF/MAP have been migrated. > > > This addresses bug HIVE-2470. > https://issues.apache.org/jira/browse/HIVE-2470 > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java > 7da5d6a > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/ConstantFoldableGenericUDF.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFArray.java > 1cf97c8 > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFArrayContains.java > d60dc86 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFIf.java 9a6a7ff > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMap.java > 3b71c48 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMapKeys.java > b384717 > ql/src/test/queries/clientpositive/constant_prop.q PRE-CREATION > ql/src/test/results/clientpositive/constant_prop.q.out PRE-CREATION > > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorFactory.java > a4689fb > > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java > 2c1a7e0 > > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardConstantListObjectInspector.java > PRE-CREATION > > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardConstantMapObjectInspector.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/2078/diff > > > Testing > ------- > > ant test > > > Thanks, > > Jonathan > >
