fix: in the upgrade window, a refusal for a missing catalog table names ALTER EXTENSION UPDATE (#1341 item 66)

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

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

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);
 

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

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 |

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

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 "

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")