HIVE-26160: Materialized View rewrite does not check tables scanned in sub-query expressions #3229
Conversation
…n sub-query expressions
kgyrtkirk
added
tests failed
tests pending
tests passed
and removed
tests pending
tests failed
labels
| /** | ||
| * Traverse the plan including sub-query expressions and collect table names from {@link TableScan} operators. | ||
| */ | ||
| protected Set<TableName> getAllTablesUsed(RelNode plan) { |
There was a problem hiding this comment.
Do we need to distinguish if we are going to look in sub-queries or not? If yes can't simply add a boolean parameter with an intuitive name instead of adding a new method (e.g., includeSubqueries, expandSubqueries)?
| */ | ||
| protected Set<TableName> getAllTablesUsed(RelNode plan) { | ||
| Set<TableName> tablesUsed = new HashSet<>(); | ||
| new HiveSubQueryVisitor() { |
There was a problem hiding this comment.
Instead of creating new visitors wouldn't make sense to apply the SubQueryRemoveRule either inside this method or before?
There was a problem hiding this comment.
This seems to be a good idea. Let's see how it works: #3246
| import org.apache.calcite.rex.RexSubQuery; | ||
| import org.apache.calcite.rex.RexVisitorImpl; | ||
|
|
||
| public class HiveSubQueryVisitor extends RelVisitor { |
There was a problem hiding this comment.
Do we really need this class? Can't we somehow exploit the SubQueryRemoveRule? If we need this then probably we want to add some basic javadoc.
| if (node instanceof Filter) { | ||
| visit((Filter) node); | ||
| } else if (node instanceof Project) { | ||
| visit((Project) node); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why do we need to focus only on Filter/Project ? Why not subqueries in Join or elsewhere? Can't we use the RelNode#accept(RexShuttle) for more uniform access?
|
|
||
| -- View can be used -> rewrite | ||
| explain cbo | ||
| select col0 from t2 where col0 in (select col0 from t1 where col0 = 1 union select col0 from t1 where col0 = 2); |
There was a problem hiding this comment.
Does it make sense to include also MV rewrite tests with subquery in project and join?
|
Closing this another solution was merged: #3246 |
What changes were proposed in this pull request?
Traverse the expressions in
ProjectandFilteroperators in CBO plan when collecting tables used.Why are the changes needed?
For Materialized View rewrite based on exact sql text match Hive uses the initial CBO plan which may contains subquery expressions.
Does this PR introduce any user-facing change?
Yes. If a query plan was rewritten to scan an outdated MV the results can be different.
How was this patch tested?