Repository navigation
HIVE-30159: SharedWorkOptimizer should not merge when it leaves a work without a ReduceSink/FileSink - #6916
HIVE-30159: SharedWorkOptimizer should not merge when it leaves a work without a ReduceSink/FileSink#6916maheshrajus wants to merge 1 commit into
Conversation
…k without a ReduceSink/FileSink
|
thomasrebele
left a comment
There was a problem hiding this comment.
The Java test looks good. Could the q file test be simplified (any tables, columns, statements that can be removed)?
| boolean hasSink = false; | ||
| for (Operator<?> workOp : findWorkOperators(optimizerCache, parent)) { | ||
| if (!discardableInputOps.contains(workOp) && !discardableOps.contains(workOp) | ||
| && (workOp instanceof ReduceSinkOperator || workOp instanceof FileSinkOperator)) { |
There was a problem hiding this comment.
Shouldn't org.apache.hadoop.hive.ql.exec.TerminalOperator be used instead?
| continue; | ||
| } | ||
| boolean hasSink = false; | ||
| for (Operator<?> workOp : findWorkOperators(optimizerCache, parent)) { |
There was a problem hiding this comment.
findWorkOperators searches in both directions (parents and children). As far as I understand the algorithm, we only want to search in one direction here?
| * feeds it, inside the upstream vertex. Its reduce-side operators (e.g. a mergepartial GroupBy | ||
| * reading KEY._colN) are then initialised against map-side rows and fail with | ||
| * "cannot find field key from [0:_col0, ...]". | ||
| */ |
There was a problem hiding this comment.
I find the description a bit difficult to understand. Would it be possible to rephrase it illustrating the problem with an example? The example could include the GroupBy, if adding it does not make the example too complicated.
|
@thomasrebele Thank you for the review. I will look into your comments and address them shortly. |
What changes were proposed in this pull request?
is skipped if deleting the duplicate's operators would leave a work with no ReduceSink/FileSink.
Why are the changes needed?
When two identical table scans are merged, the operators that only fed the duplicate scan are deleted.
Sometimes one of them is the only ReduceSink of a reducer, and all that reducer has left is a dynamic
partition pruning (DPP) event branch. GenTezWork only starts a new vertex at a sink, so that reducer never
gets its own vertex. Its mergepartial GroupBy ends up inside the upstream Map vertex, and the query fails at
runtime with: cannot find field key from [0:_col0, 1:_col1, 2:_col2]
Does this PR introduce any user-facing change?
No
How was this patch tested?
Newly added UT & qtests