test: parallel_copy_dedup gains its pytest twin, and a fingerprint of the file's first read no longer passes (#1341)

test/approx_aggregates.sh

@@ -16,8 +16,10 @@
 #     its default precision (log2m 11) has a standard error of 1.04/sqrt(2048), so
 #     three of them, 6.9%; tdigest at compression 100 within 0.1% at p99, about
 #     four times what phase 0 measured (0.025%).
-#   - each type stored in a columnar table comes back usable: an hll byte for byte,
-#     a tdigest as a digest whose union still answers within the bound.
+#   - each type stored in a columnar table comes back byte for byte, and its union
+#     still answers. For tdigest that union's bound is a premise only because the
+#     byte-for-byte arm comes first: a storage change that altered a digest reddens
+#     the arm before it can move the percentile.
 #   - the aggregate reads through the columnar scan, so the arms exercise our reader.
 #     That is an arm, not a premise: turning the custom scan off by default reddens it
 #     (#1236: a check a repository change can redden is not a premise).
@@ -69,8 +71,16 @@ TD_H="$(qv "$P0 SELECT tdigest_percentile(v, 100, 0.99) FROM ax_heap;")"
 EXACT99="$(q "SELECT percentile_cont(0.99) WITHIN GROUP (ORDER BY v) FROM ax_heap;")"
 check "tdigest over a columnar table gives exactly what it gives over heap" "$TD_C" "$TD_H"
 check "premise: and its p99 is within 0.1% of the exact percentile" "$(within "$TD_C" "$EXACT99" 0.001)" "ok"
-psql_run "CREATE TABLE ax_td (day int, t tdigest) USING pgcolumnar;
-	INSERT INTO ax_td SELECT day, tdigest(v, 100) FROM ax GROUP BY day;"
+# The digests are built ONCE, into a heap table, and that table's values are stored.
+# A tdigest depends on the order its inputs arrive in, so digests rebuilt from heap
+# could differ from these for reasons that are not storage. Copied from one source,
+# any difference is the columnar round trip's (#1339).
+psql_run "CREATE TABLE ax_td_src USING heap AS SELECT day, tdigest(v, 100) AS t FROM ax GROUP BY day;
+	CREATE TABLE ax_td (day int, t tdigest) USING pgcolumnar;
+	INSERT INTO ax_td SELECT day, t FROM ax_td_src;"
+check "a tdigest stored in a columnar table comes back byte for byte" \
+	"$(q "SELECT (SELECT string_agg(t::text, ',' ORDER BY day) FROM ax_td)
+	        = (SELECT string_agg(t::text, ',' ORDER BY day) FROM ax_td_src);")" "t"
 check "premise: a tdigest stored in a columnar table still answers: the union of daily digests is within 0.1% at p99" \
 	"$(within "$(q "SELECT tdigest_percentile(t, 0.99) FROM ax_td;")" "$EXACT99" 0.001)" "ok"
 

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 75
+suites_not_covered 73
 #
 # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become
 #   one.
@@ -5957,4 +5957,66 @@ suites_not_covered 75
 #     never         4129 + 8 = 4137
 #     dated         2791 + 7 = 2798
 #     4137 + 2798 == 6935
-checks_never_observed_red 4137
+#
+# #1339, a batch of projection checks (jdatcmd): 4137 -> 4132, and suites_not_covered
+# 75 -> 74. Seven findings, each shown green on main under its mutation in both
+# twins and red by name after the fix. projection_drop_column is seeded: it had no
+# rows, and it enters with 11 checks from five-major runs (11/11 on each of 15-19,
+# twin green on each). Its stranger arms read only 42501, so deleting the unlocked
+# ownership check passed; a stranger is now asked while the owner holds SHARE, and
+# queues to 55P03 under that mutation. Its "two refusals are the same" check lost its
+# premise: prefix, joined the shell suite, and moved ahead of the two constant arms
+# so it can fail. projection_parallel's two worker-count checks lost their premise:
+# prefix: parallel_workers = 1 reddens both. projection_privilege's read_projection
+# arms each asserted one SQLSTATE twice; the first now asks only "refused", as the
+# shell suite does, and the shell's reconstruct arms read the SQLSTATE, as the twin
+# does. projection_scan_io gains a check that an empty projection is not priced at
+# zero pages. projections gains two: an EXPLAIN ANALYZE reading that the executor
+# opened the projection, and that the old-snapshot count plans it. Its twin's two
+# session-A timeout arms now read whether A answered, not the count again, and the
+# shell's poll no longer takes an ERROR that echoes the token for an answer. 19 rows
+# are dated by ten mutations, each reddening its target by the same name in both
+# twins (the pytest twin stops at a test's first red): the unlocked ownership check
+# deleted, both deleted, parallel_workers = 1, each function's aclcheck_error
+# replaced by elog, the RLS refusal raised as 42501, the one-page floor deleted, the
+# executor never opening the projection, projection pages multiplied by 1000000,
+# and the projection scan and liveness cache both on GetLatestSnapshot(). 8 arrive
+# never: one premise and seven drop_column checks no mutation here reached.
+#
+#     ledger rows   6935 + 14 = 6949
+#     never         4137 + 8 - 13 = 4132
+#     dated         2798 + 6 + 13 = 2817
+#     4132 + 2817 == 6949
+#
+# #1339, approx_aggregates gains one arm (jdatcmd): 4132 -> 4132, unchanged. "a tdigest stored in
+# a columnar table comes back byte for byte" is dated by a reader that returns each long
+# value as the previous long value read: main's suite passed every tdigest check under it,
+# and only the hll arm went red. suites_not_covered does not move.
+#
+#     ledger rows   6949 + 1 = 6950
+#     never         4132 + 0 = 4132
+#     dated         2817 + 1 = 2818
+#     4132 + 2818 == 6950
+#
+# #1341, parallel_copy_dedup seeded, with its twin (jdatcmd): 3853 -> 3857, and
+# suites_not_covered 105 -> 104. The suite had no rows. It enters with 17 checks from
+# five-major runs (17/17 on each of 15-19, shell and twin alike), after three fixes.
+# The changed file differed from the first in its first bytes and fitted in one
+# 256 KiB read, so a fingerprint of the first read alone still loaded it and main's
+# shell passed 14/14; the changed file now differs only in its last row, past the
+# first read, and a new check compares the table's records with the server's
+# sha256() of each file. The skip's NOTICE, promised in the header, was never read:
+# with it removed main's shell passed 14/14; a new check reads it. And the two
+# control checks lost their premise: prefix under #1339: dedup is pgcolumnar's own
+# default, and forcing it on reddens them. 13 are dated by six mutations, each
+# reddening the same names in both twins: the NOTICE never raised, the first read
+# alone fingerprinted, dedup forced on, the refused load committed anyway, a
+# fingerprint matched in any table, and the refused load reporting the rows it
+# discarded. 4 arrive never: the fixture-size premise, the first load's two checks,
+# and the prepared-transaction leak.
+#
+#     ledger rows   6950 + 17 = 6967
+#     never         4132 + 4 = 4136
+#     dated         2818 + 13 = 2831
+#     4136 + 2831 == 6967
+checks_never_observed_red 4136

test/parallel_copy_dedup.sh

@@ -49,42 +49,62 @@ DATADIR="$PGC_WORKDIR/pcdedup"
 mkdir -p "$DATADIR"; chmod 777 "$DATADIR"
 if [ "$(id -u)" = "0" ]; then chown postgres "$DATADIR"; fi
 
-ROWS=20000
+# The coordinator reads the file 256 KiB at a time (PCOPY_FINGERPRINT_BUFSZ). The
+# changed file G differs from F ONLY in its last row, and 25,000 rows put that row
+# past the first read. When G differed from its first bytes and the file fitted in
+# one read, a fingerprint of the first read alone loaded G as a change and this
+# suite passed 14 of 14.
+ROWS=25000
 F="$DATADIR/load.txt"
 G="$DATADIR/other.txt"
 
 psql_run "COPY (SELECT g AS id, 'v'||g AS txt FROM generate_series(1,$ROWS) g)
           TO '$F' WITH (FORMAT text);" >/dev/null
-psql_run "COPY (SELECT g AS id, 'w'||g AS txt FROM generate_series(1,$ROWS) g)
-          TO '$G' WITH (FORMAT text);" >/dev/null
+psql_run "COPY (SELECT g AS id, CASE WHEN g = $ROWS THEN 'changed' ELSE 'v'||g END AS txt
+          FROM generate_series(1,$ROWS) g) TO '$G' WITH (FORMAT text);" >/dev/null
 
 q() {
 	env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \
 		-c "$1" 2>&1 | tail -1
 }
 
+# The SHA-256 of each file's bytes, computed by the server from the file itself.
+sha() { q "SELECT encode(sha256(pg_read_binary_file('$1')), 'hex')"; }
+sha_f="$(sha "$F")"
+sha_g="$(sha "$G")"
+check_num "premise: the changed row lies past the first 256 KiB of the file" \
+	"$(q "SELECT ((SELECT size FROM pg_stat_file('$G'))
+	        - octet_length('$ROWS'||E'\tchanged\n') >= 262144)::int")" 1
+
 mk() { psql_run "DROP TABLE IF EXISTS $1; CREATE TABLE $1 (id int, txt text) USING pgcolumnar;" >/dev/null; }
 
 # ---- the control: without dedup the retry doubles the table ----------------
 mk pcd_plain
 r1="$(q "SELECT pgcolumnar.parallel_copy('pcd_plain','$F',4)")"
 r2="$(q "SELECT pgcolumnar.parallel_copy('pcd_plain','$F',4)")"
-check "premise: without dedup a repeated load doubles the table, which is the defect" \
+# Not premises (#1339): dedup is pgcolumnar's own default, and forcing it on reddens
+# both of these.
+check "without dedup a repeated load doubles the table, which is the defect" \
 	"$(q "SELECT count(*) FROM pcd_plain")" "$(( ROWS * 2 ))"
-check "premise: and each of those loads reported its rows" "$r1/$r2" "$ROWS/$ROWS"
+check "and each of those loads reported its rows" "$r1/$r2" "$ROWS/$ROWS"
 
 # ---- the same load twice, with dedup --------------------------------------
 mk pcd
 d1="$(q "SELECT pgcolumnar.parallel_copy('pcd','$F',4,true)")"
 c1="$(q "SELECT count(*) FROM pcd")"
-d2="$(q "SELECT pgcolumnar.parallel_copy('pcd','$F',4,true)")"
+# All of psql's output, NOTICE included: the header promises the skip says so.
+o2="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \
+	-c "SELECT pgcolumnar.parallel_copy('pcd','$F',4,true)" 2>&1)"
+d2="$(printf '%s\n' "$o2" | tail -1)"
 c2="$(q "SELECT count(*) FROM pcd")"
 echo "-- dedup on: load 1 returned $d1 (table $c1), load 2 returned $d2 (table $c2)"
 
 check "the first load stores its rows normally (#403 item 7)" "$c1" "$ROWS"
 check "and reports them" "$d1" "$ROWS"
 check "the second load of the same file stores nothing (#403 item 7)" "$c2" "$ROWS"
 check "and reports 0 rather than claiming it loaded them" "$d2" "0"
+check_num "and says in a NOTICE that it skipped the load, rather than returning a silent 0" \
+	"$(printf '%s\n' "$o2" | grep -c 'NOTICE:  pgcolumnar.parallel_copy skipped a load already applied to "pcd"')" 1
 
 # ---- a changed file at the same path is a different load -------------------
 cp "$G" "$F"
@@ -112,6 +132,13 @@ check "the skipped load left exactly one record for that table and file" \
 	"$(q "SELECT count(*) FROM pgcolumnar.load_fingerprint
 	      WHERE relation_oid = 'pcd'::regclass")" "2"
 
+# The SHA-256 of the file's bytes, not of some part of them: the two records are
+# exactly the two files' digests, in load order.
+check_text "the records hold the SHA-256 of each loaded file's bytes" \
+	"$(q "SELECT string_agg(encode(fingerprint, 'hex'), ',' ORDER BY loaded_at)
+	      FROM pgcolumnar.load_fingerprint WHERE relation_oid = 'pcd'::regclass")" \
+	"$sha_f,$sha_g"
+
 check "no prepared transaction leaked through any of it" \
 	"$(q "SELECT count(*) FROM pg_prepared_xacts")" "0"
 

test/projection_drop_column.sh

@@ -39,10 +39,39 @@ psql_run "INSERT INTO pdc
 # The pre-statement dependency check must not tell a stranger which column the
 # projection covers, or let that role take the lock retained by the owner path.
 psql_run "CREATE ROLE pdc_nobody LOGIN;"
-check "a non-owner learns nothing from a projected column" \
-	"$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN sort_key;')" "42501"
+proj_state="$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN sort_key;')"
+plain_state="$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN payload;')"
+# The property is that the two refusals are INDISTINGUISHABLE, so it is compared
+# first: with both ownership checks deleted, the stranger gets 2BP01 for the
+# projected column and 42501 for the other, and this is the check that names it.
+check "and the two refusals are the same error, so neither leaks" \
+	"$proj_state" "$plain_state"
+check "a non-owner learns nothing from a projected column" "$proj_state" "42501"
 check "the same non-owner error is returned for an unprojected column" \
-	"$(sqlstate_as pdc_nobody 'ALTER TABLE pdc DROP COLUMN payload;')" "42501"
+	"$plain_state" "42501"
+
+# THE LOCK HALF OF THE COMMENT ABOVE (#1339). The ownership check runs twice:
+# once with no lock, then again after ShareUpdateExclusiveLock. The two checks
+# above cannot tell them apart, because both raise 42501. Contention can: while
+# the owner holds SHARE on pdc, which conflicts with ShareUpdateExclusiveLock, a
+# stranger refused by the unlocked check gets 42501 at once, and one that reaches
+# the lock first waits out its lock_timeout (55P03).
+holder_log="$PGC_WORKDIR/pdc_holder.log"
+env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
+	-Atq -v ON_ERROR_STOP=0 -c "BEGIN;" -c "LOCK TABLE pdc IN SHARE MODE;" \
+	-c "SELECT pg_sleep(3600) /* pdc_holder */;" -c "ROLLBACK;" > "$holder_log" 2>&1 &
+holder_pid=$!
+held=0
+for _ in $(seq 1 40); do
+	held="$(q "SELECT count(*) FROM pg_locks WHERE relation = 'pdc'::regclass AND mode = 'ShareLock' AND granted;")"
+	[ "${held:-0}" -ge 1 ] && break
+	sleep 0.25
+done
+check "premise: the owner holds SHARE on pdc before the stranger asks" "${held:-0}" "1"
+check "a non-owner is refused before it waits for the table lock" \
+	"$(sqlstate_as pdc_nobody "SET lock_timeout = '2s'; ALTER TABLE pdc DROP COLUMN sort_key;")" "42501"
+psql_run "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE query LIKE '%pdc_holder%' AND pid <> pg_backend_pid();" >/dev/null
+wait "$holder_pid" 2>/dev/null || true
 
 # Red before the fix: PostgreSQL accepts this (00000), and the next INSERT
 # reaches the projection writer with a type OID of zero.

test/projection_parallel.sh

@@ -277,10 +277,13 @@ check "premise: the covering plan is parallel at the default page cost" \
 check "a page cost that makes I/O dominate costs the parallel covering path" \
 	"$(shape "$(explain_cov_spc $SPC_HI)")" "projection-only"
 
-check "premise: EXPLAIN ANALYZE launched two workers" \
+# NOT PREMISES (#1339, #1236). The worker count is the projection path's own
+# parallel_workers: with it set to 1, Workers Launched reads 1 and both of these
+# go red. A check a pgcolumnar change can redden is an arm.
+check "EXPLAIN ANALYZE launched two workers" \
 	"$(echo "$par_on_ana" | grep -oE 'Workers Launched: [0-9]+' | head -1 | grep -oE '[0-9]+')" "2"
 
-check "premise: ANALYZE printed a rows= line per launched worker" \
+check "ANALYZE printed a rows= line per launched worker" \
 	"$n_lines" "2"
 
 # THE DEFECT the plan-shape arms cannot see: Gather is present and the count

test/projection_privilege.sh

@@ -191,8 +191,12 @@ check "premise: t_prjexec can open a session, so a refusal below is a refusal" \
 	"$(as t_prjexec 'SELECT 1;' | head -1)" "1"
 check "a role with EXECUTE but no SELECT is refused read_projection" \
 	"$(count_as t_prjexec read_projection)" "refused"
+# The SQLSTATE, not merely "refused", as the pytest twin asserts (#1339): this
+# function has no second arm reading the code, so "refused" alone accepted any
+# error at all, a login FATAL included.
 check "and is refused reconstruct_via_projection, which leaks non-covered columns" \
-	"$(count_as t_prjexec reconstruct_via_projection)" "refused"
+	"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.reconstruct_via_projection('secret','p1');")" \
+	"42501"
 check "and THAT refusal is the C check, which raises 42501 from aclcheck_error" \
 	"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');")" \
 	"42501"
@@ -241,7 +245,8 @@ check_num "the policy is in force for ordinary SQL" \
 check "read_projection now refuses a policy-restricted caller (#563)" \
 	"$(count_as t_prjsel read_projection)" "refused"
 check "and so does reconstruct_via_projection" \
-	"$(count_as t_prjsel reconstruct_via_projection)" "refused"
+	"$(state_as t_prjsel "SELECT count(*) FROM pgcolumnar.reconstruct_via_projection('secret','p1');")" \
+	"0A000"
 # 0A000, not a phrase. PgColumnarRequireNoRowSecurity raises
 # ERRCODE_FEATURE_NOT_SUPPORTED, which is a different SQLSTATE CLASS from either
 # ACL refusal -- 0A against 42 -- so no rewording of any message can confuse them.

test/projection_scan_io.sh

@@ -200,8 +200,21 @@ check "premise: the empty projection is found and its storage holds no row group
 	      GROUP BY p.proj_storage_id;")" \
 	"true/0"
 
+psio_e_plan="$(explain_scan on "SELECT ik FROM psio_e WHERE ik BETWEEN 1 AND 50")"
 check "an empty covering projection is still priced as one page, so it is chosen" \
-	"$(explain_scan on "SELECT ik FROM psio_e WHERE ik BETWEEN 1 AND 50" | grep -c 'Columnar Projection: byik')" \
+	"$(printf '%s\n' "$psio_e_plan" | grep -c 'Columnar Projection: byik')" \
 	"1"
 
+# THE OTHER DIRECTION (#1339). The arm above catches over-pricing only: priced
+# at 0 pages instead of 1, the projection is cheaper still and stays chosen.
+# With every CPU cost set to 0 by explain_scan, the projection scan's whole cost
+# is its page cost, so a total of zero means it was priced at zero pages.
+# Measured on PG17: 5.00 with the clamp, 0.00 with `if (pages < 1) pages = 1;`
+# deleted, and the projection chosen both times.
+psio_e_cost="$(printf '%s\n' "$psio_e_plan" | grep -m1 'Custom Scan' |
+	sed -n 's/.*cost=[0-9.]*\.\.\([0-9.]*\) .*/\1/p')"
+check "and its scan is not priced at zero pages" \
+	"$(awk -v c="${psio_e_cost:-none}" 'BEGIN { if (c ~ /^[0-9]+(\.[0-9]+)?$/) print (c + 0 > 0 ? "priced" : "free at " c); else print "unread: " c }')" \
+	"priced"
+
 pgc_summary