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

cli: should two statements work in sql shell? #4016

Closed
mberhault opened this issue Jan 28, 2016 · 7 comments
Closed

cli: should two statements work in sql shell? #4016

mberhault opened this issue Jan 28, 2016 · 7 comments
Assignees
Milestone

Comments

@mberhault
Copy link
Contributor

eg: running the following in the sql client:

root@:15432> select 3; show databases;
+---+
| 3 |
+---+
| 3 |
+---+

I had to change something similar in https://github.com/cockroachdb/cockroach/pull/3757/files#diff-7d7fb132e111ef7398af7150b1d54a32 Example_SQL Line 544. With the cockroach driver, the final result was from the last query. With the postgres driver, only the first query is run.

@tamird
Copy link
Contributor

tamird commented Jan 28, 2016

odd. pgwire should run all queries and return all results.

@knz
Copy link
Contributor

knz commented Jan 29, 2016

I ran into a similar issue when implementing sql -e. The issue is that the sql.driver.Queryer interface only returns one Rows set, for the last statement in the query. As long as this interface is a single Rows set I do not see how to do anything better.
(short of implementing a full SQL parser on the client side to split semicolons)

@mberhault
Copy link
Contributor Author

@knz: yup. this is where I found it too. I had to tweak one of your test cases in cli_test.go due to behavior change for multiple statements between the cockroach and pq drivers.

@knz
Copy link
Contributor

knz commented Jan 29, 2016

Oh wait are you saying that multiple statements simply don't work over pgwire? This is terrible.
I do not care that results are only reported for the last statements, but all statements must execute and an error at any statement must prevent subsequent statements from running. Otherwise #4036 can't work.

@mberhault
Copy link
Contributor Author

I honestly don't know. I was definitely not getting two sets of results. The behavior change was in getting the result from the first statement instead of the second. I didn't not check whether the second was running or not

@tamird
Copy link
Contributor

tamird commented Jan 29, 2016

If you point psql at cockroach and run two statements, you should observe
the results for both.

On Fri, Jan 29, 2016 at 11:57 AM, marc notifications@github.com wrote:

I honestly don't know. I was definitely not getting two sets of results.
The behavior change was in getting the result from the first statement
instead of the second. I didn't not check whether the second was running or
not


Reply to this email directly or view it on GitHub
#4016 (comment)
.

@mberhault
Copy link
Contributor Author

we definitely run both. eg:

$ ./cockroach sql --dev
oot@:15432> select 3; create database foo;
+---+
| 3 |
+---+
| 3 |
+---+
root@:15432> show databases;
+----------+
| Database |
+----------+
| "foo"    |
| "system" |
+----------+

Our code uses the go interface the way it should, I'm 95% sure the limitation is in the sql driver itself. see golang/go#12382 for the related issue.

petermattis added a commit to petermattis/cockroach that referenced this issue Feb 3, 2016
Use the new functionality in lib/pq to select the next set of results
when multiple statements were executed.

Switch to using the "postgres" SQL driver in the cli tests.

Remove runPrettyQueryWithFormat. It was only used in tests.

Fixes cockroachdb#4016.
petermattis added a commit to petermattis/cockroach that referenced this issue Feb 4, 2016
Use the new functionality in lib/pq to select the next set of results
when multiple statements were executed.

Switch to using the "postgres" SQL driver in the cli tests.

Remove runPrettyQueryWithFormat. It was only used in tests.

Fixes cockroachdb#4016.
@petermattis petermattis changed the title SQL: should two statements work? cli: should two statements work in sql shell? Feb 12, 2016
@petermattis petermattis removed the SQL label Feb 13, 2016
@petermattis petermattis modified the milestone: Beta Feb 14, 2016
@petermattis petermattis assigned petermattis and unassigned cuongdo Feb 21, 2016
petermattis added a commit to petermattis/cockroach that referenced this issue Feb 22, 2016
Use the new functionality in lib/pq to select the next set of results
when multiple statements were executed.

Fixes cockroachdb#4016.
petermattis added a commit to petermattis/cockroach that referenced this issue Feb 23, 2016
Use the new functionality in lib/pq to select the next set of results
when multiple statements were executed.

Fixes cockroachdb#4016.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants