Skip to content

Commit

Permalink
fix: ignore extra fields in Postgres describe parsing (#2670)
Browse files Browse the repository at this point in the history
* fix(postgres): sqlx prepare fails if shared_preload_libraries=pg_stat_statements

closes #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 #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 <31549762+mrl5@users.noreply.github.com>
  • Loading branch information
abonander and mrl5 committed Jul 31, 2023
1 parent a8a0579 commit 487b89a
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 16 deletions.
101 changes: 86 additions & 15 deletions sqlx-postgres/src/connection/describe.rs
Expand Up @@ -451,15 +451,16 @@ WHERE rngtypid = $1

let mut nullables = Vec::new();

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

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

#[derive(serde::Deserialize)]
#[derive(serde::Deserialize, Debug)]
#[serde(untagged)]
enum Explain {
/// {"Plan": ...} -- returned for most statements
Plan(Plan),
/// The string "Utility Statement" -- returned for
/// a CALL statement
#[serde(rename = "Utility Statement")]
UtilityStatement,
// NOTE: the returned JSON may not contain a `plan` field, for example, with `CALL` statements:
// https://github.com/launchbadge/sqlx/issues/1449
//
// In this case, we should just fall back to assuming all is nullable.
//
// It may also contain additional fields we don't care about, which should not break parsing:
// https://github.com/launchbadge/sqlx/issues/2587
// https://github.com/launchbadge/sqlx/issues/2622
Plan {
#[serde(rename = "Plan")]
plan: Plan,
},

// This ensures that parsing never technically fails.
//
// We don't want to specifically expect `"Utility Statement"` because there might be other cases
// and we don't care unless it contains a query plan anyway.
Other(serde::de::IgnoredAny),
}

#[derive(serde::Deserialize)]
#[derive(serde::Deserialize, Debug)]
struct Plan {
#[serde(rename = "Join Type")]
join_type: Option<String>,
Expand All @@ -512,3 +526,60 @@ struct Plan {
#[serde(rename = "Plans")]
plans: Option<Vec<Plan>>,
}

#[test]
fn explain_parsing() {
let normal_plan = r#"[
{
"Plan": {
"Node Type": "Result",
"Parallel Aware": false,
"Async Capable": false,
"Startup Cost": 0.00,
"Total Cost": 0.01,
"Plan Rows": 1,
"Plan Width": 4,
"Output": ["1"]
}
}
]"#;

// https://github.com/launchbadge/sqlx/issues/2622
let extra_field = r#"[
{
"Plan": {
"Node Type": "Result",
"Parallel Aware": false,
"Async Capable": false,
"Startup Cost": 0.00,
"Total Cost": 0.01,
"Plan Rows": 1,
"Plan Width": 4,
"Output": ["1"]
},
"Query Identifier": 1147616880456321454
}
]"#;

// https://github.com/launchbadge/sqlx/issues/1449
let utility_statement = r#"["Utility Statement"]"#;

let normal_plan_parsed = serde_json::from_str::<[Explain; 1]>(normal_plan).unwrap();
let extra_field_parsed = serde_json::from_str::<[Explain; 1]>(extra_field).unwrap();
let utility_statement_parsed = serde_json::from_str::<[Explain; 1]>(utility_statement).unwrap();

assert!(
matches!(normal_plan_parsed, [Explain::Plan { plan: Plan { .. } }]),
"unexpected parse from {normal_plan:?}: {normal_plan_parsed:?}"
);

assert!(
matches!(extra_field_parsed, [Explain::Plan { plan: Plan { .. } }]),
"unexpected parse from {extra_field:?}: {extra_field_parsed:?}"
);

assert!(
matches!(utility_statement_parsed, [Explain::Other(_)]),
"unexpected parse from {utility_statement:?}: {utility_statement_parsed:?}"
)
}
4 changes: 3 additions & 1 deletion tests/docker-compose.yml
Expand Up @@ -182,8 +182,10 @@ services:
POSTGRES_INITDB_ARGS: --auth-host=scram-sha-256
volumes:
- "./postgres/setup.sql:/docker-entrypoint-initdb.d/setup.sql"
# Loading `pg_stat_statements` should serve as a regression test for:
# https://github.com/launchbadge/sqlx/issues/2622
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
postgres_15_client_ssl:
build:
Expand Down

0 comments on commit 487b89a

Please sign in to comment.