HIVE-28196: Preserve column stats when applying UDF upper/lower - #5191
Conversation
okumin
left a comment
okumin
left a comment
There was a problem hiding this comment.
Looks good, but we have to update some .q.out of failing test cases.
| GenericUDF udf = engfd.getGenericUDF(); | ||
| Class udfClass = | ||
| (udf instanceof GenericUDFBridge) ? ((GenericUDFBridge)udf).getUdfClass() : udf.getClass(); | ||
| return udfClass == GenericUDFLower.class || udfClass == GenericUDFUpper.class; |
There was a problem hiding this comment.
I like this PR. I have also observed that an estimation was wrong, and Map Join was disabled.
We implement some estimation APIs on the UDF side. I'll share examples with you. This is not a strong suggestion, and the current implementation seems to be a good starting point. We may finally need a pluggable I/F to empower custom UDFs more.
There was a problem hiding this comment.
Thanks for sharing the examples. I moved the estimation logic to UDF side by using StatEstimatorProvider interface.
There was a problem hiding this comment.
I didn't know the interface and looks super nice!
asf-ci-hive
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
64502e1 to
2178f59
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
okumin
left a comment
okumin
left a comment
There was a problem hiding this comment.
mv_iceberg_orc5 failed, but the PR looks mostly good to me
|
asf-ci-hive
added
tests pending
tests unstable
tests passed
and removed
tests unstable
tests pending
labels
2178f59 to
ed18800
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
tests unstable
labels
|
5 participants
What changes were proposed in this pull request?
This patch preserves column stats when applying UDF
upper/loweron a column.Why are the changes needed?
Current Hive re-estimates column stats when it encounters UDF. UDF
upperandlowerdo not change column stats, but current Hive resetsavgColLentohive.stats.max.variable.length. Keeping the input data's column stats, includingavgColLen, will give more accurate stats to later query optimizers and make a better query plan.Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
Added a qfile for testing column stats computation on upper/lower UDF. Current Hive chooses MergeJoin due to column stats over-estimation, while this patch chooses MapJoin.