expression: replace equal condition by true when to constant propagation | tidb-test=pr/2625 #64309
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-tests-checked
release-note-none
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #64309 +/- ##
================================================
- Coverage 74.7061% 73.4696% -1.2365%
================================================
Files 1888 1868 -20
Lines 515172 508779 -6393
================================================
- Hits 384865 373798 -11067
- Misses 106492 112765 +6273
+ Partials 23815 22216 -1599
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
hawkingrei
changed the title
hawkingrei
changed the title
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
hawkingrei
force-pushed
the
64216
branch
2 times, most recently
from
049807e to
b1221ce
Compare
|
/retest |
|
/retest |
1 similar comment
|
/retest |
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
|
/retest |
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
|
/retest |
1 similar comment
|
/retest |
| // It will be deleted in the replaceEqCondtionWithTrueitionsWithConstants | ||
| s.unionSet.Union(lID, rID) | ||
| } else if lID != rID { | ||
| s.conditions[i] = &Constant{ |
There was a problem hiding this comment.
here means LID and RID is already in the union set(the equiv relationship is catched, so we can safely convert this EQ(LID, RID) to be a true)
what i concern here is that, next time, if we wanna do the constant prop one more time, we should build the union set from these condition slice again, while may we still be able to catch the equiv relation ship between LID and RID?
There was a problem hiding this comment.
looks like we only substitute for appended condition, yes ok.
There was a problem hiding this comment.
Unionset can capture any equivalence relations of the inferred type. for example,
a = b, b = c, c = d, d = e
We get a = e by the unionset.
There was a problem hiding this comment.
Actually, it doesn't affect the logic here, since all equivalence relations are preserved, and only the redundant equivalence relations are eliminated.
|
/retest |
Signed-off-by: Weizhen Wang <[email protected]>
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AilinKid, windtalker The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
3 participants
What problem does this PR solve?
Issue Number: close #64216
Problem Summary:
What changed and how does it work?
There are three scenarios here.
In the previous implementation, we only passed through constant values to create a new expression. However, what we actually need is to directly replace the equivalent conditions in the expression with true, in order to avoid generating unnecessary expressions.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.