From 81cca0399f34efe6745955eb177303de70ffa8a0 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 4 Aug 2026 18:58:48 -0600 Subject: [PATCH 1/2] test: pin REPACK's actual behaviour, and name it in the error (#399) design/PG18_19_OPPORTUNITIES.md concluded that REPACK "should work" on a columnar table, because REPACK reuses the CLUSTER machinery and pgColumnar implements relation_copy_for_cluster. It does not work. The callback is registered and is a stub: pgcolumnar_relation_copy_for_cluster(...) { COLUMNAR_UNSUPPORTED("CLUSTER / VACUUM FULL"); } So "we implement that callback" was true of the symbol and false of the behaviour, which is the gap a suite closes and a grep does not. Measured on 19beta2: REPACK, REPACK ... USING INDEX, REPACK (VERBOSE), CLUSTER and VACUUM FULL all raise on a columnar table, while REPACK succeeds on a heap table on the same build. REPACK CONCURRENTLY is not the syntax; it is REPACK (CONCURRENTLY), and that form fails on heap too, so it is not a columnar behaviour and the suite records it rather than attributing it to us. The user-visible defect is the message. A PostgreSQL 19 user types REPACK and is told that CLUSTER / VACUUM FULL is unsupported: two commands they did not type, and on 19 the ones REPACK replaced. It now names REPACK on 19 and hints at pgcolumnar.vacuum(), which does the work. This is a spelling difference rather than a missing capability, and the error is where someone will look for that. test/native_repack.sh, registered, gated to 19 with a VISIBLE skip below it. It asserts the behaviour that exists rather than the behaviour that was hoped for: the failure of every spelling, that the message names the command, that the table is unharmed afterwards (rows, content hash, storage options, access method), and that pgcolumnar.vacuum() succeeds and preserves content. The heap control is there so a future "REPACK broke" can be told apart from "REPACK never worked here". If someone later implements relation_copy_for_cluster these checks fail, and that is the intended signal to rewrite them to assert success. Proven by removal: on unmodified main only the "names REPACK" check fails, and every other check passes on both builds, because they pin behaviour rather than detect the change. docs/limitations.md now lists REPACK beside CLUSTER and VACUUM FULL. The design document is corrected in place rather than quietly, since it is what produced the wrong expectation. Gate: five-major matrix, ALL VERSIONS PASSED, native_repack PASS on all five. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 +++ design/PG18_19_OPPORTUNITIES.md | 10 +++ docs/limitations.md | 6 +- src/columnar_tableam.c | 24 +++++++- test/native_repack.sh | 106 ++++++++++++++++++++++++++++++++ test/run_all_versions.sh | 2 +- 6 files changed, 151 insertions(+), 4 deletions(-) create mode 100755 test/native_repack.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 11158bb..f845faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ which was true until that script existed. ### Changed +- The unsupported-rewrite error names `REPACK` on PostgreSQL 19 (#399). `REPACK` + replaces `CLUSTER` and `VACUUM FULL` in 19 and dispatches through the same + copy-for-cluster path, which pgColumnar does not implement, so a 19 user who + typed `REPACK` was told that `CLUSTER / VACUUM FULL` was unsupported: two + commands they had not typed, and on 19 the superseded ones. The message now + names the command and hints at `pgcolumnar.vacuum()`, which does the work. + - The extension's exported C symbols are namespaced under `pgcolumnar` (#382). Two extensions that both call themselves `columnar` could define the same symbol. `columnar_handler` and `columnar_relation_storageid` collided with diff --git a/design/PG18_19_OPPORTUNITIES.md b/design/PG18_19_OPPORTUNITIES.md index 4ba3aca..4817f93 100644 --- a/design/PG18_19_OPPORTUNITIES.md +++ b/design/PG18_19_OPPORTUNITIES.md @@ -97,6 +97,16 @@ extension)" and "Allow ReadStream to be consumed as raw block numbers". something the AM opts into. Concurrent REPACK on a columnar table is therefore unverified and likely needs additional work; treat it as future work. + **CORRECTED 2026-08-05 (#399). It does not work, and this entry is why inspection + was not enough.** `pgcolumnar_relation_copy_for_cluster` is *registered* but is a + stub that unconditionally raises `COLUMNAR_UNSUPPORTED`, so "pgColumnar already + implements that callback" was true of the symbol and false of the behaviour. + Measured on 19beta2: `REPACK`, `REPACK ... USING INDEX`, `REPACK (VERBOSE)`, + `CLUSTER` and `VACUUM FULL` all raise, while `REPACK` succeeds on a heap table on + the same build. `REPACK CONCURRENTLY` is not the syntax; it is + `REPACK (CONCURRENTLY)`, and that form fails on heap too, so it is not ours. + Pinned by `test/native_repack.sh`. The supported route is `pgcolumnar.vacuum()`. + ## 6. Optimizer statistics injection (PostgreSQL 18) - `pg_restore_relation_stats()`, `pg_restore_attribute_stats()`, diff --git a/docs/limitations.md b/docs/limitations.md index 0d73153..e587400 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -268,8 +268,10 @@ returning no rows. ## Vacuum and compaction -- `VACUUM FULL` and `CLUSTER` are not supported on a columnar table; the - copy-for-cluster path raises an error. Use `pgcolumnar.vacuum` or +- `REPACK`, `VACUUM FULL` and `CLUSTER` are not supported on a columnar table; the + copy-for-cluster path raises an error. `REPACK` arrives in PostgreSQL 19 and + replaces the other two, and it dispatches through the same path, so it is + refused for the same reason. Use `pgcolumnar.vacuum` or `pgcolumnar.vacuum_full` instead. - `pgcolumnar.vacuum` always rewrites the whole relation into full row groups. It accepts a `stripe_count` argument for compatibility with the interface, but it diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 259d30a..b497d69 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -1383,7 +1383,29 @@ pgcolumnar_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) static void pgcolumnar_relation_copy_for_cluster(COLUMNAR_COPY_FOR_CLUSTER_ARGS) { - COLUMNAR_UNSUPPORTED("CLUSTER / VACUUM FULL"); + /* + * Name every command that reaches here, and name them per major (#399). + * + * PostgreSQL 19 adds REPACK, which replaces CLUSTER and VACUUM FULL and + * dispatches through this same callback. A 19 user who types REPACK was + * previously told "CLUSTER / VACUUM FULL is not supported yet", naming two + * commands they did not type and, on 19, the superseded ones. The error + * should describe what the user asked for. + * + * The hint matters more than the message: the operation is available, under + * a different name. pgcolumnar.vacuum() reclaims space and the ordering + * functions cluster, so this is a spelling difference rather than a missing + * capability, and the error is where someone will look for that. + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), +#if PG_VERSION_NUM >= 190000 + errmsg("columnar: REPACK, CLUSTER and VACUUM FULL are not supported yet"), +#else + errmsg("columnar: CLUSTER and VACUUM FULL are not supported yet"), +#endif + errhint("Use pgcolumnar.vacuum() to reclaim space, or " + "pgcolumnar.vacuum_sorted() to rewrite in sorted order."))); } /* diff --git a/test/native_repack.sh b/test/native_repack.sh new file mode 100755 index 0000000..c9b5367 --- /dev/null +++ b/test/native_repack.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# +# REPACK on a columnar table (issue #399). PostgreSQL 19 and later only. +# +# REPACK replaces CLUSTER and VACUUM FULL in 19. It is not a new table-AM +# callback: it reuses the CLUSTER machinery, which dispatches a rewrite through +# relation_copy_for_cluster. design/PG18_19_OPPORTUNITIES.md concluded from that +# dispatch that REPACK "should work" on a columnar table, because pgColumnar +# registers that callback. +# +# It does not work. The callback is registered and is a stub that raises, so the +# reasoning was true of the symbol and false of the behaviour. That is the gap a +# suite closes and a grep does not, and it is why this file exists. +# +# What is asserted here is therefore the ACTUAL behaviour, not the hoped-for one: +# every spelling of the command errors, it errors naming the command the user +# typed, the table is unharmed afterwards, and the supported alternative works. +# If someone later implements relation_copy_for_cluster, these checks fail and +# should be rewritten to assert success. That is the intended signal. +# +# The heap control is not decoration. Without it, "REPACK fails here" cannot be +# told apart from "REPACK does not work on this build at all", which matters for +# REPACK (CONCURRENTLY), whose failure is not columnar-specific. +# +# Usage: test/native_repack.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +pgc_setup "${1:-/usr/local/pg19/bin/pg_config}" + +# Version gate, and it must be VISIBLE. A suite that silently passes on 15 to 18 +# is the failure mode this project keeps finding. +srv="$(q 'SHOW server_version_num')" +if [ "${srv:-0}" -lt 190000 ]; then + echo "SKIP REPACK requires PostgreSQL 19 (server_version_num=$srv)" + echo "native_repack.sh: SKIPPED" + exit 0 +fi + +ROWS=${PGC_REPACK_ROWS:-20000} + +psql_run "CREATE TABLE rp (id int, v text) USING pgcolumnar; + SELECT pgcolumnar.set_options('rp', stripe_row_limit => 5000, compression => 'zstd'); + INSERT INTO rp SELECT g, 'x'||g FROM generate_series(1,$ROWS) g; + DELETE FROM rp WHERE id % 3 = 0; + CREATE INDEX rp_id ON rp (id); + CREATE TABLE rp_heap (id int, v text); + INSERT INTO rp_heap SELECT g, 'x'||g FROM generate_series(1,$ROWS) g; + CREATE INDEX rp_heap_id ON rp_heap (id);" + +live=$(q "SELECT count(*) FROM rp") +hash_before=$(q "SELECT md5(string_agg(id::text||':'||v, ',' ORDER BY id)) FROM rp") +opts_before=$(q "SELECT stripe_row_limit || '/' || compression FROM pgcolumnar.options o + JOIN pg_class c ON c.oid = o.regclass WHERE c.relname = 'rp'") + +err_of() { psql_run "$1" 2>&1 | grep -oE 'ERROR:.*' | head -1; } + +# ---- every spelling errors, and the message names the command ---------------- +e="$(err_of 'REPACK rp;')" +check "REPACK on a columnar table errors (#399)" \ + "$(case "$e" in *ERROR*) echo yes ;; *) echo "no (succeeded)" ;; esac)" "yes" +check "and the error names REPACK, which is what the user typed (#399)" \ + "$(case "$e" in *REPACK*) echo yes ;; *) echo "no ($e)" ;; esac)" "yes" + +check "REPACK ... USING INDEX errors the same way" \ + "$(case "$(err_of 'REPACK rp USING INDEX rp_id;')" in *"not supported"*) echo yes ;; *) echo no ;; esac)" "yes" +check "REPACK (VERBOSE) errors the same way" \ + "$(case "$(err_of 'REPACK (VERBOSE) rp;')" in *"not supported"*) echo yes ;; *) echo no ;; esac)" "yes" +check "CLUSTER errors the same way" \ + "$(case "$(err_of 'CLUSTER rp USING rp_id;')" in *"not supported"*) echo yes ;; *) echo no ;; esac)" "yes" +check "VACUUM FULL errors the same way" \ + "$(case "$(err_of 'VACUUM FULL rp;')" in *"not supported"*) echo yes ;; *) echo no ;; esac)" "yes" + +# REPACK (CONCURRENTLY) fails on heap too on this build, so it is recorded rather +# than attributed to us. "REPACK CONCURRENTLY" without parentheses is not syntax. +c_col="$(err_of 'REPACK (CONCURRENTLY) rp;')" +c_heap="$(err_of 'REPACK (CONCURRENTLY) rp_heap;')" +check "REPACK (CONCURRENTLY) fails on columnar and on heap alike, so it is not ours" \ + "$( [ -n "$c_col" ] && [ -n "$c_heap" ] && echo yes || echo "no (col=[$c_col] heap=[$c_heap])")" \ + "yes" + +# ---- the control: the same command works on heap ----------------------------- +check "control: REPACK works on a heap table on this build" \ + "$(psql_run 'REPACK rp_heap;' 2>&1 | grep -c ERROR || true)" "0" + +# ---- a failed rewrite must leave the table untouched ------------------------- +check "the columnar table is unharmed: row count" "$(q 'SELECT count(*) FROM rp')" "$live" +check "the columnar table is unharmed: content hash" \ + "$(q "SELECT md5(string_agg(id::text||':'||v, ',' ORDER BY id)) FROM rp")" "$hash_before" +check "the columnar table is unharmed: storage options" \ + "$(q "SELECT stripe_row_limit || '/' || compression FROM pgcolumnar.options o + JOIN pg_class c ON c.oid = o.regclass WHERE c.relname = 'rp'")" "$opts_before" +check "the columnar table is unharmed: access method" \ + "$(q "SELECT amname FROM pg_am a JOIN pg_class c ON c.relam = a.oid WHERE c.relname = 'rp'")" \ + "pgcolumnar" + +# ---- and the supported route does work --------------------------------------- +check "pgcolumnar.vacuum is the supported alternative and succeeds" \ + "$(psql_run "SELECT pgcolumnar.vacuum('rp');" 2>&1 | grep -c ERROR || true)" "0" +check "and it kept every live row" "$(q 'SELECT count(*) FROM rp')" "$live" +check "and the content is still identical" \ + "$(q "SELECT md5(string_agg(id::text||':'||v, ',' ORDER BY id)) FROM rp")" "$hash_before" + +pgc_summary diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 5a5365b..3e9d7da 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -208,7 +208,7 @@ SRCDIR="${PGC_RUN_SRCDIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" SUITES=(harness_selftest docs_style smoke phase2 phase3 phase4 phase5 phase6 audit concurrency unique_conc \ differential recovery replication native_backend_crash fuzz fuzz_parquet fuzz_arrow hardening concurrent_diff parallel sorted_projection \ arrow_export parquet_export read_stream corruption \ - generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_fastdecode native_zonemap write_minmax_fastpath write_fsst_compressed fsst_margin encode_invariants encode_effort native_skip pushdown_report native_agg native_agg_deletes native_agg_addcolumn native_groupagg ungrouped_vector_agg parallel_vector_agg native_bloom bloom_setting bloom_lazy native_vecskip native_index native_fetch_position native_dml alter_column_type native_ios native_projection native_cluster native_compact native_recluster recluster_extent native_vacuum_race native_sort_by sort_status native_reclaim native_ownership drop_cleanup pg_dump_roundtrip native_reclaim_cycles native_reclaim_frag native_reclaim_reconcile native_gap native_format native_truncate native_rewrite native_rewrite_conc rewrite_group_scan native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening server_file_privilege native_parquet_stack native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel cancel_decode wal_envelope decode_interrupts import_exclusion import_deferred parallel_copy parallel_export_parquet fk_referencing row_triggers native_lazy_slot native_fetch_cache native_fetch_interrupt analyze_stats analyze_reltuples native_fetch_projection column_projection isolation) + generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_fastdecode native_zonemap write_minmax_fastpath write_fsst_compressed fsst_margin encode_invariants encode_effort native_skip pushdown_report native_agg native_agg_deletes native_agg_addcolumn native_groupagg ungrouped_vector_agg parallel_vector_agg native_bloom bloom_setting bloom_lazy native_vecskip native_index native_fetch_position native_dml alter_column_type native_ios native_projection native_cluster native_repack native_compact native_recluster recluster_extent native_vacuum_race native_sort_by sort_status native_reclaim native_ownership drop_cleanup pg_dump_roundtrip native_reclaim_cycles native_reclaim_frag native_reclaim_reconcile native_gap native_format native_truncate native_rewrite native_rewrite_conc rewrite_group_scan native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening server_file_privilege native_parquet_stack native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel cancel_decode wal_envelope decode_interrupts import_exclusion import_deferred parallel_copy parallel_export_parquet fk_referencing row_triggers native_lazy_slot native_fetch_cache native_fetch_interrupt analyze_stats analyze_reltuples native_fetch_projection column_projection isolation) # Default matrix: one assert-enabled pg_config per major, 15 through 19. DEFAULT_CONFIGS=( From e695e099e59686cbb608e16dc441a391c3dd995e Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 4 Aug 2026 19:44:26 -0600 Subject: [PATCH 2/2] test: REPACK (CONCURRENTLY) is ours after all, given an identity index Review catch from ChronicallyJD, and it is the good kind: my check passed for a reason that had nothing to do with what it claimed. I asserted that REPACK (CONCURRENTLY) "fails on columnar and on heap alike, so it is not ours". Both did fail, but only because the fixture had no identity index, which is a precondition PostgreSQL enforces before the access method is consulted. A test whose meaning rests on an incidental property of its fixture keeps passing after the behaviour it was written to catch has changed. With a primary key and wal_level=logical, heap succeeds and a columnar table raises OUR error. So the concurrent form is a columnar behaviour, and the earlier conclusion was wrong in the PR body, the changelog and the design note. All three are corrected. The suite now carries both fixtures and asserts the distinction rather than the outcome: without an identity index the refusal is PostgreSQL's own precondition error, and with one it is ours. Two different failures that a single "it errors" check conflated. Also asserts what the review found on the way: after the refused concurrent repack the columnar table is undamaged, rows and content hash unchanged. That was the fourth bullet's worry on #399 and the answer is reassuring, so it is pinned. The cluster needs wal_level=logical and room in the lock table for the concurrent form to be reachable at all; without them it fails for reasons unrelated to the access method, which is the same trap one level down. 19 checks, all passing on PG19, skipping visibly below it. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +- design/PG18_19_OPPORTUNITIES.md | 6 ++- test/native_repack.sh | 65 +++++++++++++++++++++++++++++---- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f845faa..fe34c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,9 @@ which was true until that script existed. copy-for-cluster path, which pgColumnar does not implement, so a 19 user who typed `REPACK` was told that `CLUSTER / VACUUM FULL` was unsupported: two commands they had not typed, and on 19 the superseded ones. The message now - names the command and hints at `pgcolumnar.vacuum()`, which does the work. + names the command and hints at `pgcolumnar.vacuum()`, which does the work. This + covers `REPACK (CONCURRENTLY)` too: given a table with an identity index, where + PostgreSQL will run it, heap succeeds and a columnar table is refused. - The extension's exported C symbols are namespaced under `pgcolumnar` (#382). Two extensions that both call themselves `columnar` could define the same diff --git a/design/PG18_19_OPPORTUNITIES.md b/design/PG18_19_OPPORTUNITIES.md index 4817f93..92856da 100644 --- a/design/PG18_19_OPPORTUNITIES.md +++ b/design/PG18_19_OPPORTUNITIES.md @@ -104,7 +104,11 @@ extension)" and "Allow ReadStream to be consumed as raw block numbers". Measured on 19beta2: `REPACK`, `REPACK ... USING INDEX`, `REPACK (VERBOSE)`, `CLUSTER` and `VACUUM FULL` all raise, while `REPACK` succeeds on a heap table on the same build. `REPACK CONCURRENTLY` is not the syntax; it is - `REPACK (CONCURRENTLY)`, and that form fails on heap too, so it is not ours. + `REPACK (CONCURRENTLY)`. That form **is** ours: on a fixture with no identity index + both access methods are refused by PostgreSQL before the AM is reached, which is + what an early reading of this mistook for "not columnar-specific". With a primary + key and `wal_level=logical`, heap succeeds and columnar raises our error. The + columnar table is undamaged afterwards. Pinned by `test/native_repack.sh`. The supported route is `pgcolumnar.vacuum()`. ## 6. Optimizer statistics injection (PostgreSQL 18) diff --git a/test/native_repack.sh b/test/native_repack.sh index c9b5367..3c2ad27 100755 --- a/test/native_repack.sh +++ b/test/native_repack.sh @@ -19,8 +19,14 @@ # should be rewritten to assert success. That is the intended signal. # # The heap control is not decoration. Without it, "REPACK fails here" cannot be -# told apart from "REPACK does not work on this build at all", which matters for -# REPACK (CONCURRENTLY), whose failure is not columnar-specific. +# told apart from "REPACK does not work on this build at all". +# +# REPACK (CONCURRENTLY) needs two fixtures for the same reason, and the first +# version of this suite got it wrong. Without an identity index both access +# methods are refused before the AM is reached, so asserting that failure alone +# passes for a reason that has nothing to do with us, and would keep passing if +# our behaviour changed. With a primary key and wal_level=logical, heap succeeds +# and columnar raises our error, so the concurrent form IS a columnar behaviour. # # Usage: test/native_repack.sh [PG_CONFIG] # Written fresh for pgColumnar. @@ -28,6 +34,13 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# REPACK (CONCURRENTLY) needs logical decoding of the relation's changes, so the +# cluster must be able to provide it. Without this the concurrent form fails for a +# reason that has nothing to do with the access method. +PGC_EXTRA_CONF="wal_level=logical +max_locks_per_transaction=1024 +max_wal_senders=10 +max_replication_slots=10" pgc_setup "${1:-/usr/local/pg19/bin/pg_config}" # Version gate, and it must be VISIBLE. A suite that silently passes on 15 to 18 @@ -73,14 +86,50 @@ check "CLUSTER errors the same way" \ check "VACUUM FULL errors the same way" \ "$(case "$(err_of 'VACUUM FULL rp;')" in *"not supported"*) echo yes ;; *) echo no ;; esac)" "yes" -# REPACK (CONCURRENTLY) fails on heap too on this build, so it is recorded rather -# than attributed to us. "REPACK CONCURRENTLY" without parentheses is not syntax. -c_col="$(err_of 'REPACK (CONCURRENTLY) rp;')" -c_heap="$(err_of 'REPACK (CONCURRENTLY) rp_heap;')" -check "REPACK (CONCURRENTLY) fails on columnar and on heap alike, so it is not ours" \ - "$( [ -n "$c_col" ] && [ -n "$c_heap" ] && echo yes || echo "no (col=[$c_col] heap=[$c_heap])")" \ +# ---- REPACK (CONCURRENTLY), which needs two fixtures to mean anything --------- +# "REPACK CONCURRENTLY" without parentheses is not syntax; it is an option. +# +# The concurrent form has a precondition: the relation needs an identity index. On +# a table without one BOTH access methods fail, and the failure says so. Asserting +# only that is a trap, because the assertion passes for the wrong reason and would +# keep passing if the columnar behaviour changed. It is kept below, labelled as +# what it is, and paired with a fixture that MEETS the precondition. +# The distinction is the assertion: without an identity index the error is +# PostgreSQL's own precondition ("cannot execute REPACK (CONCURRENTLY) on +# relation"), raised before the access method is consulted. With one, below, the +# error is ours. Two different failures that a single "it errors" check would +# conflate. +nopk="$(err_of 'REPACK (CONCURRENTLY) rp;')" +check "without an identity index it is refused by PostgreSQL, not by us" \ + "$(case "$nopk" in + *"not supported"*) echo "no (ours: $nopk)" ;; + *"cannot execute REPACK"*) echo yes ;; + *) echo "no ($nopk)" ;; + esac)" \ "yes" +psql_run "CREATE TABLE rp_pk (id int PRIMARY KEY, v text) USING pgcolumnar; + INSERT INTO rp_pk SELECT g, 'x'||g FROM generate_series(1,$ROWS) g; + CREATE TABLE rp_pk_heap (id int PRIMARY KEY, v text); + INSERT INTO rp_pk_heap SELECT g, 'x'||g FROM generate_series(1,$ROWS) g;" +pk_rows="$(q 'SELECT count(*) FROM rp_pk')" +pk_hash="$(q "SELECT md5(string_agg(id::text||':'||v, ',' ORDER BY id)) FROM rp_pk")" + +# With the precondition met, heap succeeds. So the concurrent form IS reachable on +# this build, and a columnar failure is ours rather than a missing primary key. +check "control: with an identity index, REPACK (CONCURRENTLY) succeeds on heap" \ + "$(psql_run 'REPACK (CONCURRENTLY) rp_pk_heap;' 2>&1 | grep -c 'ERROR' || true)" "0" + +c_pk="$(err_of 'REPACK (CONCURRENTLY) rp_pk;')" +check "and on a columnar table it raises OUR error, so it is a columnar behaviour" \ + "$(case "$c_pk" in *"not supported"*) echo yes ;; *) echo "no ($c_pk)" ;; esac)" "yes" + +# A half-finished concurrent rewrite would be the bad outcome. It does not happen. +check "the columnar table is undamaged after the refused concurrent repack: rows" \ + "$(q 'SELECT count(*) FROM rp_pk')" "$pk_rows" +check "the columnar table is undamaged after the refused concurrent repack: content" \ + "$(q "SELECT md5(string_agg(id::text||':'||v, ',' ORDER BY id)) FROM rp_pk")" "$pk_hash" + # ---- the control: the same command works on heap ----------------------------- check "control: REPACK works on a heap table on this build" \ "$(psql_run 'REPACK rp_heap;' 2>&1 | grep -c ERROR || true)" "0"