From 9efa364eb0ddfc883d49e7017005d5fa605f747d Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Thu, 15 Dec 2022 07:12:55 -0500 Subject: [PATCH] Fix handling for null-returning PG queries (#521) Handle cases when a query returns a NULL or a table with no rows, or a single row with a null value in it. This fully fixes #519 in the main branch --- src/pg/pg_source.rs | 21 +++++++++++-------- src/pg/utils.rs | 4 ++-- tests/fixtures/functions/function_null.sql | 7 +++++++ .../fixtures/functions/function_null_row.sql | 6 ++++++ .../fixtures/functions/function_null_row2.sql | 6 ++++++ tests/server_test.rs | 17 +++++++++++++++ 6 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/functions/function_null.sql create mode 100644 tests/fixtures/functions/function_null_row.sql create mode 100644 tests/fixtures/functions/function_null_row2.sql diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs index c5d7d305..7a833f02 100644 --- a/src/pg/pg_source.rs +++ b/src/pg/pg_source.rs @@ -73,19 +73,22 @@ impl Source for PgSource { let json = query_to_json(url_query); debug!("SQL: {query} [{xyz}, {json:?}]"); let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json]; - conn.query_one(&prep_query, params).await + conn.query_opt(&prep_query, params).await } else { debug!("SQL: {query} [{xyz}]"); - conn.query_one(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await + conn.query_opt(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await }; - let tile = tile.map(|row| row.get(0)).map_err(|e| { - if self.info.has_query_params { - GetTileWithQueryError(e, self.id.to_string(), *xyz, url_query.clone()) - } else { - GetTileError(e, self.id.to_string(), *xyz) - } - })?; + let tile = tile + .map(|row| row.map_or(Default::default(), |r| r.get::<_, Option>(0))) + .map_err(|e| { + if self.info.has_query_params { + GetTileWithQueryError(e, self.id.to_string(), *xyz, url_query.clone()) + } else { + GetTileError(e, self.id.to_string(), *xyz) + } + })? + .unwrap_or_default(); Ok(tile) } diff --git a/src/pg/utils.rs b/src/pg/utils.rs index a11efd3c..899cd222 100755 --- a/src/pg/utils.rs +++ b/src/pg/utils.rs @@ -134,10 +134,10 @@ pub enum PgError { String, ), - #[error(r#"Unable to get tile {1}/{2:#}: {0}"#)] + #[error(r#"Unable to get tile {2:#} from {1}: {0}"#)] GetTileError(#[source] bb8_postgres::tokio_postgres::Error, String, Xyz), - #[error(r#"Unable to get tile {1}/{2:#} with {:?} params: {0}"#, query_to_json(.3))] + #[error(r#"Unable to get tile {2:#} with {:?} params from {1}: {0}"#, query_to_json(.3))] GetTileWithQueryError( #[source] bb8_postgres::tokio_postgres::Error, String, diff --git a/tests/fixtures/functions/function_null.sql b/tests/fixtures/functions/function_null.sql new file mode 100644 index 00000000..ed1bdf8d --- /dev/null +++ b/tests/fixtures/functions/function_null.sql @@ -0,0 +1,7 @@ +DROP FUNCTION IF EXISTS public.function_null; + +CREATE OR REPLACE FUNCTION public.function_null(z integer, x integer, y integer) RETURNS bytea AS $$ +BEGIN + RETURN null; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_null_row.sql b/tests/fixtures/functions/function_null_row.sql new file mode 100644 index 00000000..69a43037 --- /dev/null +++ b/tests/fixtures/functions/function_null_row.sql @@ -0,0 +1,6 @@ +DROP FUNCTION IF EXISTS public.function_null_row; + +CREATE OR REPLACE FUNCTION public.function_null_row(z integer, x integer, y integer) +RETURNS TABLE(mvt bytea, key text) AS $$ + SELECT NULL::bytea, NULL::text +$$ LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_null_row2.sql b/tests/fixtures/functions/function_null_row2.sql new file mode 100644 index 00000000..5704d76d --- /dev/null +++ b/tests/fixtures/functions/function_null_row2.sql @@ -0,0 +1,6 @@ +DROP FUNCTION IF EXISTS public.function_null_row2; + +CREATE OR REPLACE FUNCTION public.function_null_row2(z integer, x integer, y integer) +RETURNS TABLE(mvt bytea, key text) AS $$ + SELECT NULL::bytea, NULL::text WHERE FALSE; +$$ LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/server_test.rs b/tests/server_test.rs index 3b399c8c..f87ae093 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -316,6 +316,23 @@ async fn get_composite_source_tile_minmax_zoom_ok() { assert_eq!(response.status(), StatusCode::NOT_FOUND); } +#[actix_rt::test] +async fn null_functions() { + let app = create_app!(mock_empty_config()); + + let req = test_get("/function_null/0/0/0"); + let response = call_service(&app, req).await; + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + let req = test_get("/function_null_row/0/0/0"); + let response = call_service(&app, req).await; + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + let req = test_get("/function_null_row2/0/0/0"); + let response = call_service(&app, req).await; + assert_eq!(response.status(), StatusCode::NO_CONTENT); +} + #[actix_rt::test] async fn get_function_source_ok() { let app = create_app!(mock_empty_config());