-
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
plan: fix predicate push down for UnionScan #7695
Conversation
/run-all-tests |
@@ -78,3 +78,16 @@ func (s *testSuite) TestDirtyTransaction(c *C) { | |||
tk.MustQuery("select * from t use index(idx) where c > 1 and d = 4").Check(testkit.Rows("1 2 3 4")) | |||
tk.MustExec("commit") | |||
} | |||
|
|||
func (s *testSuite) TestUnionScanWithCastCondition(c *C) { |
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.
It's better to add another explain test.
Eval it in |
@winoros |
@zz-jason PTAL |
id count task operator info | ||
Projection_5 8000.00 root test.ta.a | ||
└─Selection_6 8000.00 root eq(cast(test.ta.a), 1) | ||
└─UnionScan_7 10000.00 root eq(cast(test.ta.a), 1) |
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.
why not Eval it in getSnapshotRow
?
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.
It's out of the scope of UnionScan's responsibility
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.
Then why not removing all the filters inside the UnionScan
?
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.
It can be refactored later.
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.
The filter a = 1
, which is Selection_6
, should be added as a parent of TableReader_9
.
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.
This is better, we can do it in the next 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.
LGTM
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.
LGMT
/run-all-tests |
What problem does this PR solve?
Expected result is
Got
What is changed and how it works?
The predicate push down for UnionScan plan returns retained predicates instead of nil.
If we return nil predicates, the parent Selection will be removed, but conditions in UnionScan is only used to evaluate dirty table added rows, the conditions that cannot be pushed down to coprocessor will be lost.
Check List
Tests
Related changes