Skip to content

Conversation

@Achilles-96
Copy link
Contributor

@Achilles-96 Achilles-96 commented Apr 1, 2017

Signed-off-by: Raghuram Vadapalli [email protected]

Copy link
Contributor

@nijel nijel left a comment

Choose a reason for hiding this comment

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

From the test failure, it seems that it breaks parsing comments terminated by \n.

@Achilles-96
Copy link
Contributor Author

I am not much familiar with parser project. Is it possible to run these tests on local machine.

nijel added a commit that referenced this pull request Apr 3, 2017
See #147

Signed-off-by: Michal Čihař <[email protected]>
@nijel
Copy link
Contributor

nijel commented Apr 3, 2017

Indeed the documentation on this was missing, I've just added it: https://github.com/phpmyadmin/sql-parser/blob/master/CONTRIBUTING.md

nijel added a commit that referenced this pull request Apr 3, 2017
It's mostly useful for debugging changes in parser.

See #147.

Signed-off-by: Michal Čihař <[email protected]>
@nijel
Copy link
Contributor

nijel commented Apr 3, 2017

I've also added script bin/tokenize-query which you can use for testing parser as well.

With your changes the following query is tokenized wrongly:

#comment
SELECT 1

See:

$ ./bin/tokenize-query --query '#comment
SELECT 1'
[TOKEN 0]
Type = 4
Flags = 1
Value = '#comment
SELECT 1'
Token = '#comment
SELECT 1'

[TOKEN 1]
Type = 9
Flags = 0
Value = NULL
Token = NULL

While the correct (and current) tokenization is:

[TOKEN 0]
Type = 4
Flags = 1
Value = '#comment'
Token = '#comment'

[TOKEN 1]
Type = 1
Flags = 3
Value = 'SELECT'
Token = 'SELECT'

[TOKEN 2]
Type = 3
Flags = 0
Value = ' '
Token = ' '

[TOKEN 3]
Type = 6
Flags = 0
Value = 1
Token = '1'

[TOKEN 4]
Type = 9
Flags = 0
Value = NULL
Token = NULL

@nijel
Copy link
Contributor

nijel commented Apr 4, 2017

I've come with different fix, which does not break tokenization, see #148.

@nijel
Copy link
Contributor

nijel commented Apr 4, 2017

Closing this for now as it's breaking tokenization. Either we will use #148 or different patch is needed.

@nijel nijel closed this Apr 4, 2017
@nijel nijel self-assigned this Apr 4, 2017
andersonda pushed a commit to ecu-pase-lab/sql-parser that referenced this pull request Apr 11, 2017
It's mostly useful for debugging changes in parser.

See phpmyadmin#147.

Signed-off-by: Michal Čihař <[email protected]>
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.

2 participants