HIVE-28050: Disable Incremental non aggregated materialized view rebuild in presence of delete operations #5053
Conversation
kasakrisz
force-pushed
the
HIVE-28050-master-disable-mv-inc
branch
from
92b0637
to
0e4f6da
Compare
asf-ci-hive
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
| @@ -480,8 +460,6 @@ protected ASTNode fixUpAfterCbo(ASTNode originalAst, ASTNode newAst, CalcitePlan | |||
| fixUpASTAggregateInsertDeleteIncrementalRebuild(fixedAST, getMaterializedViewASTBuilder()); | |||
| return fixedAST; | |||
| case JOIN_INSERT_DELETE_REBUILD: | |||
| fixUpASTJoinInsertDeleteIncrementalRebuild(fixedAST, getMaterializedViewASTBuilder()); | |||
| return fixedAST; | |||
There was a problem hiding this comment.
This will end up throwing the UnsupportedOperationException. Don't we want to fall back to the full rebuild ?
There was a problem hiding this comment.
At this point CBO plan is already transformed to AST hence we can not fall back. However JOIN_INSERT_DELETE_REBUILD is never set in the code any more.
There was a problem hiding this comment.
Ok, just for my reference, for the non-groupby queries that this patch is addressing, could you point to where the fall back to the full rebuild (using insert-overwrite) happens ?
There was a problem hiding this comment.
Since JOIN_INSERT_DELETE_REBUILD is not used anymore why not remove it altogether?
| @@ -322,8 +318,7 @@ private RelNode applyRecordIncrementalRebuildPlan( | |||
| } | |||
| return applyAggregateInsertDeleteIncremental(basePlan, mdProvider, executorProvider); | |||
| } else { | |||
| return applyJoinInsertDeleteIncremental( | |||
| basePlan, mdProvider, executorProvider, optCluster, calcitePreMVRewritingPlan); | |||
| return calcitePreMVRewritingPlan; | |||
There was a problem hiding this comment.
@amansinha100
This is the point where we fall back to full rebuild.
| @@ -480,8 +460,6 @@ protected ASTNode fixUpAfterCbo(ASTNode originalAst, ASTNode newAst, CalcitePlan | |||
| fixUpASTAggregateInsertDeleteIncrementalRebuild(fixedAST, getMaterializedViewASTBuilder()); | |||
| return fixedAST; | |||
| case JOIN_INSERT_DELETE_REBUILD: | |||
| fixUpASTJoinInsertDeleteIncrementalRebuild(fixedAST, getMaterializedViewASTBuilder()); | |||
| return fixedAST; | |||
There was a problem hiding this comment.
Since JOIN_INSERT_DELETE_REBUILD is not used anymore why not remove it altogether?
| (1, 'alfred', 10.30, 2), | ||
| (1, 'alfred', 10.30, 2), |
There was a problem hiding this comment.
Instead of modifying existing tests to add duplicates in order to trigger the problem I think it would be better to add a new minimal test like the one you posted last under the JIRA ticket.
There was a problem hiding this comment.
I added the 2nd example from the jira as a new q test but I also kept the change of this one because we ususally face extra challenges in case of MVs with Join.
…ild in presence of delete operations
kasakrisz
force-pushed
the
HIVE-28050-master-disable-mv-inc
branch
from
0e4f6da
to
6085bc0
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
tests unstable
labels
|
What changes were proposed in this pull request?
Remove code of incremental rebuild of materialized views without aggregate in presence of delete operations
Why are the changes needed?
The current implementations leads to data correctness issues when a source table does not have a unique column.
Does this PR introduce any user-facing change?
No.
Is the change a dependency upgrade?
No.
How was this patch tested?