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

Running sql scripts from graphical clients sends all sql statements as a single query string #41

Open
kishaningithub opened this issue Nov 10, 2022 · 7 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@kishaningithub
Copy link
Contributor

Context

When i run sql scripts through vscode sql tools. The received query string in SimpleQueryFn is of the form

select * from employee limit 10;\nselect count(*) from student;

What i tried

In the SimpleQueryFn implementation that i wrote, i tried splitting the sql with ; character and running it separately and this results in the error columns have already been defined

What i expect

I expect psql wire to do the split and send queries one by one inside the SimpleQueryFn

What are your thoughts?

@jeroenrinzema
Copy link
Owner

Hi @kishaningithub, thanks for reporting this! We have to update the writer to allow for multiple column definitions to be returned. Looking at the PostgreSQL documentation it should be possible to send multiple row definitions after a single query message. I will update the writer and include a check to make sure that the previous definition has been marked as complete.

@jeroenrinzema jeroenrinzema added the bug Something isn't working label Nov 10, 2022
@jeroenrinzema
Copy link
Owner

@kishaningithub would you be able to include some debug logs of the psql server?

@kishaningithub
Copy link
Contributor Author

Below are the logs (took this by pointing my psql server to the PR)

{"level":"debug","timestamp":"2022-11-10T21:31:05.303184+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/wire.go:126","msg":"serving a new client connection"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.305053+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/wire.go:137","msg":"handshake successfull, validating authentication"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.305105+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:74","msg":"reading client parameters"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.305118+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:92","msg":"client parameter","key":"user","value":"asfsdf"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.30513+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:92","msg":"client parameter","key":"database","value":"asfsf"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.305145+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:92","msg":"client parameter","key":"client_encoding","value":"UTF8"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.305154+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/auth.go:33","msg":"authenticating client connection"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.311113+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/wire.go:150","msg":"connection authenticated, writing server parameters"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.311174+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:108","msg":"writing server parameters"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.311201+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:119","msg":"server parameter","key":"client_encoding","value":"UTF8"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.311267+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:119","msg":"server parameter","key":"is_superuser","value":"off"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.31138+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:119","msg":"server parameter","key":"session_authorization","value":"asfsdf"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.31167+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/handshake.go:119","msg":"server parameter","key":"server_encoding","value":"UTF8"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.311732+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/command.go:39","msg":"ready for query... starting to consume commands"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.312162+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/command.go:67","msg":"incoming command","length":82,"type":"Q"}
{"level":"debug","timestamp":"2022-11-10T21:31:05.312244+05:30","caller":"psql-wire@v0.4.1-0.20221110145115-cabdcd9971b3/command.go:220","msg":"incoming simple query","query":"select * from employee limit 10;\nselect count(*) from student;"}

@kishaningithub
Copy link
Contributor Author

Feel free to ask me if you need anything more

@kishaningithub
Copy link
Contributor Author

i understood #41 (comment) only now 🤦

I updated the query handler to split using ";" and run statements so that the way it performs operation is

Pseudocode

for query in queries.split(";") 
    run query
    write column metadata
    write rows
    write Complete msg

What happens is for the first query the result is written correctly. From the second query onwards i get the error

The columnWidths property is given an invalid value.

I tried it with several query combinations, it is always only the first query that succeeds

@kishaningithub
Copy link
Contributor Author

I also have this question (may be it should belong in discussions)

How should a consumer handle parameters array for bulk queries like this?

@jeroenrinzema
Copy link
Owner

jeroenrinzema commented Nov 14, 2022

It is currently a bit unclear to me how bulk queries have to be handled within the PostgreSQL wire protocol. It is mentioned within the message flow format document that multiple queries could be send.

Processing of the query string is complete. A separate message is sent to indicate this because the query string might contain multiple SQL commands. (CommandComplete marks the end of processing one SQL command, not the whole string.) ReadyForQuery will always be sent, whether processing terminates successfully or with an error.

It seems that most clients are handling bulk queries differently. Most split the query up front and send two separate queries to the server. I will try to reproduce the issue using the VSCode SQL client and try to reverse engineer how the client is expecting messages to be send back.

I would expect that all parameters for a single bulk query have to be send in one go (ex: SELECT * FROM users WHERE id = $1; SELECT * FROM settings WHERE user_id = $2;). But I will have to confirm this once I managed to reproduce the issue.

@jeroenrinzema jeroenrinzema added the help wanted Extra attention is needed label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
2 participants