From 9f1f18b364ab00accea34406cd8bc3d1ad62db61 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Wed, 16 Sep 2026 16:08:26 -0400 Subject: [PATCH] fix: fall back to proisagg/proiswindow on PostgreSQL < 11 in get_routines (#88) get_routines hardcoded the PG 11+ query that references pg_proc.prokind unconditionally. prokind was introduced in PostgreSQL 11, replacing the boolean columns proisagg/proiswindow; on 9.x/10, referencing it fails at parse time (SQLSTATE 42703, "column \"prokind\" does not exist"). The builtin driver queries current_setting('server_version_num') first and branches on >= 110000; this repo's version had no such check. Ported the version branch exactly, extracting the query-selection logic into a pure routine_query_for_version(server_version_num) function so it's unit-testable without a live server. TDD: added metadata_tests.rs (4 tests covering both branches and the exact 110000 boundary). Confirmed the new pre-PG11 tests fail against a simulated pre-fix implementation (the old hardcoded modern-only query) before restoring the real fix. Verified live against a real PostgreSQL 10.21 instance (podman postgres:10): confirmed "prokind" genuinely does not exist on that server (reproducing the exact SQLSTATE 42703 error from the issue), then confirmed the pre-fix binary fails with that identical error against the real PG10 server while the post-fix binary succeeds -- correctly listing a real function as FUNCTION and correctly EXCLUDING a real aggregate (via the legacy proisagg filter), which is the one part of this fix impossible to verify without an actual pre-PG11 server. Also verified the modern branch still works unchanged against the existing PG16 live-test database. --- src/handlers/metadata.rs | 57 ++++++++++++++++++++++++++++------ src/handlers/metadata_tests.rs | 56 +++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 src/handlers/metadata_tests.rs diff --git a/src/handlers/metadata.rs b/src/handlers/metadata.rs index 7e4bed1..d94eda7 100644 --- a/src/handlers/metadata.rs +++ b/src/handlers/metadata.rs @@ -654,15 +654,21 @@ pub async fn get_routines(id: Value, params: &Value) -> Value { .and_then(Value::as_str) .unwrap_or("public"); - // PG 11+ uses prokind; older versions use proisagg/proiswindow flags. - // CI runs PG 16, so we use the modern query. - let query = r#" - SELECT proname, prokind - FROM pg_proc - WHERE pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = $1) - AND prokind IN ('f', 'p') - ORDER BY proname - "#; + let server_version_num: i32 = match client::query_rows( + &conn_params, + "SELECT current_setting('server_version_num')::int4 AS v", + &[], + ) + .await + { + Ok(rows) => rows + .first() + .and_then(|r| r.try_get::<_, i32>("v").ok()) + .unwrap_or(0), + Err(e) => return error_response(id, -32603, &e), + }; + + let query = routine_query_for_version(server_version_num); match client::query_rows(&conn_params, query, &[&schema]).await { Ok(rows) => { @@ -689,6 +695,35 @@ pub async fn get_routines(id: Value, params: &Value) -> Value { } } +/// Pick the `get_routines` query by server version. `pg_proc.prokind` was +/// introduced in PostgreSQL 11, replacing the boolean columns `proisagg` / +/// `proiswindow`. On 9.x/10 the column does not exist, so referencing it +/// fails at parse time (SQLSTATE 42703). Matches the builtin driver's +/// `get_routines` version branch exactly (`src-tauri/src/drivers/postgres/mod.rs`). +fn routine_query_for_version(server_version_num: i32) -> &'static str { + if server_version_num >= 110_000 { + r#" + SELECT proname, prokind + FROM pg_proc + WHERE pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = $1) + AND prokind IN ('f', 'p') + ORDER BY proname + "# + } else { + // Pre-11: procedures don't exist; exclude aggregates and window + // functions and report everything else as a plain function ('f'). + // Cast to the internal "char" type so it maps to i8 like prokind. + r#" + SELECT proname, 'f'::"char" AS prokind + FROM pg_proc + WHERE pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = $1) + AND NOT proisagg + AND NOT proiswindow + ORDER BY proname + "# + } +} + pub async fn get_routine_parameters(id: Value, params: &Value) -> Value { let conn_params = ConnectionParams::from_value(inner_params(params)); let routine_name = params @@ -923,3 +958,7 @@ pub async fn get_all_columns_batch(id: Value, _params: &Value) -> Value { pub async fn get_all_foreign_keys_batch(id: Value, _params: &Value) -> Value { not_implemented(id, "get_all_foreign_keys_batch") } + +#[cfg(test)] +#[path = "metadata_tests.rs"] +mod metadata_tests; diff --git a/src/handlers/metadata_tests.rs b/src/handlers/metadata_tests.rs new file mode 100644 index 0000000..9378fc1 --- /dev/null +++ b/src/handlers/metadata_tests.rs @@ -0,0 +1,56 @@ +//! Unit tests for `metadata.rs`'s pure query-selection helper +//! (`routine_query_for_version`). Sibling test file per repo convention +//! (`.rules/rust.md` #4/#5) — loaded via +//! `#[cfg(test)] #[path = "metadata_tests.rs"] mod metadata_tests;`. + +use super::routine_query_for_version; + +#[test] +fn pg11_and_newer_uses_the_prokind_query() { + for version in [110_000, 110_001, 120_000, 160_000] { + let query = routine_query_for_version(version); + assert!( + query.contains("prokind"), + "version {version} should use the prokind query, got: {query}" + ); + assert!( + !query.contains("proisagg"), + "version {version} should not use the legacy proisagg/proiswindow query" + ); + } +} + +#[test] +fn pre_pg11_falls_back_to_proisagg_proiswindow_query() { + for version in [0, 90_600, 100_000, 100_015, 109_999] { + let query = routine_query_for_version(version); + assert!( + query.contains("proisagg") && query.contains("proiswindow"), + "version {version} should use the legacy proisagg/proiswindow query, got: {query}" + ); + assert!( + !query.contains("prokind IN"), + "version {version} must not reference the prokind column, which doesn't exist \ + before PostgreSQL 11 (SQLSTATE 42703)" + ); + } +} + +#[test] +fn boundary_is_exactly_110000_inclusive() { + // 110000 is PostgreSQL 11.0's server_version_num -- the exact version + // that introduced prokind, so it must take the modern branch. + assert!(routine_query_for_version(110_000).contains("prokind IN")); + // One below must take the legacy branch. + assert!(routine_query_for_version(109_999).contains("proisagg")); +} + +#[test] +fn every_branch_selects_and_aliases_prokind_as_a_char_column() { + // Both queries must produce a `prokind` column the handler can + // try_get::() uniformly, regardless of which branch ran. + for version in [90_600, 160_000] { + let query = routine_query_for_version(version); + assert!(query.contains("prokind"), "{query}"); + } +}