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