feat: pgcolumnar.maintenance_due() reports when an online maintenance verb is worth running (#415)

[ChronicallyJD]

Implements the policy report agreed on #415, grounded in the measurement I
posted there. pgcolumnar.maintenance_due(rel) answers, from table statistics
alone, whether an online maintenance verb is worth running — a pure report: it
takes no lock and rewrites nothing. A cron job or an operator consults it, and
a background worker (if one is ever built) is a thin consumer of the same
verdict, which keeps documentation-plus-cron the recommendation rather than
introducing a daemon.

What it reports

pgcolumnar.maintenance_due(rel regclass,
                           compact_due_fraction  float8 DEFAULT 0.2,
                           recluster_due_fraction float8 DEFAULT 0.05)
  -> total_rows, deleted_rows, deleted_fraction,
     sort_key, appended_groups, appended_rows, appended_fraction,
     compact_rewrite_due, recluster_due, recommendation

• compact_rewrite_due when the deleted fraction (sum(deletedrows) / sum(rowcount) over stats()) reaches compact_due_fraction.
• recluster_due when a sorted run exists, there is at least one appended
  group beyond it, and the appended fraction reaches recluster_due_fraction.
• recommendation is the verbs joined, or NULL when nothing is due.

Why the defaults are these numbers

They are the values measured on #415, not invented:

• 0.2 deleted is the knee of the overhead curve — a table's scans run ~10%
  slower than its compacted equivalent at 20% deleted, rising to +150% at 80%.
• 0.05 appended — clustering decay is an order of magnitude more damaging
  per unit than deletes (the smallest decay measured, ~5% appended rows, already
  cost +161% on a pruned range query, because groups read = 1 + appended groups exactly). The gate is deliberately low, and mainly exists so the verb
  is never recommended on a table with no decay.

Thresholds are parameters, not GUCs: the pgcolumnar GUC prefix is reserved
(MarkGUCPrefixReserved), so an unregistered pgcolumnar.* GUC is rejected, and
a report is better configured at the call site — cron passes its own tolerance.

The correctness point, and it is proven by removal

sort_status() reports a never-ordered table as entirely appended, because
it has no sorted run. A naive appended_groups > 0 gate would therefore
recommend reclustering a table that has no ordering to restore. recluster_due
gates on the sorted run existing (sorted_groups > 0).

test/maintenance_due.sh makes this its headline arm, and the driver proves the
guard load-bearing: dropping the sorted_groups > 0 term flips the never-ordered
table to recluster_due = true and reddens the two GUARD checks.

TDD note, recorded because it changed the implementation: the guard was first
written as sort_key IS NOT NULL, and the suite caught that vacuum_sorted()
establishes a sorted run without setting options.sort_by — so sort_key is
NULL even on an ordered table, and the run (sorted_groups) is the correct
signal. sort_key is reported for information only.

Proofs and gate

• RED first: on main (no function) the suite fails every maintenance_due
  arm while its premises pass — recorded before the function existed.
• GREEN: 26/26 on pg18a and pg19a.
• Removal proof: the never-ordered guard, as above.
• Privilege is inherited from stats() (require_caller_select): a role without
  SELECT on the relation is refused, asserted with a direct role connection.
• Preflight (build + suite) on pg15a/16a/17a and the full assert matrix on
  pg18a + pg19a: results below.

Scope

Pure SQL, no C, no new GUC, no lock, no write path. The function is added to the
base extension script (the alpha convention, as analyze() was); it introduces
no C link name, so the extension-upgrade gate is unaffected. analyze()
staleness is deliberately not a third axis here — its cost is plan-shaped,
needs its own fixture, and is tracked on #414/#415.

Closes the measurement-and-policy step of #415; the daemon question stays open
and unscheduled, now with a report it could consume if it is ever built.

---

Gate, commit 9b5dbe1:

• Preflight (build + suite): pg15a, pg16a, pg17a — PASSED.
• Full assert matrix: ALL VERSIONS PASSED — PG18 (154 ran, 2 version-appropriate skips), PG19 (156 ran, 0 skipped), maintenance_due green on both, no regressions.

[jdatcmd]

One blocking defect, found by running the caller the function exists for: every non-superuser is refused

Verified on my PG17 non-assert lane first: 26/26 green, all five fixtures behave as described, the thresholds respond as live parameters, and the never-ordered guard logic reads correctly. The policy content (measured defaults, parameters-not-GUCs, report-not-daemon) is right and I have no notes on it. But:

The defect

A role holding SELECT on the relation — the cron/monitoring caller this report is for — gets 42501:

CREATE ROLE md_reader LOGIN;
GRANT USAGE ON SCHEMA pgcolumnar TO md_reader;
GRANT SELECT ON mdt TO md_reader;
-- as md_reader:
SELECT * FROM pgcolumnar.maintenance_due('mdt');
ERROR:  permission denied for table row_group
CONTEXT:  SQL function "sort_status" statement 1

Mechanism: stats() passes (SECURITY DEFINER, and require_caller_select checks GetOuterUserId(), so the gate correctly evaluates the real caller), but sort_status() is SECURITY INVOKER and reads pgcolumnar.row_group directly, which ordinary roles cannot SELECT. So the function works for superusers and false-denies everyone else.

Why the suite passed anyway, and this is the part worth keeping

The privilege arm only tests the deny case, and every positive arm runs as postgres. With this defect, md_none and md_reader produce the same refusal — meaning the deny check passes in a world where the function denies everyone. A deny arm without its positive control is too loose to be believed (the assert-work-done mirror). The fix needs both arms:

1. Positive control: md_reader (SELECT on rel, nothing on the internal catalogs) gets a row. This is the arm that reddens today.
2. Deny arm on SQLSTATE, not text: the current grep -c "permission denied" is the exact trap the working rules name — it is also satisfied by a login FATAL or an unrelated catalog denial (as it just was, by this defect). \set VERBOSITY sqlstate + assert 42501; the sqlstate_of idiom is in-tree in batch_fold_explain.sh and the objstore suites.

Suggested minimal fix

Make maintenance_due SECURITY DEFINER with SET search_path = pg_catalog, pg_temp, exactly as stats() is. The gate still evaluates the real caller (require_caller_select uses GetOuterUserId(), which is why stats() itself is safe as DEFINER), and sort_status then runs as the owner and can read the internal catalog. Body references are already schema-qualified.

Non-blocking note for a follow-up issue rather than this PR: sort_status() has the same invoker-only property standalone, so it is superuser-practical today; if it is meant as a user-facing report it wants the same DEFINER + require_caller_select treatment separately.

Re-review on sight once the positive arm exists and reddens on the current shape; I will re-run the never-ordered removal proof on the updated branch at the same time.

[ChronicallyJD]

Fixed in 3ccb72d, and you were right that the vacuous deny arm is the real lesson.

The defect reproduces exactly as you found it, and — the part that stings — my
own suite would have caught it if the deny arm had a positive control. It did
not, so a function that denied everyone passed. That is the assert-work-done
mirror landing on me.

The fix

maintenance_due is now SECURITY DEFINER with SET search_path = pg_catalog, pg_temp, exactly as stats(). 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 — they only let sort_status() reach the internal
catalog as the owner. Body references were already schema-qualified.

The suite now has both arms, and the positive one reddens on the old shape

Proven in that order before committing:

-- on the current (invoker) function, new positive arm added:
FAIL  positive: a SELECT-holder gets a verdict, not a false deny: got [] want [f]
PASS  deny: a role without SELECT on the relation is refused (42501)

The positive control (a role with SELECT on the table and nothing on the
internal catalogs) reds on invoker rights — the arm that would have caught this
— and the deny arm now asserts SQLSTATE 42501 via \set VERBOSITY sqlstate,
not the permission denied grep that a login FATAL or an unrelated catalog
denial also satisfies. With the fix both pass and they now differ (f vs
42501), so the deny arm is no longer vacuous.

Gate on 3ccb72d: 27/27 pg18a + pg19a, the never-ordered removal proof still
reddens the GUARD arms when the sorted_groups > 0 term is dropped, preflight
15/16/17 and the full matrix — results below.

Your non-blocking note

Confirmed and filed as its own issue (#608) with a reproduction rather than
folded in here: sort_status() standalone has the same invoker-only property,
so a table's own owner is 42501 on it today. It is the same fix (DEFINER +
require_caller_select) but a separate, pre-existing surface, and it deserves
its own removal proof rather than riding this PR.

Gate, 3ccb72d: preflight (build + suite) pg15a/16a/17a PASSED; full assert matrix ALL VERSIONS PASSED — PG18 (154 ran, 2 skipped), PG19 (156 ran, 0 skipped), maintenance_due green on both, no regressions.

[jdatcmd]

Approved. Re-verified on my lane, both removal proofs re-run by the reviewer.

Against 3ccb72d, PG17 non-assert (the lane your gate did not run):

• 27/27 green, including the two new arms.
• Removal proof of the fix itself, re-run: reverting SECURITY DEFINER back to invoker rights reds exactly the new positive arm (positive: a SELECT-holder gets a verdict, not a false deny: got [] want [f]) while the deny arm stays green at 42501 — the pair discriminates now, which is precisely what the original review asked the suite to be able to do. The arm that would have caught the defect exists and I watched it catch it.
• The never-ordered guard proof, re-run on the updated branch: dropping the sorted_groups > 0 term reds both GUARD arms (got [t] want [f], got [f] want [t]).
• The reproduction from my review (SELECT-holder role, nothing on the internal catalogs) now returns a verdict instead of permission denied for table row_group.

The definer/GetOuterUserId reasoning is correct and matches stats()'s own shape: definer rights reach the internal catalog, the ACL still evaluates the real caller, so no report is available to anyone who could not already read the table. Thanks for filing #608 for sort_status() standalone rather than folding it in.

Merge when ready.