Skip to content

parser: resolve reduce/reduce conflicts#86

Merged
qiuyesuifeng merged 2 commits intomasterfrom
coocood/parser-conflict
Sep 9, 2015
Merged

parser: resolve reduce/reduce conflicts#86
qiuyesuifeng merged 2 commits intomasterfrom
coocood/parser-conflict

Conversation

@coocood
Copy link
Member

@coocood coocood commented Sep 9, 2015

Resolved all the reduce/reduce conflicts except for 1

Fixes issue #73

Resolved all the reduce/reduce conflicts except for 1

Fixes issue #73
@coocood
Copy link
Member Author

coocood commented Sep 9, 2015

@cznic

The remaining conflict is in SetStmt

"SET" VariableAssignmentList | "SET" "PASSWORD" ForUserOpt eq PasswordOpt

I didn't figure out a proper way to resolve it.

Do you have any suggestions?

Thank you.

@siddontang
Copy link
Member

should we update the make parser adding -cr for later check?

@coocood
Copy link
Member Author

coocood commented Sep 9, 2015

@siddontang I didn't use -cr, but still can see conflict check report.

@cznic
Copy link

cznic commented Sep 9, 2015

@coocood Just mechanically resolved, you have to double check the diff before adopting as I don't understand well the existence of UnReservedKeyword.

diff --git a/parser/parser.y b/parser/parser.y
index d1d0cad..c71f8fd 100644
--- a/parser/parser.y
+++ b/parser/parser.y
@@ -323,7 +323,6 @@ import (
    Field           "field expression"
    Field1          "field expression optional AS clause"
    FieldList       "field expression list"
-   ForUserOpt      "Set password for user option"
    FromClause      "From clause"
    Function        "function expr"
    FunctionCall        "function call post part"
@@ -471,6 +470,7 @@ import (
 %left  xor
 %left  andand and
 %left  between
+%precedence lower_than_eq
 %left  eq ge le neq neqSynonym '>' '<' is like in
 %left  '|'
 %left  '&'
@@ -1531,7 +1531,7 @@ Identifier:
 // TODO: Add Data Type UnReserved Keywords
 UnReservedKeyword:
    "AUTO_INCREMENT" | "BEGIN" | "BIT" | "BOOL" | "BOOLEAN" | "CHARSET" | "COLUMN" | "COLUMNS" | "DATE" | "DATETIME"
-|  "ENGINE" | "FULL" | "LOCAL" | "NAMES" | "OFFSET" | "PASSWORD" | "QUICK" | "ROLLBACK" | "SESSION" | "GLOBAL" 
+|  "ENGINE" | "FULL" | "LOCAL" | "NAMES" | "OFFSET" | "PASSWORD" %prec lower_than_eq | "QUICK" | "ROLLBACK" | "SESSION" | "GLOBAL" 
 |  "TABLES"| "TEXT" | "TIME" | "TIMESTAMP" | "TRANSACTION" | "TRUNCATE" | "VALUE" | "WARNINGS" | "YEAR" | "NOW"
 |  "SUBSTRING" | "MODE"

@@ -2487,9 +2487,13 @@ SetStmt:
    {
        $$ = &stmts.SetCharsetStmt{Charset: $3.(string)} 
    }
-|  "SET" "PASSWORD" ForUserOpt eq PasswordOpt
+|  "SET" "PASSWORD" eq PasswordOpt
    {
-       $$ = &stmts.SetPwdStmt{User: $3.(string), Password: $5.(string)} 
+       $$ = &stmts.SetPwdStmt{Password: $4.(string)} 
+   }
+|  "SET" "PASSWORD" "FOR" Username eq PasswordOpt
+   {
+       $$ = &stmts.SetPwdStmt{User: $4.(string), Password: $6.(string)} 
    }

 VariableAssignment:
@@ -2574,16 +2578,6 @@ UserVariable:
        $$ = &expressions.Variable{Name: v, IsGlobal: false, IsSystem: false}
    }

-
-ForUserOpt:
-   {
-       $$ = "" 
-   }
-|  "FOR" Username
-   {
-       $$ = $2.(string)
-   }
-
 Username:
    Identifier
    {
(10:35) jnml@r550:~/src/github.com/pingcap/tidb/parser$ goyacc -o /dev/null -cr parser.y
Parse table entries: 114264 of 366201, x 16 bits == 228528 bytes
conflicts: 25 shift/reduce
(10:35) jnml@r550:~/src/github.com/pingcap/tidb/parser$ 

BTW: I thinkg the S/R conflicts should go away as well.

@shenli
Copy link
Member

shenli commented Sep 9, 2015

@cznic
UnReservedKeyword defines a MySQL keyword list which can be used as identifier.
See: https://dev.mysql.com/doc/refman/5.7/en/keywords.html

Identifier is defined as identifier | UnReservedKeyword in parser.y. And "SELECT commit FROM tbl;" will not encounter syntax error.

@coocood
Copy link
Member Author

coocood commented Sep 9, 2015

@cznic
You helped us a lot again, thank you.

Since the Github converted tab to space, the patch can not be applied automatically, I manually applied this patch.

We do not have much experience on parser and have difficulty resolve S/R conflicts, would you like to help us addressing the issue with PR?

@cznic
Copy link

cznic commented Sep 9, 2015

@coocood I'm not sure I can manage to create enough free time to look into it. This is my first week at work after vacations and a lot of issues accumulated meanwhile.

Maybe there's some chance through this weekend, but I cannot really promise anything, so please don't be disappointed when that opportunity fails (family, etc.).

@coocood
Copy link
Member Author

coocood commented Sep 9, 2015

@cznic
Got it, thanks.

@shenli
Copy link
Member

shenli commented Sep 9, 2015

LGTM

1 similar comment
@qiuyesuifeng
Copy link
Member

LGTM

qiuyesuifeng added a commit that referenced this pull request Sep 9, 2015
parser: resolve reduce/reduce conflicts
@qiuyesuifeng qiuyesuifeng merged commit 0e9cddf into master Sep 9, 2015
@qiuyesuifeng qiuyesuifeng deleted the coocood/parser-conflict branch September 9, 2015 10:51
@cznic
Copy link

cznic commented Sep 9, 2015

@coocood I'll try to get rid of the S/R conflicts one parser state at time. Your task will be to review and carefully test those changes, ok?

I have to say that I have never before worked with a language which mixes identifiers and keywords and that whoever of the MySQL authors came with that idea introduced a lot of trouble for the grammar writers. I'm not sure if I can resolve all the conflicts 👎

@coocood
Copy link
Member Author

coocood commented Sep 9, 2015

@cznic Sure.

YuJuncen pushed a commit to YuJuncen/tidb that referenced this pull request Apr 23, 2021
…ints (pingcap#86)

* tidb-lightning-ctl: close and cleanup engine after destroying checkpoints

This prevents accumulation of open engines, which eventually leads to
ResourceTemporarilyUnavailable("Too many open engines").

* tidb-lightning-ctl: addressed comment
okJiang pushed a commit to okJiang/tidb that referenced this pull request Oct 19, 2021
* cmd: align CLI with mydumper

* -F without unit means MiB. also allow including units
* swapped -s and -S
* changed -H to -h

* cmd: added the -T flag
iosmanthus pushed a commit to iosmanthus/tidb that referenced this pull request Oct 31, 2022
* fix: reinit metrics vars after setting serverless labels

* fix br metric init

Signed-off-by: ystaticy <y_static_y@sina.com>

* fix br metric init

Signed-off-by: ystaticy <y_static_y@sina.com>

* fix comments

Signed-off-by: ystaticy <y_static_y@sina.com>

* update cd.yml

Signed-off-by: ystaticy <y_static_y@sina.com>

* update cd.yml

Signed-off-by: ystaticy <y_static_y@sina.com>

* update cd-br.yml

Signed-off-by: ystaticy <y_static_y@sina.com>

* fix br metrics call server init

Signed-off-by: ystaticy <y_static_y@sina.com>

* add status info

Signed-off-by: ystaticy <y_static_y@sina.com>

Signed-off-by: ystaticy <y_static_y@sina.com>
Co-authored-by: ystaticy <y_static_y@sina.com>
nolouch pushed a commit to tidblabs/tidb that referenced this pull request Jan 6, 2023
…ap#144 into keyspace on release 6.4  (pingcap#161)

* Merge PR pingcap#78,pingcap#79,pingcap#82,pingcap#83,pingcap#86,pingcap#144 into keyspace on release 6.4

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

* make check

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

* make check

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

* fix error

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

* fix comment

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

* make check

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>

Signed-off-by: zeminzhou <zhouzemin@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants