Skip to content
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, parser: support JOIN hint for UPDATE/DELETE statements #6626

Merged
merged 10 commits into from
May 25, 2018

Conversation

lzmhhh123
Copy link
Contributor

@lzmhhh123 lzmhhh123 commented May 23, 2018

To support JOIN hints like /*+ TiDB_SMJ */ in UPDATE & DELETE statements.

@winkyao
Copy link
Contributor

winkyao commented May 23, 2018

@lzmhhh123 Please add some description for this PR.

@XuHuaiyu
Copy link
Contributor

/run-all-tests

parser/parser.y Outdated
x.Where = $8.(ast.ExprNode)
Priority: $3.(mysql.PriorityEnum),
Quick: $4.(bool),
IgnoreErr: $5.(bool),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz fix the code indent before $5.(bool),

ast/dml.go Outdated
@@ -791,6 +791,8 @@ type DeleteStmt struct {
Quick bool
IsMultiTable bool
BeforeFrom bool
// TableHints represents the level Optimizer Hint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incomplete comment?

@@ -1998,60 +1998,67 @@ DoStmt:
*
*******************************************************************/
DeleteFromStmt:
"DELETE" PriorityOpt QuickOptional IgnoreOptional "FROM" TableName IndexHintListOpt WhereClauseOptional OrderByOptional LimitClause
"DELETE" TableOptimizerHints PriorityOpt QuickOptional IgnoreOptional "FROM" TableName IndexHintListOpt WhereClauseOptional OrderByOptional LimitClause
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seem that the TableOptimizerHints is not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but it will cause yacc reduce conflict without this TableOptimizerHints.

@shenli
Copy link
Member

shenli commented May 24, 2018

/run-all-tests

ast/dml.go Outdated
@@ -791,6 +791,8 @@ type DeleteStmt struct {
Quick bool
IsMultiTable bool
BeforeFrom bool
// TableHints represents the level Optimizer Hint for join type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean "the table level optimizer hint"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I just referred the previous comments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about fixing these wrong comments in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@zz-jason
Copy link
Member

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label May 25, 2018
@zz-jason
Copy link
Member

@winoros @lamxTyler PTAL

@shenli
Copy link
Member

shenli commented May 25, 2018

LGTM

@shenli shenli added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 25, 2018
@shenli shenli merged commit ff7c0ce into pingcap:master May 25, 2018
coocood added a commit that referenced this pull request May 31, 2018
* support high priority for DELETE/UPDATE statement

* plan, parser: support JOIN hint for UPDATE/DELETE statements (#6626)
@lzmhhh123 lzmhhh123 deleted the dev/joinhintenhance branch July 24, 2019 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants