test: a savepoint rolled back, or a statement failed, after a flush (#1341)

test/analyze_function.sh

@@ -979,5 +979,23 @@ check_text "the two routes agree which storage a WITH DATA matview owns" \
 check_text "pgcolumnar.analyze() reaches a matview created WITH DATA" \
 	"$mv_state" "succeeded"
 
+# ---- a named column that does not exist is refused (#1341) --------------------
+#
+# Silently collecting nothing for it is the failure that looks like success, and
+# the refusal comes before any work: a real column named beside it gets no
+# statistics either. A fresh table, so pg_stats starts empty for it.
+q "DROP TABLE IF EXISTS af_unk;
+   CREATE TABLE af_unk (a int, b text) USING pgcolumnar;
+   INSERT INTO af_unk SELECT g, 'v'||g FROM generate_series(1, 5000) g;" >/dev/null
+unk_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -X -At \
+	-v ON_ERROR_STOP=1 -v VERBOSITY=verbose -c "SELECT pgcolumnar.analyze('af_unk'::regclass, ARRAY['a', 'nosuch'])" 2>&1)"
+check_text "analyze() refuses a named column the table does not have, and names it" \
+	"$(sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\): \(.*\)$/\1 \2/p' <<<"$unk_out" | head -1)" \
+	"P0001 pgcolumnar.analyze(): column \"nosuch\" does not exist in af_unk"
+check_num "and writes no statistics, not even for the real column named beside it" \
+	"$(q "SELECT count(*) FROM pg_stats WHERE tablename = 'af_unk'")" "0"
+q "SELECT pgcolumnar.analyze('af_unk'::regclass, ARRAY['a'])" >/dev/null
+check_num "control: the same call naming only the real column writes its statistics" \
+	"$(q "SELECT count(*) FROM pg_stats WHERE tablename = 'af_unk' AND attname = 'a'")" "1"
 
 pgc_summary

test/check_ledger_budget.txt

@@ -15,7 +15,7 @@
 #
 #   Adding a check to a suite that is already covered does not move it, which is
 #   what makes it safe to bound.
-suites_not_covered 227
+suites_not_covered 224
 #
 # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become
 #   one.
@@ -2632,4 +2632,113 @@ suites_not_covered 227
 #     never         2374 - 3 = 2371
 #     dated          802 + 3 = 805
 #     2371 + 805 == 3176
-checks_never_observed_red 2371
+#
+# #1341, projections seeded, with the sort-type arms (OffgridwithJD): 2371 -> 2446, and
+# suites_not_covered 227 -> 226. The suite had no rows. It enters with all 76 checks
+# from five-major runs (76/76 on each of 15-19). One is dated by the projection
+# writer without its sort-type ordering check: a json sort key's back-fill then
+# crashed the backend instead of refusing with 0A000. The other 75 arrive never.
+#
+#     ledger rows   3176 + 76 = 3252
+#     never         2371 + 75 = 2446
+#     dated          805 +  1 =  806
+#     2446 + 806 == 3252
+#
+# #1341, analyze_function seeded, with the unknown-column arms (OffgridwithJD): 2446 ->
+# 2516, and suites_not_covered 226 -> 225. The suite had no rows. It enters with all 72
+# checks from five-major runs (71/71 on 18-19, and the one named version SKIP on
+# 15-17, where pgcolumnar.analyze() needs PostgreSQL 18). Two are dated, on PG18, by
+# analyze() without its unknown-column check: the call then succeeded and wrote
+# statistics for the real column named beside the missing one. The other 70 arrive
+# never.
+#
+#     ledger rows   3252 + 72 = 3324
+#     never         2446 + 70 = 2516
+#     dated          806 +  2 =  808
+#     2516 + 808 == 3324
+#
+# #1341, encode_effort seeded, with the compression-name arms (OffgridwithJD): 2516 ->
+# 2526, and suites_not_covered 225 -> 224. The suite had no rows. It enters with all
+# 12 checks from five-major runs (12/12 on each of 15-19). Two are dated by
+# set_options without its compression-name check: "brotli" was stored over the
+# zstd set before it. The other 10 arrive never.
+#
+#     ledger rows   3324 + 12 = 3336
+#     never         2516 + 10 = 2526
+#     dated          808 +  2 =  810
+#     2526 + 810 == 3336
+#
+# #1386, every relation-taking entry point names the suite that asserts its bar
+# (OffgridwithJD): 2526 -> 2532. 11 new entry_point_privilege checks: the three map arms, and
+# an owner call and a 42501 arm for each of cluster_hilbert, recluster_hilbert, truncate
+# and require_caller_select, which no arm called as a refused caller. Dated: the map arm
+# by a relation-taking function with no check (it left the suite 31/31 before), and
+# each new 42501 arm by its check removed (truncate's owner check, require_caller_select
+# checking nothing, and cluster_verb_validate's owner check for both Hilbert verbs).
+# Five-major runs: 42/42 on 15-19.
+#
+#     ledger rows   3336 + 11 = 3347
+#     never         2526 + 6 = 2532
+#     dated          810 + 5 =  815
+#     2532 + 815 == 3347
+#
+# #1339, six rollup premises that read a product operation's result (OffgridwithJD, from
+# jdatcmd's #1372 review): 2532 -> 2527. They lose the prefix, renamed with their rows.
+# Five are dated: vacuum rewriting nothing (the renumbering check), compact_rewrite and
+# expire reporting 0 (their checks), and expire retiring nothing (the bucket-D check,
+# with the two expire counts and three refusal arms beside it). "the rollup holds
+# partials the delete below cannot reach" arrives never: only a refresh that writes no
+# partials reddens it, and that reddens most of the suite. Five-major runs: 82/82, and
+# 105/105 on PG18 after the rebase onto d5c0a5e, which brought in 23 checks.
+#
+#     ledger rows   3347     = 3347
+#     never         2532 - 5 = 2527
+#     dated          815 + 5 =  820
+#     2527 + 820 == 3347
+#
+# #1341, a rollup's min and max keep each source column's collation (OffgridwithJD):
+# 2527 -> 2529. Three new rollup checks. The arm is dated by the fold comparing in
+# the partials' default collation (on an en_US database the C column's min became
+# a1 where the recompute gives B1). Its two premises arrive never. Five-major runs
+# on the files merged with #1359: rollup 85/85 on 15-19.
+#
+#     ledger rows   3347 + 3 = 3350
+#     never         2527 + 2 = 2529
+#     dated          820 + 1 = 821
+#     2529 + 821 == 3350
+#
+# #1372 review (OffgridwithJD): 2529 -> 2528. "the second refresh folds new rows into the
+# existing bucket" loses the premise prefix: refresh_rollup's return value is product
+# output. refresh_rollup returning one more than it folded reddens it, dating it, and
+# fifteen other rollup arms that count what a refresh folded, all already dated.
+# Five-major runs: rollup 85/85 on 15-19, and 108/108 on PG18 after the rebase onto
+# d5c0a5e, which brought in 23 checks (#1391's row_tid arm still runs last).
+#
+#     ledger rows   3350     = 3350
+#     never         2529 - 1 = 2528
+#     dated          821 + 1 = 822
+#     2528 + 822 == 3350
+#
+# #1341, a savepoint rolled back, or a statement failed, after a flush
+# (jdatcmd): 2528 -> 2544. savepoint_flush enters with all 30 checks from
+# five-major runs (30/30 on each of 15-19). Every shape starts from fresh tables,
+# so each date below was observed with that shape alone (OffgridwithJD, review of
+# #1430). 14 are dated on PG17 by three mutants:
+#   - the row-group list read under SnapshotAny: every arm that reads the table
+#     after an aborted savepoint, and the failing UPDATE's error arm, whose
+#     transaction then fails on the rows that came back;
+#   - every delete_vector read under SnapshotAny: the rolled-back UPDATE and the
+#     failing UPDATE, whose marks are now flushed before it fails;
+#   - the abort promoting its buffers to the parent instead of discarding them:
+#     the failing INSERT and failing UPDATE arms.
+# The two "abort keeps its buffer" mutants of the first version are not used: on
+# an assert-enabled build, after #1403, both trip a flushedUnder assertion at commit
+# and crash the server, so their reds are the crash. On a build without asserts they
+# run and redden 6 and 3 arms (OffgridwithJD, review of #1430). The 16 never are the premises, the
+# "it ran" arms and the reads inside a savepoint.
+#
+#     ledger rows   3350 + 30 = 3380
+#     never         2528 + 16 = 2544
+#     dated          822 + 14 = 836
+#     2544 + 836 == 3380
+checks_never_observed_red 2544

test/encode_effort.sh

@@ -75,6 +75,18 @@ check "a valid encode_effort is stored" \
 	"$(q "SELECT encode_effort FROM pgcolumnar.options WHERE regclass = 'ef_a'::regclass;" | tail -1)" \
 	"fast"
 
+# The compression option's own guard (#1341), its sibling. Without it an unknown
+# name is stored, and the C reader maps a name it does not know to -1 and falls
+# back to the instance default: the table silently compresses as something else.
+psql_run "SELECT pgcolumnar.set_options('ef_a', compression => 'zstd');" >/dev/null
+badc="$(psql_run "SELECT pgcolumnar.set_options('ef_a', compression => 'brotli');" 2>&1 || true)"
+check "an unknown compression name is refused" \
+	"$(case "$badc" in *'unknown columnar compression "brotli"'*) echo yes ;; *) echo "no ($badc)" ;; esac)" \
+	"yes"
+check "and the compression set before it is still the one stored" \
+	"$(q "SELECT compression FROM pgcolumnar.options WHERE regclass = 'ef_a'::regclass;" | tail -1)" \
+	"zstd"
+
 # --- 2. it changes what the writer does ---------------------------------------
 
 psql_run "DROP TABLE IF EXISTS ef_full; DROP TABLE IF EXISTS ef_fast;

test/entry_point_privilege.sh

@@ -136,7 +136,10 @@ CATALOG_GOVERNED="$(echo $CATALOG_GOVERNED)"
 
 # Relation-taking entry points that are supposed to REFUSE an unprivileged
 # caller, asserted behaviourally rather than believed.
-GUARDED_BEHAVIOURAL="read_projection reconstruct_via_projection vm_is_visible vm_selftest get_storage_id"
+# #1386 added the four after get_storage_id: each had no arm anywhere that calls it as
+# a caller its bar should stop (cluster_hilbert and recluster_hilbert share cluster's
+# owner check in code, which is a reason to expect a refusal, not an assertion of one).
+GUARDED_BEHAVIOURAL="read_projection reconstruct_via_projection vm_is_visible vm_selftest get_storage_id cluster_hilbert recluster_hilbert truncate require_caller_select"
 GUARDED_BEHAVIOURAL="$(echo $GUARDED_BEHAVIOURAL)"
 
 # ---- the enumeration, from the catalog -------------------------------------
@@ -277,6 +280,66 @@ check_num "the catalog-governed class is the six from #560" "$catalog_n" "6"
 #    engineer who closes one must come here and remove the name.
 check_num "no relation-taking C entry point is still ungated" "$pinned_n" "0"
 
+# ---- every claimed-guarded entry point names where its bar is asserted (#1386) ----
+#
+# "Claimed-guarded" was the else branch: every relation-taking name that is neither
+# catalog-governed nor pinned. A new relation-taking function with no check at all
+# landed there and nothing reddened (measured: zz_mutant_leak(regclass) left this
+# suite 31/31). So the claim is now written down, name by name, with the suite whose
+# arms call it as a caller the bar should stop. A name with no entry is red, and so
+# is an entry whose suite does not exist or never calls the function, or that names
+# a function this population no longer has.
+BAR_ASSERTED_IN="
+add_projection:vacuum_lock_privilege
+drop_projection:vacuum_lock_privilege
+vacuum:vacuum_lock_privilege
+vacuum_sorted:vacuum_lock_privilege
+cluster:vacuum_lock_privilege
+recluster:vacuum_lock_privilege
+compact:vacuum_lock_privilege
+compact_rewrite:vacuum_lock_privilege
+export_arrow:import_export_privilege
+export_parquet:import_export_privilege
+import_arrow:import_export_privilege
+import_parquet:import_export_privilege
+parallel_copy:import_export_privilege
+parallel_export_parquet:import_export_privilege
+expire:ttl_expire
+column_extent:column_extent
+maintenance_due:maintenance_due
+refresh_rollup:rollup
+rebase_rollup:rollup
+rollup_source_groups:rollup
+rollup_source_retired:rollup
+tier_partition:tier_partition
+partition_create_ahead:partition_lifecycle
+"
+# Relation-taking, with no bar to assert, each for its reason:
+#   partition_drop_candidates, partition_drop_plan  return what pg_inherits and the
+#     partition bounds already tell any role, and the plan is text: the statements
+#     it lists run as whoever runs them, under core's own privilege checks.
+NO_BAR_NEEDED="partition_drop_candidates partition_drop_plan"
+bar_home() { awk -v f="$1" -F: '$1 == f { print $2 }' <<<"$BAR_ASSERTED_IN"; }
+unhomed="" badhome="" stalehome=""
+for fn in $relfns; do
+	case " $CATALOG_GOVERNED $KNOWN_UNGUARDED $NO_BAR_NEEDED " in *" $fn "*) continue ;; esac
+	# The behavioural arms below are this suite's own home for the names they call.
+	case " $GUARDED_BEHAVIOURAL " in *" $fn "*) continue ;; esac
+	home="$(bar_home "$fn")"
+	if [ -z "$home" ]; then unhomed="$unhomed $fn"; continue; fi
+	f="$(dirname "${BASH_SOURCE[0]}")/$home.sh"
+	[ -r "$f" ] && grep -q "pgcolumnar\.$fn(" "$f" || badhome="$badhome $fn:$home"
+done
+for fn in $(awk -F: 'NF == 2 { print $1 }' <<<"$BAR_ASSERTED_IN") $NO_BAR_NEEDED; do
+	case " $relfns " in *" $fn "*) ;; *) stalehome="$stalehome $fn" ;; esac
+done
+check "every claimed-guarded entry point names the suite that asserts its bar (#1386)" \
+	"$([ -z "${unhomed// /}" ] && echo none || echo "$unhomed")" "none"
+check "and each named suite exists and calls that entry point" \
+	"$([ -z "${badhome// /}" ] && echo none || echo "$badhome")" "none"
+check "and no entry names a function that is not a relation-taking entry point" \
+	"$([ -z "${stalehome// /}" ] && echo none || echo "$stalehome")" "none"
+
 # THE COUNT ABOVE IS NOT ENOUGH, AND THIS SUITE PROVED IT ON ITSELF.
 #
 # When #558 and #562 merged, four of the nine names here became genuinely
@@ -353,7 +416,8 @@ call_for() {  # call_for <fn> -> a valid argument vector for that function
 	case "$1" in
 		read_projection|reconstruct_via_projection) echo "SELECT pgcolumnar.$1('ep_target','p1');" ;;
 		vm_is_visible|vm_selftest)                  echo "SELECT pgcolumnar.$1('ep_target',0);" ;;
-		get_storage_id)                             echo "SELECT pgcolumnar.$1('ep_target');" ;;
+		get_storage_id|truncate|require_caller_select) echo "SELECT pgcolumnar.$1('ep_target');" ;;
+		cluster_hilbert|recluster_hilbert)          echo "SELECT pgcolumnar.$1('ep_target', 'id');" ;;
 		export_arrow|export_parquet|import_arrow|import_parquet)
 			echo "SELECT pgcolumnar.$1('ep_target','/nonexistent/dir/x');" ;;
 		*) echo "" ;;
@@ -386,7 +450,8 @@ done
 check "premise: t_ep now holds EXECUTE, so a refusal below can only be the C check" \
 	"$(q "SELECT bool_and(has_function_privilege('t_ep', p.oid, 'EXECUTE'))
 	      FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace
-	      WHERE n.nspname='pgcolumnar' AND p.proname IN ('read_projection','reconstruct_via_projection','vm_is_visible','vm_selftest');")" \
+	      WHERE n.nspname='pgcolumnar' AND p.proname IN ('read_projection','reconstruct_via_projection','vm_is_visible','vm_selftest',
+	                                   'cluster_hilbert','recluster_hilbert','truncate','require_caller_select');")" \
 	"t"
 
 for fn in $GUARDED_BEHAVIOURAL; do

test/projections.sh

@@ -448,4 +448,21 @@ psql_run "INSERT INTO dt2 SELECT g, g FROM generate_series(200, 300) g;"
 check "control: a drop in its own transaction never left one" \
 	"$(proj_orphans dt2)" "0"
 
+# ---- a sort key with no ordering is refused (#1341) --------------------------
+#
+# add_projection does not check the sort column's type. The writer does, when it
+# is built: json has no btree comparison, so the back-fill over existing rows
+# must refuse with 0A000 rather than sort by a comparison function that does not
+# exist. Last in the suite, because without the check that sort crashes the
+# backend (measured).
+psql_run "CREATE TABLE pjs (id int, j json) USING pgcolumnar;
+	  INSERT INTO pjs SELECT g, json_build_object('k', g) FROM generate_series(1, 500) g;"
+check "a projection sorted on a type with no ordering (json) is refused (0A000)" \
+	"$(proj_sqlstate "SELECT pgcolumnar.add_projection('pjs', 'pjp', ARRAY['id','j'], ARRAY['j'])")" "0A000"
+# On a server that accepts connections again: without the check the sort crashes
+# its backend (measured), and this arm must answer for the catalog, not for that.
+for _i in $(seq 1 60); do [ "$(q 'SELECT 1' 2>/dev/null)" = 1 ] && break; sleep 0.5; done
+check "and nothing of it is declared" \
+	"$(q "SELECT count(*) FROM pgcolumnar.projection_declaration WHERE name = 'pjp';")" "0"
+
 pgc_summary