-
Notifications
You must be signed in to change notification settings - Fork 4
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
Transactions aren't executed with "psql -c" #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. Left some comments on style
tokio-postgres/tests/test/main.rs
Outdated
} | ||
} | ||
|
||
match client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably can just be client.simple_query(..).await.unwrap()
tokio-postgres/tests/test/main.rs
Outdated
_ => panic!("unexpected message"), | ||
} | ||
match &messages[3] { | ||
SimpleQueryMessage::CommandComplete(CommandCompleteContents { fields: f, .. }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be SimpleQueryMessage::CommandComplete(CommandCompleteContents { fields, .. }) if fields.is_none() => {}
. Then the fields.is_some()
case is captured in _ =>
. Same for the below cases.
Also this is a nit, but we typically wouldn't alias using fields: f,
instead of referencing as fields,
tokio-postgres/tests/test/main.rs
Outdated
} | ||
match &messages[4] { | ||
SimpleQueryMessage::CommandComplete(CommandCompleteContents { fields: f, .. }) => { | ||
if let Some(field_vec) = &f { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can match with SimpleQueryMessage::CommandComplete(CommandCompleteContests { fields: Some(f), ... })
instead of checking whether fields is_some() in the body.
The transaction did not appear to execute when running a set of SQL commands using psql commands with the '-c' option, such as "SELECT * FROM t; BEGIN; UPDATE t SET k = id; END;". This issue was caused by the tokio-postgres crate adding column information to all EmptyQueryResponses for every SQL query. The resolution for this issue was to only add the column information to 'SELECT' queries.
Bump deps on crates in our rust-postgres fork to include this merged PR: readysettech/rust-postgres#9 Change-Id: I59d1f7882b74d6c418eea8eef35ada1c26c4f7f7
Bump deps on crates in our rust-postgres fork to include this merged PR: readysettech/rust-postgres#9 Change-Id: I59d1f7882b74d6c418eea8eef35ada1c26c4f7f7 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/5958 Tested-by: Buildkite CI Reviewed-by: Dan Wilbanks <[email protected]>
The transaction did not appear to execute when running a set of SQL commands using psql commands with the '-c' option, such as "SELECT * FROM t; BEGIN; UPDATE t SET k = id; END;". This issue was caused by the tokio-postgres crate adding column information to all EmptyQueryResponses for every SQL query. The resolution for this issue was to only add the column information to 'SELECT' queries.