From 05e836cebc271eebd052f9d4672288e5b0598eda Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:34:00 -0600 Subject: [PATCH 1/2] fix: reject NaN compaction thresholds Co-authored-by: Cursor --- src/columnar_vacuum.c | 6 ++++-- test/native_reclaim.sh | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/columnar_vacuum.c b/src/columnar_vacuum.c index b041973b..548cb84f 100644 --- a/src/columnar_vacuum.c +++ b/src/columnar_vacuum.c @@ -25,6 +25,8 @@ #include "columnar_write_state.h" #include "columnar_compat.h" +#include + #include "fmgr.h" #include "access/genam.h" #include "access/xact.h" @@ -875,10 +877,10 @@ pgcolumnar_compact_rewrite(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("table name cannot be null"))); - if (minFrac < 0.0 || minFrac > 1.0) + if (isnan(minFrac) || minFrac < 0.0 || minFrac > 1.0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("min_deleted_fraction must be between 0 and 1"))); + errmsg("min_deleted_fraction must be a number between 0 and 1"))); PgColumnarRequireTableOwnerByOid(relid); diff --git a/test/native_reclaim.sh b/test/native_reclaim.sh index 008944b0..f29bc6e9 100755 --- a/test/native_reclaim.sh +++ b/test/native_reclaim.sh @@ -64,6 +64,17 @@ check "reused-block data matches heap mirror" \ # Reuse survives a delete + rewrite cycle too, with correct data. psql_run "DELETE FROM h WHERE id % 4 = 0;" psql_run "DELETE FROM n WHERE id % 4 = 0;" + +# NaN passes both ordinary range comparisons. Refuse it explicitly, or the +# candidate predicate is false for every group and compaction silently does no +# work despite an accepted threshold. +if psql_run "SELECT pgcolumnar.compact_rewrite('n', 'NaN'::float8);" >/dev/null 2>&1; then + nan_result="accepted" +else + nan_result="rejected" +fi +check "compact_rewrite rejects a NaN threshold" "$nan_result" "rejected" + psql_run "SELECT pgcolumnar.compact_rewrite('n', 0.0);" psql_run "SELECT pgcolumnar.recluster('n', 'id');" check "data correct after delete + compact_rewrite + recluster" \ From 6eb19735dd03c54541d5fb4840e9f1f9da0a304e Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Tue, 1 Sep 2026 21:41:33 +0000 Subject: [PATCH 2/2] test: read the SQLSTATE, and validate maintenance_due too Requested on review. Three things, each with a red arm proving it. The NaN arm was unconditionally "accepted". It decided with `if q "..."`, and lib.sh's q() ends in `|| true`, so it always exits 0 and the then-branch always ran: the arm read "accepted" on a fixed tree, an unfixed tree, and a tree with no such function. It now reads the SQLSTATE psql printed. The deny arms asserted only that the call failed, which a NULL argument, a missing function, a wrong arity or 1/0 all satisfy. They now assert 22023. The suite was also blind to the guard becoming OVER-BROAD, which is the direction a bounds check usually breaks: changing `minFrac < 0.0` to `<= 0.0` rejects the legal 0.0 and native_reclaim still reported 10 passed, 0 failed, PASSED, with the rejection visible only as an unasserted log line. Both endpoints are now pinned as accepted. And maintenance_due(), the gate the autovacuum daemon consults BEFORE it ever calls compact_rewrite, had no validation at all. Measured on a fresh 50%-deleted table, one table per value: maintenance_due(0.2) due = t correct maintenance_due(NaN) due = f suppresses the work silently maintenance_due(2.0) due = f maintenance_due(-1.0) due = t ALWAYS due, so the daemon never stops compact_rewrite(NaN | 2.0 | -1.0) -> ERROR, all three Both thresholds are now validated there, with arms for each rejected value and arms keeping 0.0 and 1.0 accepted. --- CHANGELOG.md | 29 +++++ docs/sql-reference.md | 20 +++- pgcolumnar--1.0-alpha2--1.0-alpha3.sql | 115 +++++++++++++++++++ pgcolumnar--1.0-alpha3.sql | 39 ++++++- test/native_reclaim.sh | 150 +++++++++++++++++++++++-- 5 files changed, 339 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7feec7a9..735a5047 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -150,6 +150,35 @@ true until the next version shipped. own range-table entry has `inh` false: the resulting plan is an `Append` of two `PgColumnarScan` nodes with pushdown intact, not a fallback to `Seq Scan`. +- Both compaction thresholds are validated, in both entry points (#860). + `pgcolumnar.compact_rewrite` accepted `NaN` for `min_deleted_fraction`, because + `NaN < 0.0` and `NaN > 1.0` are each false. The call then matched no row group + and reclaimed nothing while reporting success. + + `pgcolumnar.maintenance_due` is the gate the `pgcolumnar.autovacuum` daemon + consults before it ever calls `compact_rewrite`, and it validated nothing at + all. Measured on a table with 10000 of 20000 rows deleted: `NaN` and `2.0` both + reported `compact_rewrite_due = f`, suppressing the work for good, and `-1.0` + reported `t`, which makes the daemon believe compaction is always due and + rewrite every columnar table on every sweep. `NULL` behaved like `NaN`, because + the daemon reads a NULL verdict as "not due". Both thresholds now raise + `invalid_parameter_value` (SQLSTATE `22023`) for all four, matching + `compact_rewrite`. + + 0 and 1 remain legal on every threshold, and `test/native_reclaim.sh` now pins + both endpoints as accepted. That is the direction a bounds fix usually breaks, + and nothing could see it: the suite's only call at 0 was a bare `psql_run` + whose exit status nothing read, so a rejected 0 reached nothing but the server + log. Measured with `minFrac < 0.0` changed to `minFrac <= 0.0`: exactly one arm + fails, the new one, and the other 32 pass. The same mutation applied to + `compact_due_fraction` reddens the matching `maintenance_due` arm and nothing + else. + + Every deny arm asserts the SQLSTATE, not "it failed". Four calls that satisfy + "it failed" without touching the guard are pinned as controls: a missing + function and a wrong-arity call (`42883`), a null table name (`22004`) and + `1/0` (`22012`). + - A shebang and the execute bit go together, and every directory that documents a command is swept (#856). Two things were left over from #852. diff --git a/docs/sql-reference.md b/docs/sql-reference.md index 5b0d26d9..757b9154 100644 --- a/docs/sql-reference.md +++ b/docs/sql-reference.md @@ -200,6 +200,12 @@ Rewrites partially-deleted row groups, those whose deleted fraction is at least `ShareUpdateExclusiveLock`. `max_groups` caps how many groups a single call rewrites; 0 means no cap. Returns the number of groups rewritten. +`min_deleted_fraction` must be a number from 0 to 1, both ends included. The +function rejects `NaN`, a negative value and a value above 1. Each raises SQLSTATE +`22023`, `invalid_parameter_value`. `NaN` needs a test of its own because it +compares false against every bound. An accepted `NaN` would match no row group, so +the call would reclaim nothing and still report success. + ```sql SELECT pgcolumnar.compact_rewrite('events', 0.3); ``` @@ -389,9 +395,17 @@ monitoring query. Returns one row: | `recluster_due` | boolean | True when a sorted run exists and `appended_fraction` reaches `recluster_due_fraction`. | | `recommendation` | text | The verbs to run, comma-separated, or NULL when nothing is due. | -The two thresholds default to the values the daemon uses. The function is -`SECURITY DEFINER` and checks that the caller may `SELECT` the table. A monitoring -role that owns the table can therefore call it without superuser rights. +The two thresholds default to the values the daemon uses. Each must be a number +from 0 to 1, both ends included. The function rejects `NaN`, `NULL`, a negative +value and a value above 1. Each raises SQLSTATE `22023`, the same code and the +same bounds as `pgcolumnar.compact_rewrite`. An unchecked threshold fails silently +rather than loudly. `NaN`, `NULL` or a value above 1 reports nothing as due, which +suppresses maintenance for good. A negative value reports every table as due on +every sweep. + +The function is `SECURITY DEFINER` and checks that the caller may `SELECT` the +table. A monitoring role that owns the table can therefore call it without +superuser rights. ```sql SELECT recommendation FROM pgcolumnar.maintenance_due('events'); diff --git a/pgcolumnar--1.0-alpha2--1.0-alpha3.sql b/pgcolumnar--1.0-alpha2--1.0-alpha3.sql index 9b50bf87..d751acd8 100644 --- a/pgcolumnar--1.0-alpha2--1.0-alpha3.sql +++ b/pgcolumnar--1.0-alpha2--1.0-alpha3.sql @@ -296,3 +296,118 @@ COMMENT ON FUNCTION pgcolumnar.sort_status(regclass) COMMENT ON FUNCTION pgcolumnar.vacuum_sorted(regclass, name[]) IS 'compact a columnar table, storing rows sorted ascending (NULLS LAST) on the given columns. With no columns, applies the table''s declared sort_by key from set_options (#288), like a bare CLUSTER re-applying a remembered index; errors if none is declared. Supports any btree-orderable column including text and numeric, unlike Z-order cluster(), which takes integer, date/time, boolean and floating-point columns only. One-shot: not auto-maintained.'; + +-- pgcolumnar.maintenance_due(): validate both threshold parameters (#860). +-- Unvalidated, a NaN or above-1 threshold silently suppressed all +-- maintenance and a negative one made every table permanently "due", in the +-- gate the autovacuum daemon consults before calling compact_rewrite. +-- The body below is verbatim from pgcolumnar--1.0-alpha3.sql. +CREATE OR REPLACE FUNCTION pgcolumnar.maintenance_due( + rel regclass, + compact_due_fraction float8 DEFAULT 0.2, + recluster_due_fraction float8 DEFAULT 0.05, + OUT total_rows bigint, + OUT deleted_rows bigint, + OUT deleted_fraction float8, + OUT sort_key name[], + OUT appended_groups bigint, + OUT appended_rows bigint, + OUT appended_fraction float8, + OUT compact_rewrite_due boolean, + OUT recluster_due boolean, + OUT recommendation text) + RETURNS record + -- SECURITY DEFINER, mirroring stats(): the report reads pgcolumnar's internal + -- catalogs through sort_status(), which ordinary roles cannot SELECT, so an + -- invoker-rights function false-denied every non-superuser caller -- the + -- cron/monitoring role this report is for. require_caller_select (inside + -- stats()) still gates the REAL caller via GetOuterUserId(), so definer rights + -- do not widen who may read a table's statistics. search_path is pinned as a + -- definer function must. + LANGUAGE plpgsql STABLE SECURITY DEFINER + SET search_path = pg_catalog, pg_temp + AS $maintenance_due$ +DECLARE + st_rows bigint; + st_del bigint; + ss record; +BEGIN + -- Validate both thresholds before reading anything (#860). Neither one was + -- checked, and this is the gate the autovacuum daemon consults BEFORE it ever + -- calls compact_rewrite, which does check its own. Four ways an unchecked + -- threshold goes wrong, none of which raises anything: + -- NaN -- `fraction >= NaN` is false in IEEE, so nothing is ever due and + -- the work is suppressed silently and permanently. + -- > 1 -- the same outcome for any fraction: never due. + -- < 0 -- `fraction >= -1` is true for EVERY table, so the daemon believes + -- compaction is always due and rewrites every columnar table on + -- every pass. This is the dangerous direction: not a suppressed + -- report but a permanent, self-renewing rewrite. + -- NULL -- the verdict is NULL, and the daemon reads a NULL verdict as + -- "not due" (SPI_getbinval isnull), so it is the NaN case again. + -- 0.0 and 1.0 are LEGAL and stay legal: 0.0 means "any decay at all is worth + -- acting on", 1.0 means "only a fully dead table". The bounds are inclusive, + -- matching pgcolumnar.compact_rewrite's own guard, and test/native_reclaim.sh + -- pins both endpoints as ACCEPTED so this guard cannot quietly become + -- over-broad, which is how a bounds check usually breaks. + -- + -- The explicit NaN test is redundant with `> 1.0` today, because PostgreSQL + -- float8 ordering is not IEEE ordering: it sorts NaN above every other value. + -- It is written out anyway so the intent survives an edit to the bounds. + IF compact_due_fraction IS NULL + OR compact_due_fraction = 'NaN'::float8 + OR compact_due_fraction < 0.0 + OR compact_due_fraction > 1.0 THEN + RAISE EXCEPTION 'compact_due_fraction must be a number between 0 and 1' + USING ERRCODE = 'invalid_parameter_value'; + END IF; + IF recluster_due_fraction IS NULL + OR recluster_due_fraction = 'NaN'::float8 + OR recluster_due_fraction < 0.0 + OR recluster_due_fraction > 1.0 THEN + RAISE EXCEPTION 'recluster_due_fraction must be a number between 0 and 1' + USING ERRCODE = 'invalid_parameter_value'; + END IF; + + -- stats() enforces require_caller_select(rel) before it returns a row, so a + -- caller without SELECT on rel is refused here rather than reported to. + SELECT COALESCE(sum(s.rowcount), 0), COALESCE(sum(s.deletedrows), 0) + INTO st_rows, st_del + FROM pgcolumnar.stats(rel) s; + + SELECT * INTO ss FROM pgcolumnar.sort_status(rel); + + total_rows := st_rows; + deleted_rows := st_del; + deleted_fraction := CASE WHEN st_rows > 0 + THEN st_del::float8 / st_rows ELSE 0 END; + + sort_key := ss.sort_key; + appended_groups := ss.appended_groups; + appended_rows := ss.appended_rows; + appended_fraction := CASE WHEN (ss.sorted_rows + ss.appended_rows) > 0 + THEN ss.appended_rows::float8 + / (ss.sorted_rows + ss.appended_rows) + ELSE 0 END; + + compact_rewrite_due := (deleted_fraction >= compact_due_fraction); + -- A sorted RUN must exist for recluster to mean anything. sort_status() + -- reports a never-ordered table as entirely appended (no run), and + -- vacuum_sorted() establishes a run without setting options.sort_by, so the + -- run -- sorted_groups > 0 -- is the signal, not the sort_by label (sort_key + -- is reported for information and may be NULL on an ordered table). + recluster_due := (ss.sorted_groups > 0 + AND ss.appended_groups > 0 + AND appended_fraction >= recluster_due_fraction); + + recommendation := NULLIF( + concat_ws(', ', + CASE WHEN compact_rewrite_due THEN 'compact_rewrite' END, + CASE WHEN recluster_due THEN 'recluster' END), + ''); + RETURN; +END; +$maintenance_due$; + +COMMENT ON FUNCTION pgcolumnar.maintenance_due(regclass, float8, float8) + IS 'report whether an online maintenance verb (compact_rewrite, recluster) is worth running, from table statistics alone; thresholds are parameters with defaults measured on #415, each required to be a number between 0 and 1 inclusive (#860); pure report, takes no lock and rewrites nothing (#415)'; diff --git a/pgcolumnar--1.0-alpha3.sql b/pgcolumnar--1.0-alpha3.sql index 56596d89..e2743fb2 100644 --- a/pgcolumnar--1.0-alpha3.sql +++ b/pgcolumnar--1.0-alpha3.sql @@ -1788,6 +1788,43 @@ DECLARE st_del bigint; ss record; BEGIN + -- Validate both thresholds before reading anything (#860). Neither one was + -- checked, and this is the gate the autovacuum daemon consults BEFORE it ever + -- calls compact_rewrite, which does check its own. Four ways an unchecked + -- threshold goes wrong, none of which raises anything: + -- NaN -- `fraction >= NaN` is false in IEEE, so nothing is ever due and + -- the work is suppressed silently and permanently. + -- > 1 -- the same outcome for any fraction: never due. + -- < 0 -- `fraction >= -1` is true for EVERY table, so the daemon believes + -- compaction is always due and rewrites every columnar table on + -- every pass. This is the dangerous direction: not a suppressed + -- report but a permanent, self-renewing rewrite. + -- NULL -- the verdict is NULL, and the daemon reads a NULL verdict as + -- "not due" (SPI_getbinval isnull), so it is the NaN case again. + -- 0.0 and 1.0 are LEGAL and stay legal: 0.0 means "any decay at all is worth + -- acting on", 1.0 means "only a fully dead table". The bounds are inclusive, + -- matching pgcolumnar.compact_rewrite's own guard, and test/native_reclaim.sh + -- pins both endpoints as ACCEPTED so this guard cannot quietly become + -- over-broad, which is how a bounds check usually breaks. + -- + -- The explicit NaN test is redundant with `> 1.0` today, because PostgreSQL + -- float8 ordering is not IEEE ordering: it sorts NaN above every other value. + -- It is written out anyway so the intent survives an edit to the bounds. + IF compact_due_fraction IS NULL + OR compact_due_fraction = 'NaN'::float8 + OR compact_due_fraction < 0.0 + OR compact_due_fraction > 1.0 THEN + RAISE EXCEPTION 'compact_due_fraction must be a number between 0 and 1' + USING ERRCODE = 'invalid_parameter_value'; + END IF; + IF recluster_due_fraction IS NULL + OR recluster_due_fraction = 'NaN'::float8 + OR recluster_due_fraction < 0.0 + OR recluster_due_fraction > 1.0 THEN + RAISE EXCEPTION 'recluster_due_fraction must be a number between 0 and 1' + USING ERRCODE = 'invalid_parameter_value'; + END IF; + -- stats() enforces require_caller_select(rel) before it returns a row, so a -- caller without SELECT on rel is refused here rather than reported to. SELECT COALESCE(sum(s.rowcount), 0), COALESCE(sum(s.deletedrows), 0) @@ -1829,4 +1866,4 @@ END; $maintenance_due$; COMMENT ON FUNCTION pgcolumnar.maintenance_due(regclass, float8, float8) - IS 'report whether an online maintenance verb (compact_rewrite, recluster) is worth running, from table statistics alone; thresholds are parameters with defaults measured on #415; pure report, takes no lock and rewrites nothing (#415)'; + IS 'report whether an online maintenance verb (compact_rewrite, recluster) is worth running, from table statistics alone; thresholds are parameters with defaults measured on #415, each required to be a number between 0 and 1 inclusive (#860); pure report, takes no lock and rewrites nothing (#415)'; diff --git a/test/native_reclaim.sh b/test/native_reclaim.sh index f29bc6e9..24b80531 100755 --- a/test/native_reclaim.sh +++ b/test/native_reclaim.sh @@ -11,6 +11,11 @@ # consumed, and that the reused blocks hold correct data (parity with a heap # mirror). # +# It also proves the compaction threshold guards, in BOTH directions, for both +# entry points: pgcolumnar.compact_rewrite() and pgcolumnar.maintenance_due(), +# which is the gate the autovacuum daemon consults before it ever calls +# compact_rewrite (see the block near the end). +# # Usage: test/native_reclaim.sh [PG_CONFIG] # Written fresh for pgColumnar. @@ -18,6 +23,30 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" +# Print the 5-char SQLSTATE of a statement, OK when it succeeded, HANG when it +# did not finish inside the cap, or ERR_NO_SQLSTATE_rcN when psql failed without +# reporting one (a dead server, a bad connection) -- which must never read as OK. +# +# Every deny arm below asserts a SQLSTATE rather than "it failed". "It failed" is +# satisfied by a typo, a missing function, a wrong-arity call and 1/0 alike; the +# controls below assert exactly that discrimination. Modelled on +# test/arrow_import.sh's sqlstate_or_hang. +sqlstate() { + local out rc st + out="$(timeout -s KILL 120 env PATH="$PGC_BINDIR:$PATH" psql \ + -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -qtA 2>&1 </dev/null 2>&1; then - nan_result="accepted" -else - nan_result="rejected" -fi -check "compact_rewrite rejects a NaN threshold" "$nan_result" "rejected" +# ---- compact_rewrite's threshold guard, both directions (#860) -------------- +# +# NaN passes both ordinary C range comparisons (`< 0.0` and `> 1.0` are each +# false for NaN). Refuse it explicitly, or the candidate predicate is false for +# every group and compaction silently does no work despite an accepted threshold. +# 22023 is ERRCODE_INVALID_PARAMETER_VALUE, the code the guard raises. + +# Controls. These four calls are exactly what an "it errored" assertion cannot +# tell apart from the guard firing, so each one is pinned to its own SQLSTATE. +# If any of them ever returns 22023, the deny arms below prove nothing. +check "control: a missing function is 42883, not the guard's 22023" \ + "$(sqlstate "SELECT pgcolumnar.no_such_function(1)")" "42883" +check "control: a wrong-arity compact_rewrite is 42883, not 22023" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', 0.5, 1, 'x')")" "42883" +check "control: a null table name is 22004, not 22023" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite(NULL, 0.5)")" "22004" +check "control: division by zero is 22012, not 22023" \ + "$(sqlstate "SELECT 1/0")" "22012" + +check "compact_rewrite rejects a NaN threshold with 22023" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', 'NaN'::float8)")" "22023" +check "compact_rewrite rejects a threshold above 1 with 22023" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', 2.0)")" "22023" +check "compact_rewrite rejects a negative threshold with 22023" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', -1.0)")" "22023" + +# The other direction, which is how a bounds fix usually breaks. Nothing that +# was here could see the guard becoming OVER-broad: the only call at 0.0 was a +# bare `psql_run "SELECT pgcolumnar.compact_rewrite('n', 0.0);"` whose exit +# status nothing read, so rejecting the legal 0.0 reached nothing but the server +# log ("ERROR: min_deleted_fraction must be a number between 0 and 1"). +# Measured: with `minFrac < 0.0` changed to `minFrac <= 0.0`, this suite fails +# exactly one arm -- the 0.0 arm below -- and the other 32 pass. 0.0 and 1.0 are +# legal and sit exactly ON the boundary, so these two arms redden if either half +# of the comparison goes strict. +check "compact_rewrite ACCEPTS the boundary threshold 0.0" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', 0.0)")" "OK" +check "compact_rewrite ACCEPTS the boundary threshold 1.0" \ + "$(sqlstate "SELECT pgcolumnar.compact_rewrite('n', 1.0)")" "OK" -psql_run "SELECT pgcolumnar.compact_rewrite('n', 0.0);" psql_run "SELECT pgcolumnar.recluster('n', 'id');" check "data correct after delete + compact_rewrite + recluster" \ "$(pgc_set_hash 'SELECT id, v, payload FROM n ORDER BY id')" \ "$(pgc_set_hash 'SELECT id, v, payload FROM h ORDER BY id')" check "row count correct after cycle" "$(q 'SELECT count(*) FROM n;')" "$(q 'SELECT count(*) FROM h;')" +# ---- maintenance_due(): the gate consulted BEFORE compact_rewrite (#860) ---- +# +# pgcolumnar.maintenance_due() is what the autovacuum daemon reads to decide +# whether to call compact_rewrite at all, and it validated nothing. Measured on +# exactly this fixture (20000 rows, 10000 deleted) with the guard removed, beside +# what compact_rewrite answered for the same value: +# threshold compact_rewrite_due recommendation compact_rewrite() +# 0.2 t compact_rewrite OK +# NaN f (none) 22023 +# 2.0 f (none) 22023 +# -1.0 t compact_rewrite 22023 +# NULL (null) (none) OK (NULL means 0.2) +# NaN and 2.0 suppress the work for good. -1.0 is the worst of them: it is not a +# suppressed report but a permanent, self-renewing rewrite of every columnar +# table, because `fraction >= -1` is true whatever the table's real state. +# NULL is the NaN case again, because the daemon reads a NULL verdict as +# "not due" (SPI_getbinval isnull). +# +# Both thresholds are validated because both are thresholds. +psql_run "CREATE TABLE nd (id int, v int, payload text) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.set_options('nd', stripe_row_limit => 1000, chunk_group_row_limit => 1000);" +psql_run "INSERT INTO nd SELECT g, g, md5(g::text) FROM generate_series(1, 20000) g;" +psql_run "DELETE FROM nd WHERE id % 2 = 0;" + +# Premise, GATED not printed: every arm below reads a verdict about a table that +# is 50% deleted. If it is not that table, no verdict from it means anything, so +# refuse to report one. +md_total="$(q "SELECT total_rows FROM pgcolumnar.maintenance_due('nd', 0.2, 0.05)")" +md_del="$(q "SELECT deleted_rows FROM pgcolumnar.maintenance_due('nd', 0.2, 0.05)")" +check_num "premise: nd holds 20000 rows" "$md_total" "20000" +check_num "premise: 10000 of nd's rows are deleted" "$md_del" "10000" +if [ "$md_total" != "20000" ] || [ "$md_del" != "10000" ]; then + echo "FATAL: nd is not the 50%-deleted fixture these arms describe" \ + "(total_rows=[$md_total] deleted_rows=[$md_del]); refusing to report a verdict from it" + pgc_summary + exit 1 +fi + +# A legal threshold still answers, and answers correctly: 0.5 deleted >= 0.2. +check "maintenance_due(0.2) on a 50%-deleted table is still due" \ + "$(q "SELECT compact_rewrite_due FROM pgcolumnar.maintenance_due('nd', 0.2, 0.05)")" "t" +check "maintenance_due(0.6) on a 50%-deleted table is not due" \ + "$(q "SELECT compact_rewrite_due FROM pgcolumnar.maintenance_due('nd', 0.6, 0.05)")" "f" + +check "maintenance_due rejects a NaN compact threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 'NaN'::float8, 0.05)")" "22023" +check "maintenance_due rejects a compact threshold above 1 with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 2.0, 0.05)")" "22023" +check "maintenance_due rejects a negative compact threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', -1.0, 0.05)")" "22023" +check "maintenance_due rejects a NULL compact threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', NULL::float8, 0.05)")" "22023" + +check "maintenance_due rejects a NaN recluster threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 0.2, 'NaN'::float8)")" "22023" +check "maintenance_due rejects a recluster threshold above 1 with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 0.2, 2.0)")" "22023" +check "maintenance_due rejects a negative recluster threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 0.2, -1.0)")" "22023" +check "maintenance_due rejects a NULL recluster threshold with 22023" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 0.2, NULL::float8)")" "22023" + +# The new guard must not be over-broad either. 0.0 and 1.0 are legal thresholds +# on both parameters and sit exactly on the boundary. +check "maintenance_due ACCEPTS the boundary thresholds 0.0, 0.0" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 0.0, 0.0)")" "OK" +check "maintenance_due ACCEPTS the boundary thresholds 1.0, 1.0" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd', 1.0, 1.0)")" "OK" +# ... and the defaults, which the daemon uses, still resolve. +check "maintenance_due ACCEPTS its own defaults" \ + "$(sqlstate "SELECT * FROM pgcolumnar.maintenance_due('nd')")" "OK" + pgc_summary