From 2ceef2f2162abf56f71410de63a5605178448322 Mon Sep 17 00:00:00 2001 From: Ibrahim Halatci Date: Wed, 2 Sep 2026 12:24:22 +0000 Subject: [PATCH] Make the EXTRACT deparse injection test detect injection in any schema The injection check looked for the table the payload creates with to_regclass('.injected'), but the deparser fully qualifies task SQL on purpose (PushEmptySearchPath), so Citus never sets search_path on a worker for a SELECT task. A successful injection therefore creates the table in the worker's default search_path, not in the test schema, and the assertion would still report success. Look the relation up by name in pg_class instead, which is how the other run_command_on_workers checks in the suite do it, so the test fails wherever the injected table lands. Also add a positive control. The payload is expected to raise invalid_parameter_value and the DO block swallows it, so the only assertion was a negative one: if a future change stopped the expression from being pushed down, nothing would run on a worker and the test would keep passing without covering the deparse path. Asserting that EXTRACT with a valid field still returns the right value pins that path. This also gives the file real coverage on PG19, where the injection block is skipped in favour of pg19.sql. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518 --- src/test/regress/expected/extract_deparse.out | 15 +++++++++++++-- src/test/regress/expected/pg19.out | 15 +++++++++++++-- src/test/regress/sql/extract_deparse.sql | 11 +++++++++-- src/test/regress/sql/pg19.sql | 11 +++++++++-- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/test/regress/expected/extract_deparse.out b/src/test/regress/expected/extract_deparse.out index 8698d0bbdc4..c8d26470386 100644 --- a/src/test/regress/expected/extract_deparse.out +++ b/src/test/regress/expected/extract_deparse.out @@ -26,9 +26,20 @@ EXCEPTION WHEN invalid_parameter_value THEN NULL; END $$; -SELECT bool_and(result::boolean) AS extract_field_injection_blocked +-- Positive control: the same expression with a valid field must still be +-- pushed down, so the check below cannot pass merely because the expression +-- stopped reaching the workers. +SELECT EXTRACT('year' FROM ts) = 2026 AS extract_field_pushdown_works +FROM extract_deparse_source +WHERE id = 1; + extract_field_pushdown_works +--------------------------------------------------------------------- + t +(1 row) + +SELECT bool_and(result::int = 0) AS extract_field_injection_blocked FROM run_command_on_workers($$ - SELECT to_regclass('extract_deparse.injected') IS NULL + SELECT count(*) FROM pg_class WHERE relname = 'injected' $$); extract_field_injection_blocked --------------------------------------------------------------------- diff --git a/src/test/regress/expected/pg19.out b/src/test/regress/expected/pg19.out index 5fca06f1c91..de3ca4aa83e 100644 --- a/src/test/regress/expected/pg19.out +++ b/src/test/regress/expected/pg19.out @@ -45,9 +45,20 @@ EXCEPTION WHEN invalid_parameter_value THEN NULL; END $$; -SELECT bool_and(result::boolean) AS extract_field_injection_blocked +-- Positive control: the same expression with a valid field must still be +-- pushed down, so the check below cannot pass merely because the expression +-- stopped reaching the workers. +SELECT EXTRACT('year' FROM ts) = 2026 AS extract_field_pushdown_works +FROM extract_deparse_source +WHERE id = 1; + extract_field_pushdown_works +--------------------------------------------------------------------- + t +(1 row) + +SELECT bool_and(result::int = 0) AS extract_field_injection_blocked FROM run_command_on_workers($$ - SELECT to_regclass('pg19_repack.injected') IS NULL + SELECT count(*) FROM pg_class WHERE relname = 'injected' $$); extract_field_injection_blocked --------------------------------------------------------------------- diff --git a/src/test/regress/sql/extract_deparse.sql b/src/test/regress/sql/extract_deparse.sql index 2aab2d53012..39e0b2d5eba 100644 --- a/src/test/regress/sql/extract_deparse.sql +++ b/src/test/regress/sql/extract_deparse.sql @@ -25,9 +25,16 @@ EXCEPTION END $$; -SELECT bool_and(result::boolean) AS extract_field_injection_blocked +-- Positive control: the same expression with a valid field must still be +-- pushed down, so the check below cannot pass merely because the expression +-- stopped reaching the workers. +SELECT EXTRACT('year' FROM ts) = 2026 AS extract_field_pushdown_works +FROM extract_deparse_source +WHERE id = 1; + +SELECT bool_and(result::int = 0) AS extract_field_injection_blocked FROM run_command_on_workers($$ - SELECT to_regclass('extract_deparse.injected') IS NULL + SELECT count(*) FROM pg_class WHERE relname = 'injected' $$); SET client_min_messages TO ERROR; diff --git a/src/test/regress/sql/pg19.sql b/src/test/regress/sql/pg19.sql index f691cf8a558..0dd5bec7c77 100644 --- a/src/test/regress/sql/pg19.sql +++ b/src/test/regress/sql/pg19.sql @@ -46,9 +46,16 @@ EXCEPTION END $$; -SELECT bool_and(result::boolean) AS extract_field_injection_blocked +-- Positive control: the same expression with a valid field must still be +-- pushed down, so the check below cannot pass merely because the expression +-- stopped reaching the workers. +SELECT EXTRACT('year' FROM ts) = 2026 AS extract_field_pushdown_works +FROM extract_deparse_source +WHERE id = 1; + +SELECT bool_and(result::int = 0) AS extract_field_injection_blocked FROM run_command_on_workers($$ - SELECT to_regclass('pg19_repack.injected') IS NULL + SELECT count(*) FROM pg_class WHERE relname = 'injected' $$); SET citus.shard_count TO 4;