diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index d10894222..d972a21b4 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -5212,11 +5212,22 @@ prepared_writes prepared_writes no backend was terminated by a signal 15;16;17;1 prepared_writes prepared_writes premise: prepared transactions are enabled 15;16;17;18;19 never - prepared_writes prepared_writes premise: the fetch is an index scan, which is what reaches the fetch cache 15;16;17;18;19 never - prepared_writes prepared_writes premise: the server log is where a crash would be recorded 15;16;17;18;19 never - +projection_drop_column projection_drop_column a non-owner is refused before it waits for the table lock 15;16;17;18;19 2026-10-22 columnar_tableam.c: both DROP COLUMN ownership checks deleted, so a stranger gets 2BP01 for a projected column and 42501 for another;columnar_tableam.c: the unlocked DROP COLUMN ownership check deleted, so a stranger queues for ShareUpdateExclusiveLock before it is refused +projection_drop_column projection_drop_column a non-owner learns nothing from a projected column 15;16;17;18;19 2026-10-22 columnar_tableam.c: both DROP COLUMN ownership checks deleted, so a stranger gets 2BP01 for a projected column and 42501 for another +projection_drop_column projection_drop_column a parent DROP sees projections on columnar partitions 15;16;17;18;19 never - +projection_drop_column projection_drop_column and the projection still receives the row 15;16;17;18;19 never - +projection_drop_column projection_drop_column and the two refusals are the same error, so neither leaks 15;16;17;18;19 2026-10-22 columnar_tableam.c: both DROP COLUMN ownership checks deleted, so a stranger gets 2BP01 for a projected column and 42501 for another +projection_drop_column projection_drop_column dropping a projected column is refused as a dependency 15;16;17;18;19 never - +projection_drop_column projection_drop_column dropping an unrelated column remains allowed 15;16;17;18;19 never - +projection_drop_column projection_drop_column premise: the owner holds SHARE on pdc before the stranger asks 15;16;17;18;19 never - +projection_drop_column projection_drop_column the rejected DDL leaves the table writable 15;16;17;18;19 never - +projection_drop_column projection_drop_column the rejected parent DDL leaves its partition writable 15;16;17;18;19 never - +projection_drop_column projection_drop_column the same non-owner error is returned for an unprojected column 15;16;17;18;19 never - +projection_parallel projection_parallel ANALYZE printed a rows= line per launched worker 15;16;17;18;19 2026-10-22 columnar_customscan.c: the parallel projection path's parallel_workers set to 1 instead of workers +projection_parallel projection_parallel EXPLAIN ANALYZE launched two workers 15;16;17;18;19 2026-10-22 columnar_customscan.c: the parallel projection path's parallel_workers set to 1 instead of workers projection_parallel projection_parallel a covering projection can be a parallel scan 15;16;17;18;19 2026-09-18 - projection_parallel projection_parallel a page cost that makes I/O dominate costs the parallel covering path 15;16;17;18;19 2026-09-23 the partial covering total amortises I/O as well as CPU: (ioRunProj + cpuRunProj) / divisor projection_parallel projection_parallel a parallel covering projection returns the covering rows once 15;16;17;18;19 2026-10-13 a parallel worker never wires the shared row-group counter, so every worker scans every group -projection_parallel projection_parallel premise: ANALYZE printed a rows= line per launched worker 15;16;17;18;19 never - -projection_parallel projection_parallel premise: EXPLAIN ANALYZE launched two workers 15;16;17;18;19 never - projection_parallel projection_parallel premise: a covering projection exists 15;16;17;18;19 never - projection_parallel projection_parallel premise: a parallel base scan is available when the projection is off 15;16;17;18;19 never - projection_parallel projection_parallel premise: a serial covering query uses the projection 15;16;17;18;19 never - @@ -5224,18 +5235,18 @@ projection_parallel projection_parallel premise: the covering plan is parallel a projection_parallel projection_parallel premise: the table holds every inserted row 15;16;17;18;19 never - projection_parallel projection_parallel the parallel count is a parallel covering projection scan 15;16;17;18;19 never - projection_parallel projection_parallel the workers' rows add up to the covering rows, each returned once 15;16;17;18;19 2026-10-13 a parallel worker never wires the shared row-group counter, so every worker scans every group -projection_parallel projection_parallel workers share the covering projection scan, it is not a single claimer 15;16;17;18;19 2026-10-13 first-wins-claimer;only the first parallel worker claims row groups, so one worker scans the whole table (#1336) -projection_privilege projection_privilege a caller with no SELECT is told a privilege is missing, not that RLS is in force 15;16;17;18;19 never - +projection_parallel projection_parallel workers share the covering projection scan, it is not a single claimer 15;16;17;18;19 2026-10-22 columnar_customscan.c: the parallel projection path's parallel_workers set to 1 instead of workers;first-wins-claimer;only the first parallel worker claims row groups, so one worker scans the whole table (#1336) +projection_privilege projection_privilege a caller with no SELECT is told a privilege is missing, not that RLS is in force 15;16;17;18;19 2026-10-22 columnar_projection.c: read_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege a role WITH SELECT still reads the projection 15;16;17;18;19 2026-10-14 read_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows projection_privilege projection_privilege a role with EXECUTE but no SELECT is refused read_projection 15;16;17;18;19 never - projection_privilege projection_privilege a role with only schema USAGE is refused read_projection 15;16;17;18;19 never - -projection_privilege projection_privilege and THAT refusal is the C check, which raises 42501 from aclcheck_error 15;16;17;18;19 never - +projection_privilege projection_privilege and THAT refusal is the C check, which raises 42501 from aclcheck_error 15;16;17;18;19 2026-10-22 columnar_projection.c: read_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege and is refused reconstruct_via_projection 15;16;17;18;19 never - -projection_privilege projection_privilege and is refused reconstruct_via_projection, which leaks non-covered columns 15;16;17;18;19 never - +projection_privilege projection_privilege and is refused reconstruct_via_projection, which leaks non-covered columns 15;16;17;18;19 2026-10-22 columnar_projection.c: reconstruct_via_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege and read_projection does NOT, so the two differ as the issue says 15;16;17;18;19 2026-10-14 read_projection bound to the reconstruct_via_projection C symbol in the extension SQL, so it returns the non-covered columns too;read_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows -projection_privilege projection_privilege and so does reconstruct_via_projection 15;16;17;18;19 never - +projection_privilege projection_privilege and so does reconstruct_via_projection 15;16;17;18;19 2026-10-22 columnar_vacuum.c: PgColumnarRequireNoRowSecurity raises 42501 instead of 0A000 projection_privilege projection_privilege and still reconstructs 15;16;17;18;19 2026-10-14 reconstruct_via_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows -projection_privilege projection_privilege and the refusal says the feature is not supported, not that a privilege is missing 15;16;17;18;19 never - +projection_privilege projection_privilege and the refusal says the feature is not supported, not that a privilege is missing 15;16;17;18;19 2026-10-22 columnar_vacuum.c: PgColumnarRequireNoRowSecurity raises 42501 instead of 0A000 projection_privilege projection_privilege premise: t_prjexec can open a session, so a refusal below is a refusal 15;16;17;18;19 never - projection_privilege projection_privilege premise: t_prjexec really does hold EXECUTE, so it reaches the C gate 15;16;17;18;19 never - projection_privilege projection_privilege premise: t_prjsel CAN read the table, so its arm below tests the bar 15;16;17;18;19 never - @@ -5243,11 +5254,11 @@ projection_privilege projection_privilege premise: the test roles are not superu projection_privilege projection_privilege premise: the unprivileged role cannot read the table by any ordinary route 15;16;17;18;19 never - projection_privilege projection_privilege read_projection now refuses a policy-restricted caller (#563) 15;16;17;18;19 never - projection_privilege projection_privilege reconstruct really does return the NON-COVERED column 15;16;17;18;19 2026-10-14 reconstruct_via_projection renders every column the projection does not cover as NULL instead of the base value;reconstruct_via_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows -projection_privilege projection_privilege reconstruct: a caller with no SELECT is told a privilege is missing, not that RLS is in force 15;16;17;18;19 never - +projection_privilege projection_privilege reconstruct: a caller with no SELECT is told a privilege is missing, not that RLS is in force 15;16;17;18;19 2026-10-22 columnar_projection.c: reconstruct_via_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege reconstruct: the EXECUTE role reaches the projection lookup on the same call 15;16;17;18;19 never - -projection_privilege projection_privilege reconstruct: the base ACL is checked before the projection name is looked up 15;16;17;18;19 never - +projection_privilege projection_privilege reconstruct: the base ACL is checked before the projection name is looked up 15;16;17;18;19 2026-10-22 columnar_projection.c: reconstruct_via_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege reconstruct: the no-EXECUTE role never reaches the body, so the grant is what stopped it 15;16;17;18;19 never - -projection_privilege projection_privilege the base ACL is checked before the projection name is looked up 15;16;17;18;19 never - +projection_privilege projection_privilege the base ACL is checked before the projection name is looked up 15;16;17;18;19 2026-10-22 columnar_projection.c: read_projection's aclcheck_error replaced by a plain elog, so its refusal is XX000 projection_privilege projection_privilege the no-EXECUTE role never reaches the body, so the grant is what stopped it 15;16;17;18;19 never - projection_privilege projection_privilege the owner reads the projection, and gets rows 15;16;17;18;19 2026-10-14 read_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows projection_privilege projection_privilege the owner reconstructs, and gets rows 15;16;17;18;19 2026-10-14 reconstruct_via_projection stores no rows: its tuplestore_putvalues call removed, so every caller gets zero rows @@ -5365,6 +5376,7 @@ projection_scan_cost projection_scan_cost while a plain range on the sort key st projection_scan_io projection_scan_io a covering projection is not priced from the base table's pages 15;16;17;18;19 2026-09-19 rel->pages projection_scan_io projection_scan_io a covering projection whose storage cannot be found is not priced as one page 15;16;17;18;19 2026-09-21 return 1 projection_scan_io projection_scan_io an empty covering projection is still priced as one page, so it is chosen 15;16;17;18;19 2026-09-23 pages = 997 on the third return of pgcolumnar_projection_pages +projection_scan_io projection_scan_io and its scan is not priced at zero pages 15;16;17;18;19 2026-10-22 columnar_customscan.c: the one-page floor in pgcolumnar_projection_pages deleted, so an empty projection is priced at 0 pages projection_scan_io projection_scan_io premise: a covering path is still offered when projection storage cannot be found 15;16;17;18;19 never - projection_scan_io projection_scan_io premise: a covering projection exists 15;16;17;18;19 never - projection_scan_io projection_scan_io premise: the covering projection occupies a minority of the relation 15;16;17;18;19 never - @@ -5413,6 +5425,7 @@ projections projections drop unknown rejected 15;16;17;18;19 never - projections projections duplicate column rejected 15;16;17;18;19 never - projections projections duplicate name rejected 15;16;17;18;19 never - projections projections empty columns rejected 15;16;17;18;19 never - +projections projections executing it reads the projection, so the filter sees fewer rows than the base holds 15;16;17;18;19 2026-10-22 columnar_customscan.c: projection pages multiplied by 1000000, so the planner chooses the base;columnar_customscan.c: the executor never opens the chosen projection (if (false && projName)) while EXPLAIN still names it projections projections fp chunks carry min/max skip metadata 15;16;17;18;19 never - projections projections fp count after delete matches base 15;16;17;18;19 never - projections projections fp fan-out matches base (a,c) 15;16;17;18;19 never - @@ -5424,7 +5437,7 @@ projections projections fq fan-out matches base (b) 15;16;17;18;19 never - projections projections new snapshot projection scan sees both batches 15;16;17;18;19 never - projections projections no projection rows before first add 15;16;17;18;19 never - projections projections non-covering query (references b) uses the base 15;16;17;18;19 never - -projections projections old snapshot projection scan does not see post-snapshot rows 15;16;17;18;19 never - +projections projections old snapshot projection scan does not see post-snapshot rows 15;16;17;18;19 2026-10-22 columnar_customscan.c: the projection scan and its base liveness cache both read GetLatestSnapshot() instead of the query snapshot projections projections p1 columns 15;16;17;18;19 never - projections projections p1 gone after drop 15;16;17;18;19 never - projections projections p1 has its own storage id 15;16;17;18;19 never - @@ -5433,9 +5446,9 @@ projections projections p1 sort_key 15;16;17;18;19 never - projections projections p2 columns 15;16;17;18;19 never - projections projections p2 id is 2 15;16;17;18;19 never - projections projections p2 sort_key empty 15;16;17;18;19 never - -projections projections planner still uses projection after vacuum 15;16;17;18;19 never - +projections projections planner still uses projection after vacuum 15;16;17;18;19 2026-10-22 columnar_customscan.c: projection pages multiplied by 1000000, so the planner chooses the base projections projections premise: the base table holds every committed row 15;16;17;18;19 never - -projections projections projection chosen for covering + sort-key query 15;16;17;18;19 never - +projections projections projection chosen for covering + sort-key query 15;16;17;18;19 2026-10-22 columnar_customscan.c: projection pages multiplied by 1000000, so the planner chooses the base projections projections projection matches base after second vacuum 15;16;17;18;19 never - projections projections projection scan matches oracle after delete 15;16;17;18;19 never - projections projections projection survives vacuum 15;16;17;18;19 never - @@ -5454,6 +5467,7 @@ projections projections storage id resolves 15;16;17;18;19 never - projections projections table populated 15;16;17;18;19 never - projections projections table still readable after DDL 15;16;17;18;19 never - projections projections the drop really removed the projection 15;16;17;18;19 never - +projections projections the old-snapshot count is planned as a projection scan 15;16;17;18;19 2026-10-22 columnar_customscan.c: projection pages multiplied by 1000000, so the planner chooses the base projections projections the rebuild does not abort on it 15;16;17;18;19 never - projections projections two declared projections to start 15;16;17;18;19 never - projections projections two rows after first add (base + p1) 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 7e51bae74..9db5658cf 100644 --- a/test/check_ledger_budget.txt +++ b/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 74 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -5957,4 +5957,34 @@ 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 +checks_never_observed_red 4132 diff --git a/test/projection_drop_column.sh b/test/projection_drop_column.sh index 724631665..8500ba707 100755 --- a/test/projection_drop_column.sh +++ b/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. diff --git a/test/projection_parallel.sh b/test/projection_parallel.sh index 010951c60..7d041d14f 100755 --- a/test/projection_parallel.sh +++ b/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 diff --git a/test/projection_privilege.sh b/test/projection_privilege.sh index 579903195..f369c9249 100755 --- a/test/projection_privilege.sh +++ b/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. diff --git a/test/projection_scan_io.sh b/test/projection_scan_io.sh index 9aaefc3e6..6dff914db 100755 --- a/test/projection_scan_io.sh +++ b/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 diff --git a/test/projections.sh b/test/projections.sh index 5a546d47b..54313063f 100755 --- a/test/projections.sh +++ b/test/projections.sh @@ -194,6 +194,19 @@ check "projection-scan results match heap oracle" \ check "aggregate over projection scan matches oracle" \ "$(q "SELECT count(*), sum(a) FROM ps WHERE c BETWEEN 100 AND 200;")" \ "$(q "SELECT count(*), sum(a) FROM ps_h WHERE c BETWEEN 100 AND 200;")" +# THE EXECUTOR, NOT THE PLANNER (#1339). The first arm reads the name from +# EXPLAIN, which takes it before the executor decides what to open, and the two +# oracle arms pass whichever storage is read. So a scan that EXPLAINs as `pc` and +# reads the base passed all three. Executed, the sorted projection lets the +# reader skip the vectors outside 100..200 and the filter sees only part of the +# table; the base, unsorted on c, sends every row to the filter. +pj_removed="$(q "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT a, c FROM ps WHERE c BETWEEN 100 AND 200;" | + grep -m1 'Rows Removed by Filter:' | grep -oE '[0-9.]+$')" +pj_outside="$(q "SELECT count(*) FROM ps_h WHERE NOT (c BETWEEN 100 AND 200);")" +check "executing it reads the projection, so the filter sees fewer rows than the base holds" \ + "$(if pgc_is_number "$pj_removed" && pgc_is_number "$pj_outside"; then + awk -v r="$pj_removed" -v o="$pj_outside" 'BEGIN { print (r + 0 < o + 0 ? "fewer" : "all " r " of " o) }' + else echo "unread: [$pj_removed] [$pj_outside]"; fi)" "fewer" check "GUC off: no projection scan" \ "$(q "SET pgcolumnar.enable_projection_scan=off; EXPLAIN (COSTS OFF) SELECT a, c FROM ps WHERE c BETWEEN 100 AND 200;" | grep -c 'Columnar Projection')" "0" @@ -266,6 +279,12 @@ psql_run "CREATE TABLE pm (a int, c int) USING pgcolumnar;" psql_run "SELECT pgcolumnar.add_projection('pm', 'pmp', ARRAY['a','c'], ARRAY['c']);" psql_run "INSERT INTO pm SELECT g, g FROM generate_series(1,10000) g;" # batch 1 +# THE PLAN THE ARMS BELOW NAME (#1339). Four of them say "via projection" or +# "projection scan", and nothing asked how this count plans. Planned on the base, +# every one of them passes whatever the projection scan does with the snapshot. +check "the old-snapshot count is planned as a projection scan" \ + "$(q "SET pgcolumnar.enable_projection_scan = on; EXPLAIN (COSTS OFF) SELECT count(a) FROM pm WHERE c BETWEEN 1 AND 20000;" | grep -c 'Columnar Projection: pmp')" "1" + A_IN="$PGC_WORKDIR/pmA.in"; A_OUT="$PGC_WORKDIR/pmA.out" mkfifo "$A_IN"; : > "$A_OUT" env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ @@ -273,7 +292,10 @@ env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ A_PID=$! exec 9> "$A_IN" pa_send() { printf '%s\n' "$*" >&9; } -pa_wait() { local t="$1" i; for i in $(seq 1 200); do grep -q "$t" "$A_OUT" && return 0; sleep 0.1; done; return 1; } +# The token must carry a count. An ERROR echoes the statement, token included +# ("LINE 1: SELECT 'PMA_' || ..."), so matching the bare token read a failed +# query as an answer and the timeout arms below could not fire on it (#1339). +pa_wait() { local t="$1" i; for i in $(seq 1 200); do grep -qE "${t}[0-9]+" "$A_OUT" && return 0; sleep 0.1; done; return 1; } # A opens a snapshot that sees batch 1, using a projection scan. pa_send "SET pgcolumnar.enable_projection_scan = on;" diff --git a/test/pytest/test_projection_drop_column.py b/test/pytest/test_projection_drop_column.py index 9768cb38e..26e25302d 100644 --- a/test/pytest/test_projection_drop_column.py +++ b/test/pytest/test_projection_drop_column.py @@ -31,11 +31,14 @@ premise below fails first and names reachability instead of leaving `42501 != 42P01` to be interpreted. -ONE THING THIS PORT ASSERTS THAT THE BASH SUITE DOES NOT. The two non-owner arms exist -to show that a stranger cannot tell a projected column from an unprojected one. The bash -suite asserts each against the literal `42501` and leaves the reader to notice they are -the same string. Here they are also compared to each other, so the property -- these two -are INDISTINGUISHABLE -- is asserted rather than implied by two separate constants. +THE TWO NON-OWNER REFUSALS ARE COMPARED TO EACH OTHER, in both suites (#1339). The arms +exist to show that a stranger cannot tell a projected column from an unprojected one, so +the property -- these two are INDISTINGUISHABLE -- is asserted directly rather than +implied by two separate constants, and it is asserted before them, so it can fail. + +**The lock is tested by contention.** The ownership check runs before the lock and again +after it, and both raise 42501, so a SQLSTATE alone cannot tell which one refused. While +the owner holds SHARE on the table, only the unlocked check refuses a stranger at once. """ import psycopg @@ -110,15 +113,43 @@ def test_drop_column_is_refused_while_a_projection_depends_on_it(pgc_conn, expec proj_err = _as_nobody(cur, "ALTER TABLE pdc DROP COLUMN sort_key") plain_err = _as_nobody(cur, "ALTER TABLE pdc DROP COLUMN payload") + # The property is that the two are INDISTINGUISHABLE, not that each happens to + # equal a constant, so it is compared FIRST. Placed after the two constant arms + # it could never run red: `expect` stops at the first failure, and if both + # constants hold the two are equal. 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 (#1339). + expect.text(proj_err.sqlstate if proj_err else "no error", + plain_err.sqlstate if plain_err else "no error", + "and the two refusals are the same error, so neither leaks") expect.sqlstate(proj_err, "42501", "a non-owner learns nothing from a projected column") expect.sqlstate(plain_err, "42501", "the same non-owner error is returned for an unprojected column") - # EXTRA: the property is that the two are INDISTINGUISHABLE, not that each - # happens to equal a constant. - expect.text(proj_err.sqlstate if proj_err else "no error", - plain_err.sqlstate if plain_err else "no error", - "premise: and the two refusals are the same error, so neither leaks") + + # THE LOCK HALF OF THE PROPERTY (#1339). The ownership check runs twice: once + # with no lock, then again after ShareUpdateExclusiveLock. The arms 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 = psycopg.connect(pgc_conn.info.dsn, autocommit=False) + try: + holder.execute(f'SET search_path TO "{schema}"') + holder.execute("LOCK TABLE pdc IN SHARE MODE") + held = _one(cur, "SELECT count(*) FROM pg_locks WHERE relation = 'pdc'::regclass " + "AND mode = 'ShareLock' AND granted") + expect.num(held, 1, "premise: the owner holds SHARE on pdc before the stranger asks") + cur.execute("SET lock_timeout = '2s'") + try: + waited_err = _as_nobody(cur, "ALTER TABLE pdc DROP COLUMN sort_key") + finally: + cur.execute("RESET lock_timeout") + expect.sqlstate(waited_err, "42501", + "a non-owner is refused before it waits for the table lock") + finally: + holder.rollback() + holder.close() # --- the owner is refused as a dependency --------------------------- # diff --git a/test/pytest/test_projection_parallel.py b/test/pytest/test_projection_parallel.py index c1b673552..56524235d 100644 --- a/test/pytest/test_projection_parallel.py +++ b/test/pytest/test_projection_parallel.py @@ -232,15 +232,18 @@ def test_projection_parallel(pgc_conn, expect): want, "a parallel covering projection returns the covering rows once", ) + # 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 these go red. + # A check a pgcolumnar change can redden is an arm. expect.num( launched, 2, - "premise: EXPLAIN ANALYZE launched two workers", + "EXPLAIN ANALYZE launched two workers", ) expect.num( len(worker_rows), 2, - "premise: ANALYZE printed a rows= line per launched worker", + "ANALYZE printed a rows= line per launched worker", ) expect.num( n_busy, diff --git a/test/pytest/test_projection_privilege.py b/test/pytest/test_projection_privilege.py index 40e8b986c..eb84a8682 100644 --- a/test/pytest/test_projection_privilege.py +++ b/test/pytest/test_projection_privilege.py @@ -254,11 +254,18 @@ def test_a_role_with_execute_but_no_select_is_refused(pgc_cluster, pgc_conn, exp schema = _fixture(cur) _, err = _as(pgc_cluster, EXEC, f"SELECT count(*) FROM pgcolumnar.{func}('prj_secret','p1')", schema) - expect.sqlstate(err, "42501", name) if func == "read_projection": + # TWO PROPERTIES, TWO ARMS, as in `projection_privilege.sh` (#1339). The first + # asks only that the call was refused; the second asks which code refused it. + # Both used to assert 42501 on the same error, so the second could never fail + # alone: with read_projection's aclcheck_error replaced by a plain elog + # (XX000), the first arm went red under the second's name. + expect.text("refused" if err is not None else "noerror", "refused", name) expect.sqlstate( err, "42501", "and THAT refusal is the C check, which raises 42501 from aclcheck_error") + else: + expect.sqlstate(err, "42501", name) def test_which_layer_refused_without_reading_the_message(pgc_cluster, pgc_conn, expect): @@ -372,12 +379,16 @@ def test_a_policy_restricted_caller_is_refused(pgc_cluster, pgc_conn, expect, fu _, err = _as(pgc_cluster, SEL, f"SELECT count(*) FROM pgcolumnar.{func}('prj_secret','p1')", schema) - expect.sqlstate(err, "0A000", name) if func == "read_projection": + # Refused, then WHICH refusal, as in the shell suite (#1339): asserting 0A000 + # twice on one error left the second arm unable to fail on its own. + expect.text("refused" if err is not None else "noerror", "refused", name) expect.sqlstate( err, "0A000", "and the refusal says the feature is not supported, not that a privilege " "is missing") + else: + expect.sqlstate(err, "0A000", name) def test_the_acl_is_checked_before_rls_so_no_privilege_discloses_no_rls_state( diff --git a/test/pytest/test_projection_scan_io.py b/test/pytest/test_projection_scan_io.py index 13a2a20aa..5d58cbc18 100644 --- a/test/pytest/test_projection_scan_io.py +++ b/test/pytest/test_projection_scan_io.py @@ -263,3 +263,14 @@ def test_an_empty_covering_projection_is_priced_as_one_page(pgc_conn, expect): "onck", "an empty covering projection is still priced as one page, so it is chosen", ) + # 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. `_plan` + # sets every CPU cost to 0, so the scan's whole cost is its page cost, and a total + # of zero means it was priced at zero pages. Measured on PG17: 5.0 with the clamp, + # 0.0 with `if (pages < 1) pages = 1;` deleted, and the projection chosen both times. + cost = (node or {}).get("Total Cost") + expect.text( + "unread" if cost is None else ("priced" if cost > 0 else f"free at {cost}"), + "priced", + "and its scan is not priced at zero pages", + ) diff --git a/test/pytest/test_projections.py b/test/pytest/test_projections.py index 29894df6b..3d5ffde13 100644 --- a/test/pytest/test_projections.py +++ b/test/pytest/test_projections.py @@ -136,6 +136,20 @@ def walk(node): return found[0] if found else None +def _plan_field(plan, key): + """-> the first value of `key` anywhere in a JSON plan, or None.""" + if isinstance(plan, dict): + if key in plan: + return plan[key] + plan = list(plan.values()) + if isinstance(plan, list): + for v in plan: + found = _plan_field(v, key) + if found is not None: + return found + return None + + def _read_projection(cur, table, name): """The projection's rows as the server renders them: columns joined by '|'.""" return _rows(cur, "SELECT pgcolumnar.read_projection(%s, %s)", (table, name)) @@ -492,6 +506,23 @@ def test_a_covering_sort_key_query_reads_the_projection(planner, expect): _rows(cur, "SELECT count(*), sum(a) FROM ps_h " "WHERE c BETWEEN 100 AND 200"), "aggregate over projection scan matches oracle") + # THE EXECUTOR, NOT THE PLANNER (#1339). The first arm reads the name from + # EXPLAIN, which takes it before the executor decides what to open, and the + # two oracle arms pass whichever storage is read. So a scan that EXPLAINs as + # `pc` and reads the base passed all three. Executed, the sorted projection + # lets the reader skip the vectors outside 100..200 and the filter sees only + # part of the table; the base, unsorted on c, sends every row to the filter. + # Measured on PG17: 5980 removed against 17980 for the base. + analyzed = _one(cur, "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, " + "FORMAT JSON) " + COVERING) + removed = _plan_field(analyzed, "Rows Removed by Filter") + outside = _one(cur, "SELECT count(*) FROM ps_h WHERE NOT (c BETWEEN 100 AND 200)") + expect.text( + "unread" if removed is None + else ("fewer" if removed < outside else f"all {removed} of {outside}"), + "fewer", + "executing it reads the projection, so the filter sees fewer rows than " + "the base holds") def test_the_guc_is_an_off_switch(planner, expect): @@ -620,6 +651,32 @@ def test_an_old_snapshot_never_sees_rows_committed_after_it(pgc_conn, expect): dsn = pgc_conn.info.dsn COUNT = "SELECT count(a) FROM pm WHERE c BETWEEN 1 AND 20000" + + # THE PLAN THE ARMS BELOW NAME (#1339). Four of them say "via projection" or + # "projection scan", and nothing asked how COUNT plans. Planned on the base, every + # one of them passes whatever the projection scan does with the snapshot. + with pgc_conn.cursor() as cur: + cur.execute("SET pgcolumnar.enable_projection_scan = on") + expect.text(_projection_in_plan(_plan(cur, COUNT)) or "none", "pmp", + "the old-snapshot count is planned as a projection scan") + + def answer(ac): + """-> session A's count, or the error that stopped it. + + The shell suite's two timeout arms fail when session A gives no count. Here a + statement_timeout stands in for its 20-second poll, and an error is caught + rather than raised, so those two arms read what they say: that A answered. + """ + try: + return _one(ac, COUNT) + except psycopg.Error as exc: + ac.connection.rollback() + return exc + + def answered(reading): + return ("answered" if not isinstance(reading, psycopg.Error) + else f"no answer: {reading.sqlstate}") + a = psycopg.connect(dsn, autocommit=False) try: with a.cursor() as ac: @@ -627,10 +684,11 @@ def test_an_old_snapshot_never_sees_rows_committed_after_it(pgc_conn, expect): # the same one or it would read a different (absent) table. ac.execute(f'SET search_path TO "{schema}", public') ac.execute("SET pgcolumnar.enable_projection_scan = on") + ac.execute("SET statement_timeout = '20s'") ac.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") - base = _one(ac, COUNT) - expect.num(base, 10000, "session A opened snapshot") + base = answer(ac) + expect.text(answered(base), "answered", "session A opened snapshot") expect.num(base, 10000, "session A baseline via projection sees batch 1") @@ -638,8 +696,8 @@ def test_an_old_snapshot_never_sees_rows_committed_after_it(pgc_conn, expect): cur.execute("INSERT INTO pm SELECT g, g " "FROM generate_series(10001,20000) g") - after = _one(ac, COUNT) - expect.num(after, 10000, "session A responded post-commit") + after = answer(ac) + expect.text(answered(after), "answered", "session A responded post-commit") expect.num(after, 10000, "old snapshot projection scan does not see post-snapshot rows") a.commit()