diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8ab46fb54..ab5ab84d8 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -188,7 +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) +- [143. test_native_upgrade_converge.py: every release upgrades to a fresh catalog, and the window before it](#143-test_native_upgrade_convergepy-every-release-upgrades-to-a-fresh-catalog-and-the-window-before-it) - [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) - [145. test_truncate_cleanup.py: TRUNCATE leaves no catalog rows behind, and keeps what it must](#145-test_truncate_cleanuppy-truncate-leaves-no-catalog-rows-behind-and-keeps-what-it-must) - [146. test_native_dml.py: DELETE, UPDATE and MERGE on a native table agree with heap](#146-test_native_dmlpy-delete-update-and-merge-on-a-native-table-agree-with-heap) @@ -7529,14 +7529,20 @@ Part of the pytest twin of `test/drop_cleanup.sh` (#1265, ported for #1341): its | `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 +## 143. test_native_upgrade_converge.py: every release upgrades to a fresh catalog, and the window before it -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. +The pytest twin of `test/native_upgrade_converge.sh` (#1341), by the shell's names, except its release-tag checks, so the pair is declared INCOMPLETE. `ALTER EXTENSION pgcolumnar UPDATE` must bring a database on any released version to a catalog identical to a fresh install of the current `default_version`: every function's definition, ACL and comment, and every relation, column, type and foreign-data wrapper. The released starting points are the install scripts in `test/fixtures`. They are staged into the extension directory once for the module, under a lock that makes a second stager wait, and whatever was there is put back after. A backup already there means an earlier run died, so it holds the original and is kept. + +Between installing a new library and running the update, a database runs the new library over its previous release's catalog (#1341 item 66). Reads and writes must work there. `DROP TABLE`, `TRUNCATE` and `pgcolumnar.vacuum()` need a catalog table the old one lacks, so they must be refused with 42P01, change nothing, and carry a hint naming the update. After the update all three succeed on the same rows. ### Every test | test | what it holds | | --- | --- | +| `test_a_run_after_one_that_died_keeps_the_original` | with a died run's state in a temporary directory, staging and restoring leave the prefix's own file, not the fixture | +| `test_two_stagings_at_once_keep_the_original` | a second staging that starts inside the first, as another `pytest -n` worker's would, waits for it under the staging lock, and the prefix's own file survives both | +| `test_the_staged_fixtures_are_the_committed_ones` | the control's `default_version`; no backup was already staged; every fixture was staged, and each staged script is the committed fixture | +| `test_each_release_upgrades_to_a_fresh_catalog` | parametrized by the four fixtures: a fresh install of the target has more than 100 objects to compare, the upgrade applies, the extension is at the target, and its catalog equals the fresh install's | | `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 diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 0301996d0..6b01c6e7c 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -856,4 +856,6 @@ guard_tests 442 # native_groupagg_wide_cost.sh: five tests, one per check, by collection over the cluster files. # cluster_tests 1011 -> 1018 for #1341's test_hardening.py, the twin of hardening.sh: seven # tests, one per check, by collection over the cluster files. -cluster_tests 1018 +# cluster_tests 1018 -> 1025 for #1341, test_native_upgrade_converge.py ported by name except its +# release-tag arms (1 -> 8 tests), by collection over the cluster files. +cluster_tests 1025 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index e532b7139..88f4f7038 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -226,10 +226,12 @@ # 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 ported every arm but the release-tag ones: each tagged fixture is + # byte-identical to what its tag shipped. CI checks out without tags, so the shell + # skips them there with check_skip, and this layer has no verdict for an arm that + # runs on one checkout and not another. + "native_upgrade_converge": "every arm is ported except the release-tag sweep and " + "the per-tag fixture-blob comparisons", # #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 index 5a2c1d54b..d1f220b17 100644 --- a/test/pytest/test_native_upgrade_converge.py +++ b/test/pytest/test_native_upgrade_converge.py @@ -1,28 +1,119 @@ -"""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. +"""The pytest twin of `test/native_upgrade_converge.sh`, by the shell's names, except its +release-tag arms (see INCOMPLETE in test_compare_to_bash.py). + +ALTER EXTENSION pgcolumnar UPDATE must bring a database on any released version to a +catalog identical to a fresh install of the current default_version: every function's +definition, ACL and comment, every relation, column, type and foreign-data wrapper. +The released starting points are frozen install scripts in test/fixtures. They are not +shipped, so each is staged into the extension directory for the module, keeping what +was there, and put back after. A backup already there means an earlier run died before +putting it back: it holds the original, so it is kept, and a premise says so. + +The upgrade window (#1341 item 66): 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. """ +import contextlib +import difflib +import fcntl +import hashlib +import os import pathlib import re import shutil import subprocess +import tempfile +import threading import psycopg import pytest +from pgc_vacuity import query_error + REPO = pathlib.Path(__file__).resolve().parents[2] +FIXTURES = REPO / "test" / "fixtures" +BACKUP = ".pytest-bak" +# The released starting points, oldest first; the last is the previous release. +VERSIONS = ["1.0-alpha", "1.0-alpha2", "1.0-alpha3", "1.0-alpha4"] + +# One line per object in the pgcolumnar schema: every function's definition, ACL and +# comment; every relation's kind, ACL and comment; every column's type, NOT NULL and +# comment; every non-base type; every foreign-data wrapper's handler and validator. The +# comments are here because pg_get_functiondef does not emit COMMENT. +SNAPSHOT = """ + SELECT 'FN|'||p.oid::regprocedure||'|'||md5(pg_get_functiondef(p.oid))||'|'||coalesce(p.proacl::text,'(def)')||'|'||md5(coalesce(obj_description(p.oid,'pg_proc'),'')) + FROM pg_proc p JOIN pg_namespace n ON n.oid=p.pronamespace WHERE n.nspname='pgcolumnar' + UNION ALL SELECT 'REL|'||c.relkind::text||'|'||c.relname||'|'||coalesce(c.relacl::text,'(def)')||'|'||md5(coalesce(obj_description(c.oid,'pg_class'),'')) + FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace WHERE n.nspname='pgcolumnar' + UNION ALL SELECT 'COL|'||c.relname||'.'||a.attname||'|'||format_type(a.atttypid,a.atttypmod)||'|'||a.attnotnull::text||'|'||md5(coalesce(col_description(c.oid,a.attnum),'')) + FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace JOIN pg_attribute a ON a.attrelid=c.oid + WHERE n.nspname='pgcolumnar' AND c.relkind IN ('r','p') AND a.attnum>0 AND NOT a.attisdropped + UNION ALL SELECT 'TYP|'||t.typname||'|'||t.typtype::text FROM pg_type t JOIN pg_namespace n ON n.oid=t.typnamespace + WHERE n.nspname='pgcolumnar' AND t.typtype<>'b' + UNION ALL SELECT 'FDW|'||w.fdwname||'|'||coalesce(h.oid::regproc::text,'-')||'|'||coalesce(v.oid::regproc::text,'-') + FROM pg_foreign_data_wrapper w LEFT JOIN pg_proc h ON h.oid=w.fdwhandler LEFT JOIN pg_proc v ON v.oid=w.fdwvalidator + WHERE w.fdwname LIKE 'pgcolumnar%' +""" + + +def _extdir(pgc_cluster): + sharedir = subprocess.run([pgc_cluster.pg_config, "--sharedir"], capture_output=True, + text=True, check=True).stdout.strip() + return pathlib.Path(sharedir) / "extension" + + +def _stage(extdir, versions): + """Copy each version's fixture into `extdir`, keeping what was there. -> staged paths.""" + staged = [] + for v in versions: + if not (FIXTURES / f"pgcolumnar--{v}.sql").exists(): + continue + dst = extdir / f"pgcolumnar--{v}.sql" + backup = dst.with_name(dst.name + BACKUP) + # A backup already here is the original, left by a run that died: taking it + # again would copy the installed fixture over it. + if dst.exists() and not backup.exists(): + shutil.copy2(dst, backup) + shutil.copy2(FIXTURES / f"pgcolumnar--{v}.sql", dst) + staged.append(dst) + return staged + + +def _unstage(staged): + """Put back what `_stage` found, or remove what it added.""" + for dst in staged: + backup = dst.with_name(dst.name + BACKUP) + if backup.exists(): + shutil.move(backup, dst) + else: + dst.unlink(missing_ok=True) + + +@contextlib.contextmanager +def _staged(extdir, versions): + """Stage `versions` into `extdir` for the life of the block, then put back what was + there. -> (staged paths, versions whose backup was already there: an earlier run died). + + Holds an exclusive lock for the whole block, so a second stager of the same directory + (another `pytest -n` worker) waits until this one has put everything back. Without it, + the second keeps the first's backup and restores it, and the first then deletes the + original. The lock file lives with the build lock in pgc_cluster.build_once, keyed by + the directory, so nothing is added to the prefix.""" + lock_dir = pathlib.Path(tempfile.gettempdir()) / f"pgc-pytest-{os.getuid()}" + lock_dir.mkdir(mode=0o700, exist_ok=True) + key = hashlib.sha256(str(extdir.resolve()).encode()).hexdigest()[:16] + with open(lock_dir / f"staging-{key}.lock", "w") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + died = [v for v in versions if (extdir / f"pgcolumnar--{v}.sql{BACKUP}").exists()] + staged = _stage(extdir, versions) + try: + yield staged, died + finally: + _unstage(staged) def _target_and_previous(): @@ -33,23 +124,31 @@ def _target_and_previous(): return target, prev +def _admin(pgc_cluster, sql): + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as c: + c.execute(sql) + + +@pytest.fixture(scope="module") +def staging(pgc_cluster): + """Stage every fixture once for the module. -> (extdir, staged paths, versions whose + backup was already there before staging, which means an earlier run died).""" + extdir = _extdir(pgc_cluster) + with _staged(extdir, VERSIONS) as (staged, died): + yield extdir, staged, died + + @pytest.fixture -def window_db(pgc_cluster): +def window_db(pgc_cluster, staging): """-> (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) + extdir, staged, _ = staging + assert extdir / f"pgcolumnar--{prev}.sql" in staged, f"{prev} is not among the staged fixtures" def admin(sql): - with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as c: - c.execute(sql) + _admin(pgc_cluster, sql) admin("DROP DATABASE IF EXISTS pgc_upgrade_window") admin("CREATE DATABASE pgc_upgrade_window") @@ -60,10 +159,6 @@ def admin(sql): 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): @@ -92,6 +187,59 @@ def _succeeds(dsn, *sqls): return f"refused ({exc.sqlstate}: {str(exc).splitlines()[0]})" +def test_a_run_after_one_that_died_keeps_the_original(tmp_path, expect): + """A run that dies before `_unstage` leaves the fixture installed and the original in + the backup. The next run must not back up again: that copies the fixture over the + original, and the prefix's own file is gone for good.""" + v = "1.0-alpha4" + dst = tmp_path / f"pgcolumnar--{v}.sql" + shutil.copy2(FIXTURES / dst.name, dst) + original = "-- the prefix's own install script\n" + dst.with_name(dst.name + BACKUP).write_text(original) + _unstage(_stage(tmp_path, [v])) + left = dst.read_text() if dst.exists() else None + expect.text({original: "the prefix's own file", None: "no file"}.get( + left, "the fixture" if left == (FIXTURES / dst.name).read_text() + else "some other file"), + "the prefix's own file", + "a run after one that died restores the prefix's own file, not the fixture") + + +def test_two_stagings_at_once_keep_the_original(tmp_path, expect): + """Two processes staging one extension directory at once, as `pytest -n` workers + do. Unserialised, B keeps A's backup and restores it, and then A, finding no backup, + deletes the file it takes for its own fixture: the prefix's original (review of + #1536). B starts inside A's staging and is given a moment to interleave.""" + v = "1.0-alpha4" + dst = tmp_path / f"pgcolumnar--{v}.sql" + original = "-- the prefix's own install script\n" + dst.write_text(original) + a_in, a_go = threading.Event(), threading.Event() + + def a(): + with _staged(tmp_path, [v]): + a_in.set() + a_go.wait(10) + + def b(): + with _staged(tmp_path, [v]): + pass + + ta = threading.Thread(target=a) + ta.start() + a_in.wait(10) + tb = threading.Thread(target=b) + tb.start() + tb.join(0.5) + a_go.set() + ta.join(10) + tb.join(10) + left = dst.read_text() if dst.exists() else None + expect.text({original: "the prefix's own file", None: "no file"}.get(left, "some other file"), + "the prefix's own file", + "two stagings at once leave the prefix's own file in place") + + 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, @@ -124,3 +272,68 @@ def rows(): "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") + + +def _snapshot(dsn): + with psycopg.connect(dsn, autocommit=True) as c: + return sorted("|".join(r) for r in c.execute(SNAPSHOT).fetchall()) + + +@pytest.fixture(scope="module") +def reference(pgc_cluster): + """-> (target version, snapshot of a fresh install of it): what every upgrade must reach.""" + target, _ = _target_and_previous() + _admin(pgc_cluster, "DROP DATABASE IF EXISTS pgc_conv_fresh") + _admin(pgc_cluster, "CREATE DATABASE pgc_conv_fresh") + try: + dsn = pgc_cluster.dsn(dbname="pgc_conv_fresh") + with psycopg.connect(dsn, autocommit=True) as c: + c.execute(f"CREATE EXTENSION pgcolumnar VERSION '{target}'") + yield target, _snapshot(dsn) + finally: + _admin(pgc_cluster, "DROP DATABASE IF EXISTS pgc_conv_fresh WITH (FORCE)") + + +def test_the_staged_fixtures_are_the_committed_ones(staging, expect): + extdir, staged, died = staging + target, _ = _target_and_previous() + expect.text(target, "1.0-alpha5", "control default_version is 1.0-alpha5") + expect.text(" ".join(died) or "none", "none", + "premise: no earlier run of this suite died with a backup still staged") + expect.num(len(staged), len(VERSIONS), "premise: every fixture named for staging was staged") + wrong = [p.name for p in staged if p.read_bytes() != (FIXTURES / p.name).read_bytes()] + expect.text(" ".join(wrong) or "none", "none", + "every staged install script is the committed fixture, not a leftover") + + +@pytest.mark.parametrize("from_", VERSIONS) +def test_each_release_upgrades_to_a_fresh_catalog(pgc_cluster, staging, reference, from_, expect): + extdir, _, _ = staging + target, ref = reference + # In this test, not one of its own: two empty snapshots compare equal, so the + # comparison below is evidence only where this holds. + expect.at_least(len(ref), 101, f"fresh {target} install has objects to compare") + if not (extdir / f"pgcolumnar--{from_}.sql").exists(): + expect.text("missing", "present", f"fixture for {from_} present") + db = "pgc_conv_from_" + re.sub(r"[.-]", "_", from_) + _admin(pgc_cluster, f"DROP DATABASE IF EXISTS {db}") + _admin(pgc_cluster, f"CREATE DATABASE {db}") + try: + dsn = pgc_cluster.dsn(dbname=db) + with psycopg.connect(dsn, autocommit=True) as c: + c.execute(f"CREATE EXTENSION pgcolumnar VERSION '{from_}'") + try: + with psycopg.connect(dsn, autocommit=True) as c: + c.execute(f"ALTER EXTENSION pgcolumnar UPDATE TO '{target}'") + applied = "OK" + except psycopg.Error as exc: + applied = query_error(f"{exc.sqlstate}: {str(exc).splitlines()[0]}") + expect.text(applied, "OK", f"{from_} -> {target} upgrade applies without error") + expect.text(_one(dsn, "SELECT extversion FROM pg_extension WHERE extname = 'pgcolumnar'"), + target, f"{from_} is at {target} after upgrade") + got = _snapshot(dsn) + diverged = list(difflib.unified_diff(ref, got, "fresh", from_, n=0, lineterm=""))[:40] + expect.text("\n".join(diverged) or "CONVERGED", "CONVERGED", + f"{from_} -> {target} converges to a fresh {target} catalog") + finally: + _admin(pgc_cluster, f"DROP DATABASE IF EXISTS {db} WITH (FORCE)")