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

Fix for multiline insert queries #54

Merged
merged 4 commits into from
Nov 26, 2018
Merged

Fix for multiline insert queries #54

merged 4 commits into from
Nov 26, 2018

Conversation

errx
Copy link
Contributor

@errx errx commented Nov 13, 2018

When insert query contains multiple lines batchmode is not working.

To fix that I have added regexp flag to match "." to "\n"

stmt_test.go Outdated
@@ -194,6 +194,18 @@ func (s *stmtSuite) TestFixDoubleInterpolateInStmt() {
s.NoError(rows.Close())
}

func (s *stmtSuite) TestFixMultiLineInsert() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a new testcase for test TestExecMulti.
like this:

queries = []string{"INSERT INTO data (i64) VALUES (?)", ""INSERT INTO data(\ni64\n) VALUES (\n?\n)"}
for q in queries:
   //.. test case logic

Copy link
Contributor

Choose a reason for hiding this comment

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

this test can be removed, since TestExecMulti covers this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@bgaifullin
Copy link
Contributor

Thanks for your PR, there is just one comment about test case
Also seems like new golint does not work with go 1.8, I need to fix CI before merge this PR

@errx
Copy link
Contributor Author

errx commented Nov 20, 2018

I've added a testcase

@bgaifullin
Copy link
Contributor

LGTM, thank you for PR

@bgaifullin bgaifullin merged commit 41f7b82 into mailru:master Nov 26, 2018
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