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 handling of multiple statements. #4

Merged
merged 2 commits into from
Feb 25, 2016

Conversation

petermattis
Copy link

When a query contained multiple statements any statement which did not
return data (e.g. SET, CREATE DATABASE, etc) would be ignored unless
it was the last statement.

@bdarnell
Copy link

This needs a test.

@petermattis petermattis force-pushed the pmattis/fix-multiple-results branch 2 times, most recently from 0b27271 to e91e749 Compare February 24, 2016 20:28
@petermattis
Copy link
Author

Test added.

// If set, this connection will not skip returning results for
// multi-statement queries where one of the statements does not return
// results such as CREATE TABLE or SET.
disableSkipZeroResultQueries bool

Choose a reason for hiding this comment

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

The double negative of disableSkip is hard to parse. How about enableMultipleResults, since that's what you're really opting in to here?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I knew that was the wrong name when writing this. Done.

@bdarnell
Copy link

LGTM

When a query contained multiple statements any statement which did not
return data (e.g. `SET`, `CREATE DATABASE`, etc) would be ignored unless
it was the last statement.
petermattis added a commit that referenced this pull request Feb 25, 2016
@petermattis petermattis merged commit dd29f9a into master Feb 25, 2016
@petermattis petermattis deleted the pmattis/fix-multiple-results branch February 25, 2016 01:14
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