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

Improve splitting of SQL scripts into statements #1627

Merged
merged 14 commits into from Jul 23, 2019
Merged

Conversation

rnorth
Copy link
Member

@rnorth rnorth commented Jul 19, 2019

Based on #1563 by @DuanYuePeng, with some minor changes by myself:

Issue 1
for below sql statement, the split function will fail because the table name contains "END".

CREATE TABLE bar (foo VARCHAR(255));
CREATE TABLE gender (gender VARCHAR(255));
CREATE TABLE foo (bar VARCHAR(255));

fixed this issue by checking whether the keywords (END or BEGIN) is separated by any controlling char. eg. '\t', '\n' or ' '.
Issue 2
for below sql statement, the split function will fail because of incorrect inBlockComment flag.

CREATE TABLE bar (foo VARCHAR(255));
/* Insert Values */
INSERT INTO bar (foo) values ('--1');
INSERT INTO bar (foo) values ('--2');

fixed this issue by setting inBlockComment to false after skipping the block comment.

This should fix #1547

@rnorth rnorth changed the title Duan yue peng master fixed sql splitting issue Jul 19, 2019
@rnorth
Copy link
Member Author

rnorth commented Jul 20, 2019

Hmm, both Azure builds are failing on:

GenericContainerRuleTest > sharedMemorySetTest

🤔

@bsideup
Copy link
Member

bsideup commented Jul 20, 2019

@rnorth it seems to be flaky, I think I've seen this failure on other CIs too...

FYI I just created #1629

@rnorth rnorth added this to the next milestone Jul 20, 2019
@rnorth rnorth changed the title fixed sql splitting issue Improve splitting of SQL scripts into statements Jul 20, 2019
flushStringBuilder(sb, statements);
}

private static StringBuilder flushStringBuilder(StringBuilder sb, List<String> statements) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! 👍

@rnorth
Copy link
Member Author

rnorth commented Jul 23, 2019

Merging, with what appears to be a flaky test on Travis CI.

@rnorth rnorth merged commit fcfff1a into master Jul 23, 2019
@rnorth rnorth deleted the DuanYuePeng-master branch July 23, 2019 10:53
rnorth added a commit that referenced this pull request Aug 4, 2019
rnorth added a commit that referenced this pull request Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScriptUtils parsing failed when using field name with 'end' keyword
3 participants