Planner support push down predicates past agg, win and sort#1471
Planner support push down predicates past agg, win and sort#1471morningman merged 6 commits intoapache:masterfrom chenhao7253886:master
Conversation
morningman
left a comment
morningman
left a comment
There was a problem hiding this comment.
Please create an ISSUE to describe the problem and how you solve this problem.
chenhao7253886
changed the title
chenhao7253886
changed the title
| getIds(exprTupleIds, exprSlotIds); | ||
|
|
||
| final List<SlotId> intersection = Lists.newArrayList(); | ||
| intersection.addAll(exprSlotIds); |
There was a problem hiding this comment.
Why not just reuse the exprSlotIds to calculate the intersection?
like:
exprSlotIds.retainAll(slotIds);
And you can just !return exprSlotIds.retainAll(slotIds).
If exprSlotIds changes, retainAll will return true.
There was a problem hiding this comment.
because it change the param arg.
| * Push down predicates rules | ||
| */ | ||
|
|
||
| private void pushDownPredicates(Analyzer analyzer, SelectStmt stmt) throws AnalysisException { |
There was a problem hiding this comment.
Could add some comment to these functions?
There is no comment at all?
There was a problem hiding this comment.
ok, i will explain it.
| return false; | ||
| } | ||
|
|
||
| private boolean putPredicatesOnFrom(SelectStmt stmt, Analyzer analyzer, List<Expr> predicates) throws AnalysisException { |
There was a problem hiding this comment.
Maybe the return of this function is useless.
There was a problem hiding this comment.
Ok, i will remove it.
|
LGTM |
| for (SlotId slotId : slotIds) { | ||
| final SlotDescriptor slotDesc = analyzer.getDescTbl().getSlotDesc(slotId); | ||
| if (slotDesc.getSourceExprs().get(0).getFn() instanceof AggregateFunction) { | ||
| isAllSlotReferingGroupBys = false; |
| putPredicatesOnFrom(stmt, analyzer, pushDownPredicates); | ||
| } | ||
|
|
||
| private List<Expr> getPredicatesBoundedByGroupbysAndReplaceSlotWithSourceExpr(List<Expr> predicates, Analyzer analyzer) { |
There was a problem hiding this comment.
can you make this function name short
There was a problem hiding this comment.
OK, i will fix it.
4 participants
#1438