-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Optimize MySQL INSERT INTO VALUES AS row_alias parse SQLStatement #36680
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
base: master
Are you sure you want to change the base?
Optimize MySQL INSERT INTO VALUES AS row_alias parse SQLStatement #36680
Conversation
| : INSERT insertSpecification INTO? tableName partitionNames? insertBody onDuplicateKeyClause? returningClause? | ||
| ; | ||
|
|
||
| insertBody : insertValuesClause | setAssignmentsClause valueReference? | insertSelectClause valueReference? ; |
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.
Please put the content after : on a new line.
| result.setInsertColumns(new InsertColumnsSegment(ctx.start.getStartIndex() - 1, ctx.start.getStartIndex() - 1, Collections.emptyList())); | ||
| } | ||
| result.getValues().addAll(createInsertValuesSegments(ctx.assignmentValues())); | ||
|
|
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.
Please remove useless blank line.
| ValueReferenceSegment valueRef = (ValueReferenceSegment) visit(ctx.valueReference()); | ||
| result.setValueReference(valueRef); | ||
| } | ||
|
|
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.
Please remove useless blank line.
|
|
||
| @Override | ||
| public ASTNode visitRowAlias(final RowAliasContext ctx) { | ||
| return new CommonExpressionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.getText()); |
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 return CommonExpressionSegment?
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.
May use RowAliasSegment here is better.
| } | ||
| return new SetAssignmentSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), assignments); | ||
| SetAssignmentSegment setAssignment = new SetAssignmentSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), assignments); | ||
| return setAssignment; |
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.
Please remove setAssignment and return new xxx directly.
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.
Done
|
Hi @strongduanmu , this pr is ready to review~ |
Ok, I will review this pr later. |
Fixes #36583.
Changes proposed in this pull request:
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.