Skip to content

parser: improved handling of SET#3777

Merged
sougou merged 1 commit intovitessio:masterfrom
sougou:betterset
Mar 23, 2018
Merged

parser: improved handling of SET#3777
sougou merged 1 commit intovitessio:masterfrom
sougou:betterset

Conversation

@sougou
Copy link
Copy Markdown
Contributor

@sougou sougou commented Mar 23, 2018

This change improves the handling of constructs like
SET NAMES and SET CHARSET. It allows intermixing of
these with other assignments.

VTGate and mysqlproxy have been correspondingly updated.

Signed-off-by: Sugu Sougoumarane ssougou@gmail.com

@sougou sougou requested review from bbeaudreault and demmer March 23, 2018 21:26
@sougou
Copy link
Copy Markdown
Contributor Author

sougou commented Mar 23, 2018

@arthurnn FYI

Copy link
Copy Markdown
Contributor

@bbeaudreault bbeaudreault left a comment

Choose a reason for hiding this comment

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

this is what i was thinking as well, lgtm! Probably want a couple tests for the issue at hand though: comma-separated set commands

This change improves the handling of constructs like
SET NAMES and SET CHARSET. It allows intermixing of
these with other assignments.

VTGate and mysqlproxy have been correspondingly updated.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou
Copy link
Copy Markdown
Contributor Author

sougou commented Mar 23, 2018

Oh. Good point. Test added.

Copy link
Copy Markdown
Member

@demmer demmer left a comment

Choose a reason for hiding this comment

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

LGTM!
Much better cleanup.

@sougou sougou merged commit 65a49c1 into vitessio:master Mar 23, 2018
@arthurnn
Copy link
Copy Markdown
Contributor

Sweet! ❤️

@sougou sougou deleted the betterset branch May 5, 2018 17:09
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.

4 participants