Commit Graph

3 Commits (c5dde4b1154ce140a59a2c2f19be8a41c68e3f81)

Author SHA1 Message Date
Naisila Puka c5dde4b115
Fix crash on create statistics with non-RangeVar type pt2 (#8227)
Fixes #8225 
very similar to #8213 
Also the error message changed between pg18rc1 and pg18.0
2025-10-07 11:56:20 +03:00
Naisila Puka bb840e58a7
Fix crash on create statistics with non-RangeVar type (#8213)
This crash has been there for a while but wasn't tested before pg18.

PG18 added this test:
CREATE STATISTICS tst ON a FROM (VALUES (x)) AS foo;

which tries to create statistics on a derived-on-the-fly table (which is
not allowed) However Citus assumes we always have a valid table when
intercepting CREATE STATISTICS command to check for Citus tables
Added a check to return early if needed.

pg18 commit: https://github.com/postgres/postgres/commit/3eea4dc2c

Fixes #8212
2025-10-01 00:09:11 +03:00
Mehmet YILMAZ 10d62d50ea
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.

---
2025-09-22 15:50:32 +03:00