-
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
ddl: don't rely on expression.Column.ColName
#11255
ddl: don't rely on expression.Column.ColName
#11255
Conversation
6e41a73
to
15a379b
Compare
Codecov Report
@@ Coverage Diff @@
## master #11255 +/- ##
================================================
- Coverage 81.2684% 81.2643% -0.0041%
================================================
Files 423 423
Lines 90094 90117 +23
================================================
+ Hits 73218 73233 +15
- Misses 11581 11586 +5
- Partials 5295 5298 +3 |
Codecov Report
@@ Coverage Diff @@
## master #11255 +/- ##
=======================================
Coverage 81.33% 81.33%
=======================================
Files 423 423
Lines 90750 90750
=======================================
Hits 73807 73807
Misses 11632 11632
Partials 5311 5311 |
|
||
func extractPartitionColumns(partExpr string, tblInfo *model.TableInfo) ([]*model.ColumnInfo, error) { | ||
partExpr = "select " + partExpr | ||
stmts, _, err := parser.New().Parse(partExpr, "", "") |
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.
Should we handle warnings like ParseSimpleExprWithTableInfo
?
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 think we don't need. The warning generated before is not handled either.
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.
LGTM
LGTM |
} | ||
extractor := &columnNameExtractor{ | ||
tblInfo: tblInfo, | ||
extractedColumns: make([]*model.ColumnInfo, 0), |
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.
You can keep extractedColumns as nil
/run-all-tests |
What problem does this PR solve?
Clean the dependencies of
expression.Column
.What is changed and how it works?
Directly extract the columns. Don't build expression and use
expression.Column.ColName
.Check List
Tests