-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
planner/core: use constant propagate before predicates push down #21061
Changes from 20 commits
f99363c
35c9f20
b6accd8
207e1b5
a7ad554
f2576ff
4969290
9894447
cd4b268
748db3c
d35a362
18f75f9
bb70c38
3c76c56
48f0397
032ed69
4cae468
0b1b82f
dd10a6d
3353f09
82c3d73
0d5151b
7edd92f
f24801b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,7 @@ func (p *LogicalUnionScan) PredicatePushDown(predicates []expression.Expression) | |
|
||
// PredicatePushDown implements LogicalPlan PredicatePushDown interface. | ||
func (ds *DataSource) PredicatePushDown(predicates []expression.Expression) ([]expression.Expression, LogicalPlan) { | ||
predicates = expression.PropagateConstant(ds.ctx, predicates) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we would do |
||
ds.allConds = predicates | ||
ds.pushedDownConds, predicates = expression.PushDownExprs(ds.ctx.GetSessionVars().StmtCtx, predicates, ds.ctx.GetClient(), kv.UnSpecified) | ||
return predicates, ds | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,7 +123,7 @@ | |
{ | ||
"SQL": "select * from t where a = 1 and (b = 1 or b = 2) and b = 3 and c > 1;", | ||
"Plan": [ | ||
"TableDual_5 0.00 root rows:0" | ||
"TableDual_5 8000.00 root rows:0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when 'stats' is 'pseudo',this value doesn't make much sense. |
||
], | ||
"Result": null | ||
}, | ||
|
@@ -169,9 +169,7 @@ | |
{ | ||
"SQL": "select * from t where a = 1 and b is null and b = 1 and c > 1;", | ||
"Plan": [ | ||
"IndexReader_7 0.27 root index:Selection_6", | ||
"└─Selection_6 0.27 cop[tikv] isnull(test.t.b)", | ||
" └─IndexRangeScan_5 0.33 cop[tikv] table:t, index:a(a, b, c) range:(1 1 1,1 1 +inf], keep order:false, stats:pseudo" | ||
"TableDual_5 8000.00 root rows:0" | ||
], | ||
"Result": null | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only need two of three expressions from
eq(test_partition.t1.a, test_partition.t1.id), gt(test_partition.t1.a, 10), gt(test_partition.t1.id, 10)
to determine the result.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have discussed it with @ou-bing . The reason is
PropagateConstant
needs to keep the origin equal condition for some other optimization like join operators. We can improve it in another PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@winoros What's your opinion? PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this three expression, the
t1.a=t1.id
must be kept and at least one of thet1.a > 10
andt1.id > 10
should be kept.If we remove
t1.a=t1.id
and keep the other two, the result will be obviously wrong.We can keep its behavior here and do the optimization later. This should be a improvement for the
ConstantPropagation
itself.