> On 2011-10-03 08:31:46, Igor Kabiljo wrote: > > Nice, this is even better then what I thought. > > > > This now supports all OIs, except for struct and unions, right? No reason > > not to add structs, should we add both? > > Also, should we change so that every VoidObjectInspector is constant (not > > sure if that would complicate things more, in which case - no)? > > > > I still think it would be preferable that constant OI guarantees that > > objects it returns are constant - if somebody changes returned value, next > > values it returns shouldn't be affected. But this doesn't matter that much. > > > > Jonathan Chang wrote: > * I agree that structs and unions would be good to have at some point but > it turns out they are much harder to implement. In particular, they contain > references to constituent OIs and do not have natural Java/Writable > equivalents. Thus, the way the constant value should be stored is not > obvious and you have weird interactions where some field OI maybe a > constantOI but not the whole thing, etc. This is complex enough that I think > it's worth deferring to another diff > > * I will convert void OI to be constant. > > * I'm not sure that constant OI guarantee that its objects are constant; > it might imply unnecessary copies and currently none of the rest of the UDF > architecture functions this way (most things passed around are writable and > it's up to consumers not to assume ownership).
- I didn't know it was like that (for structs and unions), I thought it was straightforward. so definitely for next diff, not this one - constant OI guarantee will not incur much overhead - since all those are executed only once - since they are constant. but I agree, the rest of architecture functions with that assumption, so it is better to be consistent (and later structs - Object[] - cannot be made constant) - Igor ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2078/#review2259 ----------------------------------------------------------- On 2011-10-03 01:36:16, Jonathan Chang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2078/ > ----------------------------------------------------------- > > (Updated 2011-10-03 01:36:16) > > > 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/exec/ExprNodeGenericFuncEvaluator.java > ebc236d > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeGenericFuncDesc.java > 7da5d6a > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java 570e408 > 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 > >
