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.

Sign up for GitHub

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(postgres): sqlx prepare fails if shared_preload_libraries=pg_stat_statements #2626

Closed
wants to merge 3 commits into from
Closed

fix(postgres): sqlx prepare fails if shared_preload_libraries=pg_stat_statements #2626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d82d6f
fix(postgres): sqlx prepare fails if shared_preload_libraries=pg_stat…
mrl5 Jul 17, 2023
f0696ea
fix(postgres): regression of #1449
mrl5 Jul 17, 2023
9102d5d
refactor(postgres): don't define unused fields in QueryPlan
mrl5 Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions sqlx-postgres/src/connection/describe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,15 +451,11 @@ WHERE rngtypid = $1

let mut nullables = Vec::new();

if let Explain::Plan(
plan @ Plan {
output: Some(outputs),
..
},
) = &explain
{
nullables.resize(outputs.len(), None);
visit_plan(&plan, outputs, &mut nullables);
if let Explain::QueryPlan(query_plan @ QueryPlan { plan, .. }) = &explain {
if let Some(outputs) = &query_plan.plan.output {
nullables.resize(outputs.len(), None);
visit_plan(&plan, outputs, &mut nullables);
}
}

Ok(nullables)
Expand Down Expand Up @@ -492,13 +488,19 @@ fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>
}

#[derive(serde::Deserialize)]
#[serde(untagged)]
enum Explain {
/// {"Plan": ...} -- returned for most statements
Plan(Plan),
QueryPlan(QueryPlan),
/// The string "Utility Statement" -- returned for
/// a CALL statement
#[serde(rename = "Utility Statement")]
UtilityStatement,
UtilityStatement(String),
mrl5 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

So this enum variant is never used? Its definiton was changed, but I see no other code that had to be updated because of that.

Copy link
Contributor Author

@mrl5 mrl5 Jul 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

correct, that's my understanding how it was working before my changes as well. I can include "clean up" refactor to this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so apparently it's required for compile-time checks agains DB - to match string:

error: error occurred while decoding column 0: data did not match any variant of untagged enum Explain at line 3 column 1
Error:    --> tests/postgres/macros.rs:103:15
    |
103 |     let row = sqlx::query!(r#"CALL forty_two(null)"#)
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this error originates in the macro `$crate::sqlx_macros::expand_query` which comes from the expansion of the macro `sqlx::query` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `sqlx` (test "postgres-macros") due to previous error
Error: warning: build failed, waiting for other jobs to finish...
Error: The process '/home/runner/.cargo/bin/cargo' failed with exit code 101

https://github.com/launchbadge/sqlx/actions/runs/5632237064/job/15260004403?pr=2626

}

#[derive(serde::Deserialize)]
struct QueryPlan {
#[serde(rename = "Plan")]
plan: Plan,
}

#[derive(serde::Deserialize)]
Expand Down
2 changes: 1 addition & 1 deletion tests/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ services:
volumes:
- "./postgres/setup.sql:/docker-entrypoint-initdb.d/setup.sql"
command: >
-c ssl=on -c ssl_cert_file=/var/lib/postgresql/server.crt -c ssl_key_file=/var/lib/postgresql/server.key
-c ssl=on -c ssl_cert_file=/var/lib/postgresql/server.crt -c ssl_key_file=/var/lib/postgresql/server.key -c shared_preload_libraries=pg_stat_statements
Copy link
Contributor Author

@mrl5 mrl5 Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if -c shared_preload_libraries=pg_stat_statements should be added to other postgres containers


postgres_15_client_ssl:
build:
Expand Down