From 7ac79e58c87aea1a4bd3e6b06207e1a6daf27bab Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 16 Oct 2026 05:01:05 +0000 Subject: [PATCH 1/3] fix: in the upgrade window, a refusal for a missing catalog table names ALTER EXTENSION UPDATE (#1341 item 66) Between installing a new library and running ALTER EXTENSION pgcolumnar UPDATE, a database runs the 1.0-alpha5 library over its 1.0-alpha4 catalog. The library deletes from pgcolumnar.retired_group, which that catalog lacks, so DROP TABLE, TRUNCATE and pgcolumnar.vacuum() on a columnar table fail there with 42P01 'columnar metadata table "pgcolumnar.retired_group" does not exist', which names neither the cause nor the remedy. Measured: each refusal changes nothing, reads and writes work throughout, and after the update all three succeed on the same rows. open_columnar_table's error now carries a hint naming the update. docs/installation.md's "If you skipped step 3" shows the error and the hint. Red first, both twins, PG18: native_upgrade_converge gains the window's arms in a database at the previous release's catalog (the newest fixture), and the three refusal arms fail on main with `got [42P01|no-hint]`. test_native_upgrade_converge.py is their partial twin, declared INCOMPLETE; it stages the previous release's base script from test/fixtures as the shell does. Green on 15-19 in both twins. cluster_tests 660 -> 661 by collection; TESTS.md gets section 143. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 ++ docs/installation.md | 12 ++ src/columnar_metadata.c | 13 +- test/native_upgrade_converge.sh | 49 ++++++++ test/pytest/TESTS.md | 11 ++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 4 + test/pytest/test_native_upgrade_converge.py | 126 ++++++++++++++++++++ 8 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 test/pytest/test_native_upgrade_converge.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d2b779a6d..c0dffb95f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -400,6 +400,18 @@ true until the next version shipped. Reported by @OffgridwithJD. ### Fixed +- **In the upgrade window, a refusal for a missing catalog table names `ALTER + EXTENSION UPDATE`.** After the new library is installed and the server restarted, + but before `ALTER EXTENSION pgcolumnar UPDATE`, `DROP TABLE`, `TRUNCATE` and + `pgcolumnar.vacuum()` on a columnar table fail, because the 1.0-alpha5 library + needs `pgcolumnar.retired_group` and a 1.0-alpha4 catalog lacks it. They change + nothing. The error used to name only the missing table, and it now carries a hint: + `The pgcolumnar catalog in this database may be older than the installed library. + Run ALTER EXTENSION pgcolumnar UPDATE in this database.` Reads and writes work + throughout, and after the update every one of these succeeds on the same rows + (#1341 item 66). `test/native_upgrade_converge.sh` and + `test_native_upgrade_converge.py`. + - **A join whose runtime-filter build is empty no longer reads the fact table.** When a large dimension filtered to no row was joined to a columnar table, the hash join diff --git a/docs/installation.md b/docs/installation.md index 2618c47bf..6cf867108 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -152,6 +152,18 @@ The fix is step 3. Run `ALTER EXTENSION pgcolumnar UPDATE;` in that database and the error goes away. **Your data is not affected.** The tables are intact and no conversion happens. Only the catalog entry is stale. +Between releases that add a catalog table, the new library can also need a table +the old catalog does not have. Upgrading from `1.0-alpha4` to `1.0-alpha5`, +`DROP TABLE`, `TRUNCATE` and `pgcolumnar.vacuum()` on a columnar table fail until +step 3, and change nothing; reads and writes keep working: + +``` +ERROR: columnar metadata table "pgcolumnar.retired_group" does not exist +HINT: The pgcolumnar catalog in this database may be older than the installed library. Run ALTER EXTENSION pgcolumnar UPDATE in this database. +``` + +The fix is the same: step 3. + Do not use `DROP EXTENSION` instead. A plain `DROP EXTENSION pgcolumnar` fails while columnar tables exist, because they depend on the access method. The form that succeeds is `DROP EXTENSION pgcolumnar CASCADE`, and it drops every diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index bd0120efb..eb5bdc53f 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -239,11 +239,22 @@ open_columnar_table(const char *name, LOCKMODE lockmode) nspOid = pgcolumnar_schema_oid(); relOid = get_relname_relid(name, nspOid); + /* + * NAME THE REMEDY (#1341 item 66). The usual way here is the upgrade window: + * the new library is installed and the server restarted (docs/installation.md + * steps 1 and 2), but this database's catalog is still the previous release's + * until ALTER EXTENSION UPDATE (step 3). A catalog table this library added, + * such as retired_group in 1.0-alpha5, is then missing, and DROP TABLE, + * TRUNCATE and pgcolumnar.vacuum() stop here. The error alone named only the + * table, which says neither why nor what to do. + */ if (!OidIsValid(relOid)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_TABLE), errmsg("columnar metadata table \"%s.%s\" does not exist", - COLUMNAR_SCHEMA_NAME, name))); + COLUMNAR_SCHEMA_NAME, name), + errhint("The pgcolumnar catalog in this database may be older than the installed library. " + "Run ALTER EXTENSION pgcolumnar UPDATE in this database."))); rel = table_open(relOid, lockmode); diff --git a/test/native_upgrade_converge.sh b/test/native_upgrade_converge.sh index 0249d5655..454900137 100755 --- a/test/native_upgrade_converge.sh +++ b/test/native_upgrade_converge.sh @@ -310,4 +310,53 @@ done rm -f "$REF" P -d postgres -c "DROP DATABASE IF EXISTS conv_fresh;" >/dev/null +# ---- the window: this library over the previous release's catalog (#1341 item 66) ---- +# +# docs/installation.md steps 1 and 2 put the new library in place; step 3, +# ALTER EXTENSION UPDATE, brings the catalog up to it. Between them a database +# runs this library over its previous release's catalog. The library deletes +# from retired_group unconditionally (columnar_metadata.c, PgColumnarDeleteMetadata), +# which that catalog does not have, so DROP TABLE, TRUNCATE and pgcolumnar.vacuum() +# fail there: measured, with 42P01 'columnar metadata table +# "pgcolumnar.retired_group" does not exist', which names neither the cause nor step 3. +# +# The arms hold what the window must keep: reads and writes work, a refused +# operation changes nothing and names the remedy, and step 3 makes every one of +# them work on the same rows. PREV is the newest fixture, so these follow the +# release they describe rather than naming it. +PREV="${_NUC_FIXTURES[${#_NUC_FIXTURES[@]}-1]}" +WIN=conv_window +P -d postgres -c "DROP DATABASE IF EXISTS $WIN;" >/dev/null +P -d postgres -c "CREATE DATABASE $WIN;" >/dev/null +P -d "$WIN" -c "CREATE EXTENSION pgcolumnar VERSION '$PREV';" >/dev/null +check "premise: the window database runs this library over the previous release's catalog" \ + "$(P -d "$WIN" -c "SELECT extversion FROM pg_extension WHERE extname='pgcolumnar';")" "$PREV" +check "premise: that catalog has no retired_group, which this library deletes from" \ + "$(P -d "$WIN" -c "SELECT count(*) FROM pg_class WHERE relname = 'retired_group' AND relnamespace = 'pgcolumnar'::regnamespace;")" "0" +P -d "$WIN" -c "CREATE TABLE w (a int, b text) USING pgcolumnar;" >/dev/null +P -d "$WIN" -c "INSERT INTO w SELECT g, 'x' || g FROM generate_series(1, 20000) g;" >/dev/null +P -d "$WIN" -c "DELETE FROM w WHERE a % 3 = 0;" >/dev/null +win_rows() { P -d "$WIN" -c "SELECT count(*) || '|' || sum(a) FROM w;"; } +check "in the window, a columnar table is written, deleted from and read" "$(win_rows)" "13334|133346667" +win_refusal() { # win_refusal SQL -> SQLSTATE|whether the HINT names step 3, or what happened instead + local out + out="$(P -d "$WIN" -v VERBOSITY=verbose -c "$1" 2>&1)" && { echo accepted; return; } + echo "$(sed -nE 's/^ERROR: ([0-9A-Z]{5}): .*/\1/p' <<<"$out" | head -1)|$(grep -q '^HINT: .*ALTER EXTENSION pgcolumnar UPDATE' <<<"$out" && echo names-the-update || echo no-hint)" +} +check "in the window, DROP TABLE is refused and names ALTER EXTENSION UPDATE" \ + "$(win_refusal "DROP TABLE w;")" "42P01|names-the-update" +check "in the window, TRUNCATE is refused and names ALTER EXTENSION UPDATE" \ + "$(win_refusal "TRUNCATE w;")" "42P01|names-the-update" +check "in the window, pgcolumnar.vacuum() is refused and names ALTER EXTENSION UPDATE" \ + "$(win_refusal "SELECT pgcolumnar.vacuum('w');")" "42P01|names-the-update" +check "in the window, the refused operations changed nothing" "$(win_rows)" "13334|133346667" + +P -d "$WIN" -c "ALTER EXTENSION pgcolumnar UPDATE;" >/dev/null +check "after ALTER EXTENSION UPDATE, the same rows are there" "$(win_rows)" "13334|133346667" +check "after ALTER EXTENSION UPDATE, pgcolumnar.vacuum() succeeds and keeps them" \ + "$(P -d "$WIN" -c "SELECT pgcolumnar.vacuum('w');" >/dev/null 2>&1 && win_rows || echo refused)" "13334|133346667" +check "after ALTER EXTENSION UPDATE, TRUNCATE and DROP TABLE succeed" \ + "$(P -d "$WIN" -c "TRUNCATE w;" >/dev/null 2>&1 && P -d "$WIN" -c "DROP TABLE w;" >/dev/null 2>&1 && echo succeeded || echo refused)" "succeeded" +P -d postgres -c "DROP DATABASE IF EXISTS $WIN;" >/dev/null + pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 9212ee1a5..46d0a068c 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -188,6 +188,7 @@ behaviour, the source of that number is named. - [140. test_parquet_import.py: a partitioned columnar parent is refused by import_parquet](#140-test_parquet_importpy-a-partitioned-columnar-parent-is-refused-by-import_parquet) - [141. test_phase5.py: set_options and reset_options agree](#141-test_phase5py-set_options-and-reset_options-agree) - [142. test_drop_cleanup.py: a columnar matview's options row goes with its DROP](#142-test_drop_cleanuppy-a-columnar-matviews-options-row-goes-with-its-drop) +- [143. test_native_upgrade_converge.py: the upgrade window, before ALTER EXTENSION UPDATE](#143-test_native_upgrade_convergepy-the-upgrade-window-before-alter-extension-update) ## 1. How to read a test in here @@ -7377,3 +7378,13 @@ Part of the pytest twin of `test/drop_cleanup.sh` (#1265, ported for #1341): its | `test_a_columnar_matview_takes_its_options_row_with_it` | a planted options row is gone after DROP MATERIALIZED VIEW | | `test_refresh_keeps_a_matviews_options_row` | a REFRESH that rewrote the matview leaves its set_options row in place | | `test_an_unrelated_options_row_survives` | another table's row survives a matview's DROP, and an ordinary columnar table still takes its own | + +## 143. test_native_upgrade_converge.py: the upgrade window, before ALTER EXTENSION UPDATE + +Part of the pytest twin of `test/native_upgrade_converge.sh` (#1341 item 66): its upgrade-window arms only, by the shell's names, and the pair is declared INCOMPLETE. Between installing a new library and running `ALTER EXTENSION pgcolumnar UPDATE`, a database runs the new library over its previous release's catalog. Reads and writes must work there. `DROP TABLE`, `TRUNCATE` and `pgcolumnar.vacuum()`, which need a catalog table the old one lacks, must be refused with 42P01, change nothing, and carry a hint naming the update; after the update all three succeed on the same rows. The previous release is the version the shipped upgrade script to the current `default_version` starts from; its base script is staged from `test/fixtures` for the test and removed after. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_the_upgrade_window` | at the previous catalog: rows written, deleted and read; the three operations refused with the hint and nothing changed; after the update the rows are intact and all three succeed | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index a6a77ccba..f758b74f1 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -747,4 +747,6 @@ guard_tests 438 # seven tests, by collection over the cluster files on this tree. # cluster_tests 653 -> 660 for #1341 item 29's partial twins: test_inheritance.py (2), # test_parquet_import.py (1), test_phase5.py (1) and test_drop_cleanup.py (3), by collection. -cluster_tests 660 +# cluster_tests 660 -> 661 for #1341 item 66's test_native_upgrade_converge.py, one test, +# by collection over the cluster files on this tree. +cluster_tests 661 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 4140fbf32..4e0dbf689 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -179,6 +179,10 @@ # about the PORT rather than a licence: the standing arm does not grade it, so the # reason is the only thing standing between a declared gap and a forgotten one. INCOMPLETE = { + # #1341 item 66 ported the arms it added: the upgrade window, this library over + # the previous release's catalog before ALTER EXTENSION UPDATE. + "native_upgrade_converge": "the upgrade-window arms are ported; the fixture-blob, " + "staging and catalog-convergence arms are not", # #1341 item 29 ported the arms four fixes added with shell arms only. Each # pair below holds that fix's arms and nothing else of its shell suite. "inheritance": "the #1259 arms (a partitioned columnar parent is refused with " diff --git a/test/pytest/test_native_upgrade_converge.py b/test/pytest/test_native_upgrade_converge.py new file mode 100644 index 000000000..5a2c1d54b --- /dev/null +++ b/test/pytest/test_native_upgrade_converge.py @@ -0,0 +1,126 @@ +"""The upgrade window: this library over the previous release's catalog, before ALTER +EXTENSION UPDATE (#1341 item 66). Part of the pytest twin of +`test/native_upgrade_converge.sh`: these arms only, by the shell's names, and the pair +is declared INCOMPLETE. + +docs/installation.md steps 1 and 2 put a new library in place; step 3, ALTER EXTENSION +UPDATE, brings each database's catalog up to it. Between them a database runs the new +library over the old catalog, and the library deletes from catalog tables the old one +does not have (retired_group, new in 1.0-alpha5). Reads and writes must work there; an +operation that needs a missing table must be refused cleanly, change nothing and name +step 3; and after step 3 every one of them must work on the same rows. + +The previous release is the version the shipped upgrade script to the current +default_version starts from, and its base script is staged from test/fixtures into the +extension directory for the test, as the shell does, then removed. +""" +import pathlib +import re +import shutil +import subprocess + +import psycopg +import pytest + +REPO = pathlib.Path(__file__).resolve().parents[2] + + +def _target_and_previous(): + target = re.search(r"^default_version\s*=\s*'([^']+)'", + (REPO / "pgcolumnar.control").read_text(), re.M).group(1) + prev = [p.name[len("pgcolumnar--"):-len(f"--{target}.sql")] + for p in REPO.glob(f"pgcolumnar--*--{target}.sql")] + return target, prev + + +@pytest.fixture +def window_db(pgc_cluster): + """-> (dsn, previous version) of a database at the previous release's catalog.""" + target, prev = _target_and_previous() + assert len(prev) == 1, f"one upgrade script should lead to {target}, found {prev}" + prev = prev[0] + sharedir = subprocess.run([pgc_cluster.pg_config, "--sharedir"], capture_output=True, + text=True, check=True).stdout.strip() + dst = pathlib.Path(sharedir) / "extension" / f"pgcolumnar--{prev}.sql" + backup = dst.with_name(dst.name + ".pytest-bak") + if dst.exists(): + shutil.copy2(dst, backup) + shutil.copy2(REPO / "test" / "fixtures" / f"pgcolumnar--{prev}.sql", dst) + + def admin(sql): + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as c: + c.execute(sql) + + admin("DROP DATABASE IF EXISTS pgc_upgrade_window") + admin("CREATE DATABASE pgc_upgrade_window") + dsn = pgc_cluster.dsn(dbname="pgc_upgrade_window") + try: + with psycopg.connect(dsn, autocommit=True) as c: + c.execute(f"CREATE EXTENSION pgcolumnar VERSION '{prev}'") + yield dsn, prev + finally: + admin("DROP DATABASE IF EXISTS pgc_upgrade_window WITH (FORCE)") + if backup.exists(): + shutil.move(backup, dst) + else: + dst.unlink(missing_ok=True) + + +def _one(dsn, sql): + with psycopg.connect(dsn, autocommit=True) as c: + return c.execute(sql).fetchone()[0] + + +def _refusal(dsn, sql): + """-> 'SQLSTATE|names-the-update' or '|no-hint', or 'accepted'.""" + try: + with psycopg.connect(dsn, autocommit=True) as c: + c.execute(sql) + return "accepted" + except psycopg.Error as exc: + hint = exc.diag.message_hint or "" + return f"{exc.sqlstate}|{'names-the-update' if 'ALTER EXTENSION pgcolumnar UPDATE' in hint else 'no-hint'}" + + +def _succeeds(dsn, *sqls): + try: + with psycopg.connect(dsn, autocommit=True) as c: + for sql in sqls: + c.execute(sql) + return "succeeded" + except psycopg.Error as exc: + return f"refused ({exc.sqlstate}: {str(exc).splitlines()[0]})" + + +def test_the_upgrade_window(window_db, expect): + dsn, prev = window_db + expect.text(_one(dsn, "SELECT extversion FROM pg_extension WHERE extname = 'pgcolumnar'"), prev, + "premise: the window database runs this library over the previous release's catalog") + expect.num(_one(dsn, "SELECT count(*) FROM pg_class WHERE relname = 'retired_group' " + "AND relnamespace = 'pgcolumnar'::regnamespace"), 0, + "premise: that catalog has no retired_group, which this library deletes from") + with psycopg.connect(dsn, autocommit=True) as c: + c.execute("CREATE TABLE w (a int, b text) USING pgcolumnar") + c.execute("INSERT INTO w SELECT g, 'x' || g FROM generate_series(1, 20000) g") + c.execute("DELETE FROM w WHERE a % 3 = 0") + + def rows(): + return _one(dsn, "SELECT count(*) || '|' || sum(a) FROM w") + + expect.text(rows(), "13334|133346667", "in the window, a columnar table is written, deleted from and read") + expect.text(_refusal(dsn, "DROP TABLE w"), "42P01|names-the-update", + "in the window, DROP TABLE is refused and names ALTER EXTENSION UPDATE") + expect.text(_refusal(dsn, "TRUNCATE w"), "42P01|names-the-update", + "in the window, TRUNCATE is refused and names ALTER EXTENSION UPDATE") + expect.text(_refusal(dsn, "SELECT pgcolumnar.vacuum('w')"), "42P01|names-the-update", + "in the window, pgcolumnar.vacuum() is refused and names ALTER EXTENSION UPDATE") + expect.text(rows(), "13334|133346667", "in the window, the refused operations changed nothing") + + with psycopg.connect(dsn, autocommit=True) as c: + c.execute("ALTER EXTENSION pgcolumnar UPDATE") + expect.text(rows(), "13334|133346667", "after ALTER EXTENSION UPDATE, the same rows are there") + vacuumed = _succeeds(dsn, "SELECT pgcolumnar.vacuum('w')") + expect.text(rows() if vacuumed == "succeeded" else vacuumed, "13334|133346667", + "after ALTER EXTENSION UPDATE, pgcolumnar.vacuum() succeeds and keeps them") + expect.text(_succeeds(dsn, "TRUNCATE w", "DROP TABLE w"), "succeeded", + "after ALTER EXTENSION UPDATE, TRUNCATE and DROP TABLE succeed") From 110aa0bf584f065f14ef3e068548d41827e58fdd Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 16 Oct 2026 05:01:05 +0000 Subject: [PATCH 2/3] test: fk_referencing gets its pytest twin, and a referenced table may still be set to a row store (#1341) test_fk_referencing.py asserts every shell check by name; compare_to_bash grades it as covering all 28 (27 checks on PG17+, and the pre-17 branch's own), and it joins COMPLETE. The conversion guard's over-rejection control converted a table with no foreign key back to heap, so it could not see a guard that refused every SET ACCESS METHOD on a referenced table whatever the target: with the guard's columnar-target test removed, the suite stayed 26/26. A new arm sets the referenced table itself to heap; under that mutant both twins now fail it. cluster_tests 661 -> 666 by collection; TESTS.md gets section 144. Co-Authored-By: Claude Opus 5.5 --- test/fk_referencing.sh | 10 ++ test/pytest/TESTS.md | 15 +++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_fk_referencing.py | 158 ++++++++++++++++++++++++++++ 5 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 test/pytest/test_fk_referencing.py diff --git a/test/fk_referencing.sh b/test/fk_referencing.sh index 73b59c41d..cd49dcecd 100755 --- a/test/fk_referencing.sh +++ b/test/fk_referencing.sh @@ -220,6 +220,16 @@ check "converting back to heap is unaffected" \ "$(case "$back" in *ERROR*) echo "no ($back)" ;; *) echo yes ;; esac)" \ "yes" +# AND ON THE REFERENCED TABLE ITSELF (#1341). The control above converts a table +# with no foreign key, so it cannot see a rule that refuses every SET ACCESS METHOD +# on a referenced table whatever the target: with the guard's columnar-target test +# removed, this file stayed 26/26. Only a columnar target needs the row lock the +# constraint cannot get. +refheap="$(psql_run "ALTER TABLE fk_conv_parent SET ACCESS METHOD heap;" 2>&1 || true)" +check "an FK-referenced table may still be set to a row store" \ + "$(case "$refheap" in *ERROR*) echo "no ($refheap)" ;; *) echo yes ;; esac)" \ + "yes" + # --- 7. a partitioned parent is refused too (#201) ----------------------------- # A partitioned table has no storage, so setting its access method breaks nothing diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 46d0a068c..579d1b082 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -189,6 +189,7 @@ behaviour, the source of that number is named. - [141. test_phase5.py: set_options and reset_options agree](#141-test_phase5py-set_options-and-reset_options-agree) - [142. test_drop_cleanup.py: a columnar matview's options row goes with its DROP](#142-test_drop_cleanuppy-a-columnar-matviews-options-row-goes-with-its-drop) - [143. test_native_upgrade_converge.py: the upgrade window, before ALTER EXTENSION UPDATE](#143-test_native_upgrade_convergepy-the-upgrade-window-before-alter-extension-update) +- [144. test_fk_referencing.py: a foreign key referencing a columnar table is refused where it is chosen](#144-test_fk_referencingpy-a-foreign-key-referencing-a-columnar-table-is-refused-where-it-is-chosen) ## 1. How to read a test in here @@ -7388,3 +7389,17 @@ Part of the pytest twin of `test/native_upgrade_converge.sh` (#1341 item 66): it | test | what it holds | | --- | --- | | `test_the_upgrade_window` | at the previous catalog: rows written, deleted and read; the three operations refused with the hint and nothing changed; after the update the rows are intact and all three succeed | + +## 144. test_fk_referencing.py: a foreign key referencing a columnar table is refused where it is chosen + +The pytest twin of `test/fk_referencing.sh` (ported for #1341), every check by the shell's name. Enforcing a foreign key locks the referenced row, and columnar tables cannot lock rows, so a constraint referencing one would make every insert into the referencing table fail. It is refused at DDL by either route into that state: creating the constraint against a columnar table, or converting a referenced table (or partitioned parent) to columnar. Only the referenced side is refused, and the controls hold everything else steady, including converting the referenced table itself to a row store. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_the_referenced_side_is_refused_at_ddl` | CREATE TABLE and ALTER TABLE ADD CONSTRAINT referencing a columnar table are refused, saying why, and nothing is left behind | +| `test_the_referencing_side_still_works` | a columnar table referencing a heap table is created, accepts satisfying rows and rejects a violating one | +| `test_constraints_that_need_no_row_lock_are_unaffected` | primary key, unique, check and not-null on a columnar table, and a heap-to-heap foreign key, are unaffected | +| `test_converting_a_referenced_table_is_refused_and_nothing_else_is` | SET ACCESS METHOD pgcolumnar on a referenced table and the helper are refused, the table stays heap and its child writable; an unreferenced table, the referencing side, an unrelated ALTER, a conversion back to heap and the referenced table to a row store all still work | +| `test_a_partitioned_referenced_parent` | a columnar partition and setting the parent's access method are refused, the parent does not become columnar, an ordinary partition still works; an unreferenced partitioned table converts from PG17, and before 17 core refuses it | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index f758b74f1..db4d7c30f 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -749,4 +749,6 @@ guard_tests 438 # test_parquet_import.py (1), test_phase5.py (1) and test_drop_cleanup.py (3), by collection. # cluster_tests 660 -> 661 for #1341 item 66's test_native_upgrade_converge.py, one test, # by collection over the cluster files on this tree. -cluster_tests 661 +# cluster_tests 661 -> 666 for #1341's test_fk_referencing.py, the twin of fk_referencing.sh: +# five tests, by collection over the cluster files on this tree. +cluster_tests 666 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 4e0dbf689..a0fd43008 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -106,6 +106,7 @@ "differential", "encode_post_codec", "entry_point_privilege", + "fk_referencing", "format_downgrade", "hilbert_cluster", "hilbert_locality", diff --git a/test/pytest/test_fk_referencing.py b/test/pytest/test_fk_referencing.py new file mode 100644 index 000000000..3e7a60274 --- /dev/null +++ b/test/pytest/test_fk_referencing.py @@ -0,0 +1,158 @@ +"""A foreign key that references a columnar table is refused where it is chosen +(#1341). The pytest twin of `test/fk_referencing.sh`, every check by the shell's name. + +Referential integrity reads the referenced row with FOR KEY SHARE, and row locking is +not implemented for columnar tables, so a constraint referencing one would make every +insert into the referencing table fail. It is refused at DDL instead, by either route +into that state: creating the constraint against a columnar table, or converting a +referenced table to columnar. Only the REFERENCED side is refused: a columnar table may +reference a heap table, and the controls hold everything else steady -- other +constraints, heap-to-heap keys, conversions of an unreferenced table, of the +referencing side, back to heap, and of the referenced table to a row store. +""" +import psycopg + + +def _err(conn, sql): + """-> '' when sql succeeds, else the error's message, detail and hint, joined.""" + try: + with conn.cursor() as cur: + cur.execute(sql) + return "" + except psycopg.Error as exc: + d = exc.diag + return " ".join(x for x in (d.message_primary, d.message_detail, d.message_hint) if x) or str(exc) + + +def _has(text, needle): + return "yes" if needle in text else f"no ({text})" + + +def _ok(text): + return "yes" if not text else f"no ({text})" + + +def _one(conn, sql): + with conn.cursor() as cur: + cur.execute(sql) + return cur.fetchone()[0] + + +def _run(conn, *sqls): + with conn.cursor() as cur: + for sql in sqls: + cur.execute(sql) + + +def test_the_referenced_side_is_refused_at_ddl(pgc_conn, expect): + conn = pgc_conn + _run(conn, "CREATE TABLE fk_parent (id int PRIMARY KEY) USING pgcolumnar", + "INSERT INTO fk_parent VALUES (1), (2)") + err = _err(conn, "CREATE TABLE fk_child (id int REFERENCES fk_parent(id))") + expect.text(_has(err, "cannot create a foreign key referencing columnar table"), "yes", + "a foreign key referencing a columnar table is refused") + expect.text(_has(err, "row locking is not supported"), "yes", "and the error says why") + expect.num(_one(conn, "SELECT count(*) FROM pg_class WHERE relname = 'fk_child' " + "AND relnamespace = current_schema()::regnamespace"), 0, + "the refused table was not left behind") + + _run(conn, "CREATE TABLE fk_child2 (id int)") + err2 = _err(conn, "ALTER TABLE fk_child2 ADD CONSTRAINT fk_c2 FOREIGN KEY (id) REFERENCES fk_parent(id)") + expect.text(_has(err2, "cannot create a foreign key referencing columnar table"), "yes", + "ALTER TABLE ADD CONSTRAINT is refused by the same rule") + + +def test_the_referencing_side_still_works(pgc_conn, expect): + conn = pgc_conn + _run(conn, "CREATE TABLE fk_h_parent (id int PRIMARY KEY)", "INSERT INTO fk_h_parent VALUES (1), (2), (3)") + expect.text(_ok(_err(conn, "CREATE TABLE fk_c_child (id int REFERENCES fk_h_parent(id), v text) " + "USING pgcolumnar")), "yes", + "a columnar table may reference a heap table") + _err(conn, "INSERT INTO fk_c_child VALUES (1, 'a'), (2, 'b')") + expect.num(_one(conn, "SELECT count(*) FROM fk_c_child"), 2, "and rows that satisfy it are accepted") + expect.text(_has(_err(conn, "INSERT INTO fk_c_child VALUES (99, 'nope')"), "violates foreign key constraint"), + "yes", "a row that violates it is still rejected") + + +def test_constraints_that_need_no_row_lock_are_unaffected(pgc_conn, expect): + conn = pgc_conn + expect.text(_ok(_err(conn, "CREATE TABLE fk_other (id int PRIMARY KEY, k int UNIQUE, v int CHECK (v > 0), " + "w int NOT NULL) USING pgcolumnar")), "yes", + "primary key, unique, check and not-null on a columnar table are unaffected") + _err(conn, "INSERT INTO fk_other VALUES (1, 10, 5, 7)") + expect.num(_one(conn, "SELECT count(*) FROM fk_other"), 1, "and they still work") + expect.text(_has(_err(conn, "INSERT INTO fk_other VALUES (1, 11, 5, 7)"), "duplicate key"), "yes", + "the primary key is still enforced") + _run(conn, "CREATE TABLE fk_hh_parent (id int PRIMARY KEY)") + expect.text(_ok(_err(conn, "CREATE TABLE fk_hh_child (id int REFERENCES fk_hh_parent(id))")), "yes", + "a foreign key between two heap tables is unaffected") + + +def test_converting_a_referenced_table_is_refused_and_nothing_else_is(pgc_conn, expect): + conn = pgc_conn + _run(conn, "CREATE TABLE fk_conv_parent (id int PRIMARY KEY)", + "CREATE TABLE fk_conv_child (id int REFERENCES fk_conv_parent(id))", + "INSERT INTO fk_conv_parent VALUES (1)", "INSERT INTO fk_conv_child VALUES (1)") + conv = _err(conn, "ALTER TABLE fk_conv_parent SET ACCESS METHOD pgcolumnar") + expect.text(_has(conv, "referenced by a foreign key"), "yes", + "converting an FK-referenced table to columnar is refused") + expect.text(_has(conv, "row locking is not supported"), "yes", "and the error names the constraint") + expect.text(_one(conn, "SELECT am.amname::text FROM pg_class c JOIN pg_am am ON am.oid = c.relam " + "WHERE c.oid = 'fk_conv_parent'::regclass"), "heap", "the parent is still a row store") + _err(conn, "INSERT INTO fk_conv_parent VALUES (2)") + expect.text(_ok(_err(conn, "INSERT INTO fk_conv_child VALUES (2)")), "yes", + "the child is still writable afterwards") + expect.text(_has(_err(conn, "SELECT pgcolumnar.alter_table_set_access_method('fk_conv_parent', 'pgcolumnar')"), + "referenced by a foreign key"), "yes", "the conversion helper is refused identically") + + # The over-rejection controls: a rule keyed on the wrong side, or firing for any + # ALTER TABLE or any target, passes every arm above and fails one of these. + _run(conn, "CREATE TABLE fk_conv_plain (id int PRIMARY KEY)", "INSERT INTO fk_conv_plain VALUES (1)") + expect.text(_ok(_err(conn, "ALTER TABLE fk_conv_plain SET ACCESS METHOD pgcolumnar")), "yes", + "a table with no foreign key still converts") + expect.text(_ok(_err(conn, "ALTER TABLE fk_conv_child SET ACCESS METHOD pgcolumnar")), "yes", + "converting the referencing side is still allowed") + expect.text(_ok(_err(conn, "ALTER TABLE fk_conv_parent ADD COLUMN note text")), "yes", + "an unrelated ALTER TABLE on a referenced table is unaffected") + expect.text(_ok(_err(conn, "ALTER TABLE fk_conv_plain SET ACCESS METHOD heap")), "yes", + "converting back to heap is unaffected") + # The control above has no foreign key; this one is the referenced table itself. + expect.text(_ok(_err(conn, "ALTER TABLE fk_conv_parent SET ACCESS METHOD heap")), "yes", + "an FK-referenced table may still be set to a row store") + + +def test_a_partitioned_referenced_parent(pgc_conn, expect): + """Setting a partitioned parent's access method chooses what every later partition + inherits, and core clones the foreign key to each, so it is refused too (#201).""" + conn = pgc_conn + _run(conn, "CREATE TABLE fk_pp (id int PRIMARY KEY) PARTITION BY RANGE (id)", + "CREATE TABLE fk_pp1 PARTITION OF fk_pp FOR VALUES FROM (1) TO (100)", + "CREATE TABLE fk_pc (id int REFERENCES fk_pp(id))") + expect.text(_has(_err(conn, "CREATE TABLE fk_pp2 PARTITION OF fk_pp FOR VALUES FROM (100) TO (200) " + "USING pgcolumnar"), "cannot create a foreign key referencing columnar table"), + "yes", "a columnar partition of an FK-referenced parent is refused") + expect.text(_has(_err(conn, "ALTER TABLE fk_pp SET ACCESS METHOD pgcolumnar"), "cannot convert table"), "yes", + "and setting the parent's access method is refused as well") + # "did it become columnar", not "is it heap": a partitioned table never given an + # access method has relam = 0, which joins to no pg_am row. + expect.num(_one(conn, "SELECT count(*) FROM pg_class c JOIN pg_am a ON a.oid = c.relam " + "WHERE c.oid = 'fk_pp'::regclass AND a.amname = 'pgcolumnar'"), 0, + "so the parent did not become columnar") + expect.num(0 if not _err(conn, "CREATE TABLE fk_pp3 PARTITION OF fk_pp FOR VALUES FROM (200) TO (300)") else 1, + 0, "and a partition can still be created the ordinary way") + + _run(conn, "CREATE TABLE fk_free (id int) PARTITION BY RANGE (id)") + major = int(_one(conn, "SHOW server_version_num")) // 10000 + if major >= 17: + expect.num(0 if not _err(conn, "ALTER TABLE fk_free SET ACCESS METHOD pgcolumnar") else 1, 0, + "a partitioned table with no foreign key still converts") + _err(conn, "CREATE TABLE fk_free1 PARTITION OF fk_free FOR VALUES FROM (1) TO (100)") + expect.text(_one(conn, "SELECT amname::text FROM pg_class c JOIN pg_am a ON a.oid = c.relam " + "WHERE c.oid = 'fk_free1'::regclass"), "pgcolumnar", + "and its partitions inherit the columnar access method") + else: + # Before 17 core refuses SET ACCESS METHOD on every partitioned table, so the + # refusal must be core's, by core's message. + expect.text(_has(_err(conn, "ALTER TABLE fk_free SET ACCESS METHOD pgcolumnar"), + "cannot change access method of a partitioned table"), "yes", + "before 17, core refuses this for every partitioned table") From 4ee6e1fdc3377e33e1d3bf6017a716c63446f3ce Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 16 Oct 2026 05:01:05 +0000 Subject: [PATCH 3/3] test: fk_referencing seeds the ledger, 17 of 28 arms dated by four mutations of the foreign-key guards (#1341) 28 rows from five-major runs (27/27 on 17-19, 26/26 on 15-16). Dated on PG18 by: the referenced-side refusal removed (5), the guard keyed on the referencing table (8), the conversion refusal removed (8), and the conversion refused whatever the target (1). suites_not_covered 219 -> 218; the census is 3647 rows, 2686 never, 961 dated, restated on main db561684. Co-Authored-By: Claude Opus 5.5 --- test/check_ledger.tsv | 28 ++++++++++++++++++++++++++++ test/check_ledger_budget.txt | 22 ++++++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 74fcd205f..2c094ebec 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -804,6 +804,34 @@ fdw_plan_privilege fdw_plan_privilege and a metadata_path that does not exist pl fdw_plan_privilege fdw_plan_privilege premise: the Parquet file was written 15;16;17;18;19 never - fdw_plan_privilege fdw_plan_privilege premise: the reader is not a member of pg_read_server_files 15;16;17;18;19 never - fdw_plan_privilege fdw_plan_privilege the unprivileged Iceberg scan is still refused (42501) 15;16;17;18;19 never - +fk_referencing fk_referencing ALTER TABLE ADD CONSTRAINT is refused by the same rule 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing a columnar partition of an FK-referenced parent is refused 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing a columnar table may reference a heap table 15;16;17;18;19 2026-10-16 columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing a foreign key between two heap tables is unaffected 15;16;17;18;19 never - +fk_referencing fk_referencing a foreign key referencing a columnar table is refused 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing a partitioned table with no foreign key still converts 17;18;19 never - +fk_referencing fk_referencing a row that violates it is still rejected 15;16;17;18;19 2026-10-16 columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing a table with no foreign key still converts 15;16;17;18;19 never - +fk_referencing fk_referencing an FK-referenced table may still be set to a row store 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table refused whatever the target access method +fk_referencing fk_referencing an unrelated ALTER TABLE on a referenced table is unaffected 15;16;17;18;19 never - +fk_referencing fk_referencing and a partition can still be created the ordinary way 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing and its partitions inherit the columnar access method 17;18;19 never - +fk_referencing fk_referencing and rows that satisfy it are accepted 15;16;17;18;19 2026-10-16 columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing and setting the parent's access method is refused as well 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing and the error names the constraint 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing and the error says why 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +fk_referencing fk_referencing and they still work 15;16;17;18;19 never - +fk_referencing fk_referencing before 17, core refuses this for every partitioned table 15;16 never - +fk_referencing fk_referencing converting an FK-referenced table to columnar is refused 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing converting back to heap is unaffected 15;16;17;18;19 never - +fk_referencing fk_referencing converting the referencing side is still allowed 15;16;17;18;19 never - +fk_referencing fk_referencing primary key, unique, check and not-null on a columnar table are unaffected 15;16;17;18;19 never - +fk_referencing fk_referencing so the parent did not become columnar 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing the child is still writable afterwards 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing the conversion helper is refused identically 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing the parent is still a row store 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused +fk_referencing fk_referencing the primary key is still enforced 15;16;17;18;19 never - +fk_referencing fk_referencing the refused table was not left behind 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one harness_selftest 030-assertions nothing leaked into the squatter 15;16;17;18;19 never - harness_selftest 030-assertions pgc_port_free says the squatter's port is busy 15;16;17;18;19 never - harness_selftest 030-assertions squatter survived untouched 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 3f8e29cde..42a7cd12d 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 219 +suites_not_covered 218 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -2943,4 +2943,22 @@ suites_not_covered 219 # never 2676 - 1 = 2675 # dated 943 + 1 = 944 # 2675 + 944 == 3619 -checks_never_observed_red 2675 +# +# #1341, fk_referencing seeded, with its twin (OffgridwithJD): 2675 -> 2686, and +# suites_not_covered 219 -> 218. The suite had no rows. It enters with 28 rows +# from five-major runs: 27 checks on PG17-19 and 26 on 15-16, where the +# partitioned-control branch asserts core's refusal instead of a conversion. 17 are +# dated by four mutations of the foreign-key guards: the referenced-side refusal +# removed, the guard keyed on the referencing table, the conversion refusal +# removed, and the conversion refused whatever the target access method (the new +# referenced-table-to-row-store arm). Eleven arrive never: the over-rejection +# controls that no single guard reaches (another constraint, a heap-to-heap key, +# an unreferenced conversion, an unrelated ALTER, a conversion back to heap, the +# referencing side, the partitioned control on either side of 17) and the primary +# key's own arms. +# +# ledger rows 3619 + 28 = 3647 +# never 2675 + 11 = 2686 +# dated 944 + 17 = 961 +# 2686 + 961 == 3647 +checks_never_observed_red 2686