Stabilize table_checks across PG15–PG18: switch to pg_constraint, remove dupes, exclude NOT NULL (#8140)

DESCRIPTION: Stabilize table_checks across PG15–PG18: switch to
pg_constraint, remove dupes, exclude NOT NUL

fixes #8138
fixes #8131 

**Problem**

```diff
diff -dU10 -w /__w/citus/citus/src/test/regress/expected/multi_create_table_constraints.out /__w/citus/citus/src/test/regress/results/multi_create_table_constraints.out
--- /__w/citus/citus/src/test/regress/expected/multi_create_table_constraints.out.modified	2025-08-18 12:26:51.991598284 +0000
+++ /__w/citus/citus/src/test/regress/results/multi_create_table_constraints.out.modified	2025-08-18 12:26:52.004598519 +0000
@@ -403,22 +403,30 @@
     relid = 'check_example_partition_col_key_365068'::regclass;
     Column     |  Type   |  Definition   
 ---------------+---------+---------------
  partition_col | integer | partition_col
 (1 row)
 
 SELECT "Constraint", "Definition" FROM table_checks WHERE relid='public.check_example_365068'::regclass;
              Constraint              |            Definition             
 -------------------------------------+-----------------------------------
  check_example_other_col_check       | CHECK other_col >= 100
+ check_example_other_col_check       | CHECK other_col >= 100
+ check_example_other_col_check       | CHECK other_col >= 100
+ check_example_other_col_check       | CHECK other_col >= 100
+ check_example_other_col_check       | CHECK other_col >= 100
  check_example_other_other_col_check | CHECK abs(other_other_col) >= 100
-(2 rows)
+ check_example_other_other_col_check | CHECK abs(other_other_col) >= 100
+ check_example_other_other_col_check | CHECK abs(other_other_col) >= 100
+ check_example_other_other_col_check | CHECK abs(other_other_col) >= 100
+ check_example_other_other_col_check | CHECK abs(other_other_col) >= 100
+(10 rows)
```

On PostgreSQL 18, `NOT NULL` is represented as a cataloged constraint
and surfaces through `information_schema.check_constraints`.
14e87ffa5c
Our helper view `table_checks` (built on
`information_schema.check_constraints` + `constraint_column_usage`)
started returning:

* Extra `…_not_null` rows (noise for our tests)
* Duplicate rows for real CHECKs due to the one-to-many join via
`constraint_column_usage`
* Occasional literal formatting differences (e.g., dates) coming from
the information\_schema deparser

### What changed

1. **Rewrite `table_checks` to use system catalogs directly**
We now select only expression-based, table-level constraints—excluding
NOT NULL—by filtering on `contype <> 'n'` and requiring `conbin IS NOT
NULL`. This yields the same effective set as real CHECKs while remaining
future-proof against non-CHECK constraint types.

```sql
CREATE OR REPLACE VIEW table_checks AS
SELECT
  c.conname AS "Constraint",
  'CHECK ' ||
  -- drop a single pair of outer parens if the deparser adds them
  regexp_replace(pg_get_expr(c.conbin, c.conrelid, true), '^\((.*)\)$', '\1')
    AS "Definition",
  c.conrelid AS relid
FROM pg_catalog.pg_constraint AS c
WHERE c.contype <> 'n'         -- drop NOT NULL (PG18)
  AND c.conbin IS NOT NULL     -- only expression-bearing constraints (i.e., CHECKs)
  AND c.conrelid <> 0          -- table-level only (exclude domains)
ORDER BY "Constraint", "Definition";
```

Why this filter?

* `contype <> 'n'` excludes PG18’s NOT NULL rows.
* `conbin IS NOT NULL` restricts to expression-backed constraints
(CHECKs); PK/UNIQUE/FK/EXCLUSION don’t have `conbin`.
* `conrelid <> 0` removes domain constraints.

2. **Add a PG18-specific regression test for `contype = 'n'`**
   New test (`pg18_not_null_constraints`) verifies:

* Coordinator tables have `n` rows for NOT NULL (columns `a`, `c`),
* A worker shard has matching `n` rows,
* Dropping a NOT NULL on the coordinator propagates to shards (count
goes from 2 → 1),
* `table_checks` *never* reports NOT NULL, but does report a real CHECK
added for the test.

---

### Why this works (PG15–PG18)

* **Stable source of truth:** Directly reads `pg_constraint` instead of
`information_schema`.
* **No duplicates:** Eliminates the `constraint_column_usage` join,
removing multiplicity.
* **No NOT NULL noise:** PG18’s `contype = 'n'` is filtered out by
design.
* **Deterministic text:** Uses `pg_get_expr` and strips a single outer
set of parentheses for consistent output.

---

### Impact on tests

* Removes spurious `…_not_null` entries and duplicate `checky_…` rows
(e.g., in `multi_name_lengths` and similar).
* Existing expected files stabilize without adding brittle
normalizations.
* New PG18 test asserts correct catalog behavior and Citus propagation
while remaining a no-op on earlier PG versions.

---
pull/8210/head
Mehmet YILMAZ 2025-09-22 15:50:32 +03:00 committed by GitHub
parent b4cb1a94e9
commit 10d62d50ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 328 additions and 18 deletions

View File

@ -70,15 +70,19 @@ SELECT "name" AS "Column",
"relid" "relid"
FROM table_attrs; FROM table_attrs;
CREATE VIEW table_checks AS CREATE OR REPLACE VIEW table_checks AS
SELECT cc.constraint_name AS "Constraint", SELECT
('CHECK ' || regexp_replace(check_clause, '^\((.*)\)$', '\1')) AS "Definition", c.conname AS "Constraint",
format('%I.%I', ccu.table_schema, ccu.table_name)::regclass::oid AS relid 'CHECK ' ||
FROM information_schema.check_constraints cc, -- drop a single pair of outer parens if the deparser adds them
information_schema.constraint_column_usage ccu regexp_replace(pg_get_expr(c.conbin, c.conrelid, true), '^\((.*)\)$', '\1')
WHERE cc.constraint_schema = ccu.constraint_schema AND AS "Definition",
cc.constraint_name = ccu.constraint_name c.conrelid AS relid
ORDER BY cc.constraint_name ASC; FROM pg_catalog.pg_constraint AS c
WHERE c.contype <> 'n' -- drop NOT NULL
AND c.conbin IS NOT NULL -- only things with an expression (i.e., CHECKs)
AND c.conrelid <> 0 -- table-level (exclude domain checks)
ORDER BY "Constraint", "Definition";
CREATE VIEW index_attrs AS CREATE VIEW index_attrs AS
WITH indexoid AS ( WITH indexoid AS (

View File

@ -0,0 +1,163 @@
--
-- PG18
--
SHOW server_version \gset
SELECT substring(:'server_version', '\d+')::int >= 18 AS server_version_ge_18
\gset
\if :server_version_ge_18
\else
\q
\endif
-- PG17-specific tests go here.
--
-- Purpose: Verify PG18 behavior that NOT NULL constraints are materialized
-- as pg_constraint rows with contype = 'n' on both coordinator and
-- worker shards. Also confirm our helper view (table_checks) does
-- NOT surface NOT NULL entries.
-- https://github.com/postgres/postgres/commit/14e87ffa5c543b5f30ead7413084c25f7735039f
CREATE SCHEMA pg18_nn;
SET search_path TO pg18_nn;
-- Local control table
DROP TABLE IF EXISTS nn_local CASCADE;
NOTICE: table "nn_local" does not exist, skipping
CREATE TABLE nn_local(
a int NOT NULL,
b int,
c text NOT NULL
);
-- Distributed table
DROP TABLE IF EXISTS nn_dist CASCADE;
NOTICE: table "nn_dist" does not exist, skipping
CREATE TABLE nn_dist(
a int NOT NULL,
b int,
c text NOT NULL
);
SELECT create_distributed_table('nn_dist', 'a');
create_distributed_table
---------------------------------------------------------------------
(1 row)
-- Coordinator: count NOT NULL constraint rows
SELECT 'local_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_local'::regclass
GROUP BY contype
ORDER BY contype;
label | contype | count
---------------------------------------------------------------------
local_n_count | n | 2
(1 row)
SELECT 'dist_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_dist'::regclass
GROUP BY contype
ORDER BY contype;
label | contype | count
---------------------------------------------------------------------
dist_n_count | n | 2
(1 row)
-- Our helper view should exclude NOT NULL
SELECT 'table_checks_local_count' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_local'::regclass;
label | count
---------------------------------------------------------------------
table_checks_local_count | 0
(1 row)
SELECT 'table_checks_dist_count' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_dist'::regclass;
label | count
---------------------------------------------------------------------
table_checks_dist_count | 0
(1 row)
-- Add a real CHECK to ensure table_checks still reports real checks
ALTER TABLE nn_dist ADD CONSTRAINT nn_dist_check CHECK (b IS DISTINCT FROM 42);
SELECT 'table_checks_dist_with_real_check' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_dist'::regclass;
label | count
---------------------------------------------------------------------
table_checks_dist_with_real_check | 1
(1 row)
-- === Worker checks ===
\c - - - :worker_1_port
SET client_min_messages TO WARNING;
SET search_path TO pg18_nn;
-- Pick one heap shard of nn_dist in our schema
SELECT format('%I.%I', n.nspname, c.relname) AS shard_regclass
FROM pg_class c
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE n.nspname = 'pg18_nn'
AND c.relname LIKE 'nn_dist_%'
AND c.relkind = 'r'
ORDER BY c.relname
LIMIT 1
\gset
-- Expect: 2 NOT NULL rows (a,c) + 1 CHECK row on the shard
SELECT 'worker_shard_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = :'shard_regclass'::regclass
GROUP BY contype
ORDER BY contype;
label | contype | count
---------------------------------------------------------------------
worker_shard_n_count | c | 1
worker_shard_n_count | n | 2
(2 rows)
-- table_checks on shard should hide NOT NULL
SELECT 'table_checks_worker_shard_count' AS label, count(*)
FROM public.table_checks
WHERE relid = :'shard_regclass'::regclass;
label | count
---------------------------------------------------------------------
table_checks_worker_shard_count | 1
(1 row)
-- Drop one NOT NULL on coordinator; verify propagation
\c - - - :master_port
SET search_path TO pg18_nn;
ALTER TABLE nn_dist ALTER COLUMN c DROP NOT NULL;
-- Re-check on worker: NOT NULL count should drop to 1
\c - - - :worker_1_port
SET search_path TO pg18_nn;
SELECT 'worker_shard_n_after_drop' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = :'shard_regclass'::regclass
GROUP BY contype
ORDER BY contype;
label | contype | count
---------------------------------------------------------------------
worker_shard_n_after_drop | c | 1
worker_shard_n_after_drop | n | 1
(2 rows)
-- And on coordinator
\c - - - :master_port
SET search_path TO pg18_nn;
SELECT 'dist_n_after_drop' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_dist'::regclass
GROUP BY contype
ORDER BY contype;
label | contype | count
---------------------------------------------------------------------
dist_n_after_drop | c | 1
dist_n_after_drop | n | 1
(2 rows)
-- cleanup
RESET client_min_messages;
RESET search_path;
DROP SCHEMA pg18_nn CASCADE;
NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to table pg18_nn.nn_local
drop cascades to table pg18_nn.nn_dist

View File

@ -0,0 +1,9 @@
--
-- PG18
--
SHOW server_version \gset
SELECT substring(:'server_version', '\d+')::int >= 18 AS server_version_ge_18
\gset
\if :server_version_ge_18
\else
\q

View File

@ -68,6 +68,7 @@ test: pg14
test: pg15 test: pg15
test: pg15_jsonpath detect_conn_close test: pg15_jsonpath detect_conn_close
test: pg17 pg17_json test: pg17 pg17_json
test: pg18
test: drop_column_partitioned_table test: drop_column_partitioned_table
test: tableam test: tableam

View File

@ -71,15 +71,19 @@ SELECT "name" AS "Column",
"relid" "relid"
FROM table_attrs; FROM table_attrs;
CREATE VIEW table_checks AS CREATE OR REPLACE VIEW table_checks AS
SELECT cc.constraint_name AS "Constraint", SELECT
('CHECK ' || regexp_replace(check_clause, '^\((.*)\)$', '\1')) AS "Definition", c.conname AS "Constraint",
format('%I.%I', ccu.table_schema, ccu.table_name)::regclass::oid AS relid 'CHECK ' ||
FROM information_schema.check_constraints cc, -- drop a single pair of outer parens if the deparser adds them
information_schema.constraint_column_usage ccu regexp_replace(pg_get_expr(c.conbin, c.conrelid, true), '^\((.*)\)$', '\1')
WHERE cc.constraint_schema = ccu.constraint_schema AND AS "Definition",
cc.constraint_name = ccu.constraint_name c.conrelid AS relid
ORDER BY cc.constraint_name ASC; FROM pg_catalog.pg_constraint AS c
WHERE c.contype <> 'n' -- drop NOT NULL
AND c.conbin IS NOT NULL -- only things with an expression (i.e., CHECKs)
AND c.conrelid <> 0 -- table-level (exclude domain checks)
ORDER BY "Constraint", "Definition";
CREATE VIEW index_attrs AS CREATE VIEW index_attrs AS
WITH indexoid AS ( WITH indexoid AS (

View File

@ -0,0 +1,129 @@
--
-- PG18
--
SHOW server_version \gset
SELECT substring(:'server_version', '\d+')::int >= 18 AS server_version_ge_18
\gset
\if :server_version_ge_18
\else
\q
\endif
-- PG17-specific tests go here.
--
-- Purpose: Verify PG18 behavior that NOT NULL constraints are materialized
-- as pg_constraint rows with contype = 'n' on both coordinator and
-- worker shards. Also confirm our helper view (table_checks) does
-- NOT surface NOT NULL entries.
-- https://github.com/postgres/postgres/commit/14e87ffa5c543b5f30ead7413084c25f7735039f
CREATE SCHEMA pg18_nn;
SET search_path TO pg18_nn;
-- Local control table
DROP TABLE IF EXISTS nn_local CASCADE;
CREATE TABLE nn_local(
a int NOT NULL,
b int,
c text NOT NULL
);
-- Distributed table
DROP TABLE IF EXISTS nn_dist CASCADE;
CREATE TABLE nn_dist(
a int NOT NULL,
b int,
c text NOT NULL
);
SELECT create_distributed_table('nn_dist', 'a');
-- Coordinator: count NOT NULL constraint rows
SELECT 'local_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_local'::regclass
GROUP BY contype
ORDER BY contype;
SELECT 'dist_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_dist'::regclass
GROUP BY contype
ORDER BY contype;
-- Our helper view should exclude NOT NULL
SELECT 'table_checks_local_count' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_local'::regclass;
SELECT 'table_checks_dist_count' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_dist'::regclass;
-- Add a real CHECK to ensure table_checks still reports real checks
ALTER TABLE nn_dist ADD CONSTRAINT nn_dist_check CHECK (b IS DISTINCT FROM 42);
SELECT 'table_checks_dist_with_real_check' AS label, count(*)
FROM public.table_checks
WHERE relid = 'pg18_nn.nn_dist'::regclass;
-- === Worker checks ===
\c - - - :worker_1_port
SET client_min_messages TO WARNING;
SET search_path TO pg18_nn;
-- Pick one heap shard of nn_dist in our schema
SELECT format('%I.%I', n.nspname, c.relname) AS shard_regclass
FROM pg_class c
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE n.nspname = 'pg18_nn'
AND c.relname LIKE 'nn_dist_%'
AND c.relkind = 'r'
ORDER BY c.relname
LIMIT 1
\gset
-- Expect: 2 NOT NULL rows (a,c) + 1 CHECK row on the shard
SELECT 'worker_shard_n_count' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = :'shard_regclass'::regclass
GROUP BY contype
ORDER BY contype;
-- table_checks on shard should hide NOT NULL
SELECT 'table_checks_worker_shard_count' AS label, count(*)
FROM public.table_checks
WHERE relid = :'shard_regclass'::regclass;
-- Drop one NOT NULL on coordinator; verify propagation
\c - - - :master_port
SET search_path TO pg18_nn;
ALTER TABLE nn_dist ALTER COLUMN c DROP NOT NULL;
-- Re-check on worker: NOT NULL count should drop to 1
\c - - - :worker_1_port
SET search_path TO pg18_nn;
SELECT 'worker_shard_n_after_drop' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = :'shard_regclass'::regclass
GROUP BY contype
ORDER BY contype;
-- And on coordinator
\c - - - :master_port
SET search_path TO pg18_nn;
SELECT 'dist_n_after_drop' AS label, contype, count(*)
FROM pg_constraint
WHERE conrelid = 'pg18_nn.nn_dist'::regclass
GROUP BY contype
ORDER BY contype;
-- cleanup
RESET client_min_messages;
RESET search_path;
DROP SCHEMA pg18_nn CASCADE;