Skip to content

Commit 487b89a

Browse files
abonandermrl5
andauthored
fix: ignore extra fields in Postgres describe parsing (launchbadge#2670)
* fix(postgres): sqlx prepare fails if shared_preload_libraries=pg_stat_statements closes launchbadge#2622 refs: * https://serde.rs/enum-representations.html#untagged * https://serde.rs/field-attrs.html#skip * https://www.postgresql.org/docs/current/pgstatstatements.html * https://www.postgresql.org/docs/current/runtime-config-statistics.html#GUC-COMPUTE-QUERY-ID * fix(postgres): regression of launchbadge#1449 ``` 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 ``` * refactor(postgres): don't define unused fields in QueryPlan * refactor(postgres): simplify query plan handling, add unit test * chore: document why we load `pg_stat_statements` in tests --------- Co-authored-by: mrl5 <[email protected]>
1 parent a8a0579 commit 487b89a

File tree

2 files changed

+89
-16
lines changed

2 files changed

+89
-16
lines changed

sqlx-postgres/src/connection/describe.rs

+86-15
Original file line numberDiff line numberDiff line change
@@ -451,15 +451,16 @@ WHERE rngtypid = $1
451451

452452
let mut nullables = Vec::new();
453453

454-
if let Explain::Plan(
455-
plan @ Plan {
456-
output: Some(outputs),
457-
..
458-
},
459-
) = &explain
454+
if let Explain::Plan {
455+
plan:
456+
plan @ Plan {
457+
output: Some(ref outputs),
458+
..
459+
},
460+
} = &explain
460461
{
461462
nullables.resize(outputs.len(), None);
462-
visit_plan(&plan, outputs, &mut nullables);
463+
visit_plan(plan, outputs, &mut nullables);
463464
}
464465

465466
Ok(nullables)
@@ -491,17 +492,30 @@ fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>
491492
}
492493
}
493494

494-
#[derive(serde::Deserialize)]
495+
#[derive(serde::Deserialize, Debug)]
496+
#[serde(untagged)]
495497
enum Explain {
496-
/// {"Plan": ...} -- returned for most statements
497-
Plan(Plan),
498-
/// The string "Utility Statement" -- returned for
499-
/// a CALL statement
500-
#[serde(rename = "Utility Statement")]
501-
UtilityStatement,
498+
// NOTE: the returned JSON may not contain a `plan` field, for example, with `CALL` statements:
499+
// https://github.com/launchbadge/sqlx/issues/1449
500+
//
501+
// In this case, we should just fall back to assuming all is nullable.
502+
//
503+
// It may also contain additional fields we don't care about, which should not break parsing:
504+
// https://github.com/launchbadge/sqlx/issues/2587
505+
// https://github.com/launchbadge/sqlx/issues/2622
506+
Plan {
507+
#[serde(rename = "Plan")]
508+
plan: Plan,
509+
},
510+
511+
// This ensures that parsing never technically fails.
512+
//
513+
// We don't want to specifically expect `"Utility Statement"` because there might be other cases
514+
// and we don't care unless it contains a query plan anyway.
515+
Other(serde::de::IgnoredAny),
502516
}
503517

504-
#[derive(serde::Deserialize)]
518+
#[derive(serde::Deserialize, Debug)]
505519
struct Plan {
506520
#[serde(rename = "Join Type")]
507521
join_type: Option<String>,
@@ -512,3 +526,60 @@ struct Plan {
512526
#[serde(rename = "Plans")]
513527
plans: Option<Vec<Plan>>,
514528
}
529+
530+
#[test]
531+
fn explain_parsing() {
532+
let normal_plan = r#"[
533+
{
534+
"Plan": {
535+
"Node Type": "Result",
536+
"Parallel Aware": false,
537+
"Async Capable": false,
538+
"Startup Cost": 0.00,
539+
"Total Cost": 0.01,
540+
"Plan Rows": 1,
541+
"Plan Width": 4,
542+
"Output": ["1"]
543+
}
544+
}
545+
]"#;
546+
547+
// https://github.com/launchbadge/sqlx/issues/2622
548+
let extra_field = r#"[
549+
{
550+
"Plan": {
551+
"Node Type": "Result",
552+
"Parallel Aware": false,
553+
"Async Capable": false,
554+
"Startup Cost": 0.00,
555+
"Total Cost": 0.01,
556+
"Plan Rows": 1,
557+
"Plan Width": 4,
558+
"Output": ["1"]
559+
},
560+
"Query Identifier": 1147616880456321454
561+
}
562+
]"#;
563+
564+
// https://github.com/launchbadge/sqlx/issues/1449
565+
let utility_statement = r#"["Utility Statement"]"#;
566+
567+
let normal_plan_parsed = serde_json::from_str::<[Explain; 1]>(normal_plan).unwrap();
568+
let extra_field_parsed = serde_json::from_str::<[Explain; 1]>(extra_field).unwrap();
569+
let utility_statement_parsed = serde_json::from_str::<[Explain; 1]>(utility_statement).unwrap();
570+
571+
assert!(
572+
matches!(normal_plan_parsed, [Explain::Plan { plan: Plan { .. } }]),
573+
"unexpected parse from {normal_plan:?}: {normal_plan_parsed:?}"
574+
);
575+
576+
assert!(
577+
matches!(extra_field_parsed, [Explain::Plan { plan: Plan { .. } }]),
578+
"unexpected parse from {extra_field:?}: {extra_field_parsed:?}"
579+
);
580+
581+
assert!(
582+
matches!(utility_statement_parsed, [Explain::Other(_)]),
583+
"unexpected parse from {utility_statement:?}: {utility_statement_parsed:?}"
584+
)
585+
}

tests/docker-compose.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,10 @@ services:
182182
POSTGRES_INITDB_ARGS: --auth-host=scram-sha-256
183183
volumes:
184184
- "./postgres/setup.sql:/docker-entrypoint-initdb.d/setup.sql"
185+
# Loading `pg_stat_statements` should serve as a regression test for:
186+
# https://github.com/launchbadge/sqlx/issues/2622
185187
command: >
186-
-c ssl=on -c ssl_cert_file=/var/lib/postgresql/server.crt -c ssl_key_file=/var/lib/postgresql/server.key
188+
-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
187189
188190
postgres_15_client_ssl:
189191
build:

0 commit comments

Comments
 (0)