diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a3c1a75..16064222 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -615,18 +615,26 @@ true until the next version shipped. This is the third arm in this file to be repaired for counting a string across a whole file. The `deltuples` comment 15 lines above records the first, fixed by scoping; these two were left as whole-file counts and did the same thing again. -- A table-AM parallel scan was a single claimer. - - `pgcolumnar_read_start` treated `phs_nallocated` as a first-wins flag: the - first participant loaded every row group and the others marked themselves - exhausted. Workers launched, then sat idle while one backend (usually the - leader) read the table. The custom-scan path already claims distinct groups - from a shared counter; the AM path now uses `phs_nallocated` the same way, - as a group index, not a mutex. - - Measured with the custom scan off, two workers, and leader participation - off: both workers produced rows (19000 and 31000 of 50000). Restoring - first-wins returns one worker to 0. +- A fetching index scan on a correlated key stayed cheaper than the custom scan + through ~50,000 rows, while doing about 27x the work (#913). + + The index-fetch penalty prices distinct row-group decodes. On a clustered key + that count is ceil(rows / stripe), so it does not grow through the first group. + Core's heap-fetch cost grows with rows; the extra columnar work of reconstructing + each fetched row after the group is cached did not. The 50,000-row range then + stayed on the index (cost 2243 against the custom scan's 2504) while the point + lookup was already correctly on it. + + The per-row term is `cpu_tuple_cost * rows * decodeUnits`, the same units #503 + uses for a projection. It is not a conversion from heap instructions-per-cost: + #766 showed that conversion predicts the wrong winner. Uncapped it grows with + the whole table and costs a clustered ORDER BY off its index (#355). Cap it at + half a group: that is enough to move the 50,000-row range and small enough to + leave the ordered scan on the index. + + Plan choice is the property, not a cost number: at 50,000 rows the planner now + picks the custom scan; a point lookup still uses the index; a clustered ORDER BY + stays on the index; both paths return the same aggregate. - `compare_to_bash.py`'s corpus arm called a WRAPPED name fabricated. A name too long for one line is written as adjacent literals, and Python joins them at parse time, diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index d795950d..484e4ca4 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -1892,6 +1892,20 @@ pgcolumnar_scan_decode_shape(RelOptInfo *rel, Index rti, Oid relid, * * decode_per_group is one group's cost: its pages read once, plus the per-value * decode of R rows across the columns the scan needs. + * + * That is not the whole fetch. Even after the group is in the statement-scoped + * cache, each row still pays tuple reconstruction and group location that a heap + * fetch does not. Without a per-row term the clustered penalty is independent of + * how many rows are fetched, so a 50,000-row correlated range stays on the index + * while doing about 27x the work of the custom scan (#913). cpu_tuple_cost is the + * same unit core already uses for a heap tuple; decodeUnits is the projection in + * #503's 4-byte-column units. Do not scale this from heap instructions-per-cost: + * #766 showed that conversion predicts the wrong winner. + * + * Cap the per-row term at half a group. Uncapped it grows with the whole table + * and costs a clustered ORDER BY off its index, which #355 must not do. Half a + * group is enough to move the 50,000-row range and small enough to leave the + * ordered scan on the index. */ static Cost pgcolumnar_index_fetch_penalty(RelOptInfo *rel, Oid relid, double rows, double rho, @@ -1954,7 +1968,22 @@ pgcolumnar_index_fetch_penalty(RelOptInfo *rel, Oid relid, double rows, double r if (groups_decoded < 0) groups_decoded = 0; - return groups_decoded * decode_per_group; + /* + * Per fetched row, after the group decode is paid once (#913). On a + * clustered key, groups_decoded is ceil(rows/R) and does not grow through + * the first group, so this is the term that moves a 50,000-row range off + * the index without costing a point lookup out of it. Cap at half a + * group so a clustered ORDER BY of the whole table stays on the index + * (#355). + */ + { + Cost per_row = cpu_tuple_cost * rows * decodeUnits; + Cost half_group = 0.5 * decode_per_group; + + if (per_row > half_group) + per_row = half_group; + return groups_decoded * decode_per_group + per_row; + } } /* diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 65151499..69156af0 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1169,6 +1169,12 @@ harness_selftest 490-a-one-line-body-ends-at-its-own-brace control: and the reco harness_selftest 490-a-one-line-body-ends-at-its-own-brace premise: the skip-loop tool is present 15;16;17;18;19 never - harness_selftest 490-a-one-line-body-ends-at-its-own-brace premise: the tool answers with an emitter set, so 'q is absent' is about q 15;16;17;18;19 never - harness_selftest 490-a-one-line-body-ends-at-its-own-brace premise: the tool reports an unclosed count at all, so a zero below means zero 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover a 50000-row correlated range uses the custom scan, not a fetching index 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover a selective point lookup still uses the index 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover both paths return the same aggregate at 50000 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover premise: the btree on id exists 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover premise: the table holds all 1000000 rows 15;16;17;18;19 never - +index_fetch_penalty_crossover index_fetch_penalty_crossover the fetch penalty leaves a clustered ORDER BY on its index 15;16;17;18;19 never - native_chunk_length_bound native_chunk_length_bound a sequential scan of the same poisoned chunk is refused (XX001) 15;16;17;18;19 never - native_chunk_length_bound native_chunk_length_bound an index fetch of a chunk whose page_length is 2^32 too large is refused (XX001) 15;16;17;18;19 never - native_chunk_length_bound native_chunk_length_bound backend survived the sequential refusal 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 979fb74f..9ad7a7d1 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -58,4 +58,4 @@ suites_not_covered 249 # that is not this one. Neither survives. Re-derived by COUNTING on the merged tree, # which is the only resolution this number has: # awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -checks_never_observed_red 1249 +checks_never_observed_red 1255 diff --git a/test/index_fetch_penalty_crossover.sh b/test/index_fetch_penalty_crossover.sh new file mode 100755 index 00000000..0f5c3a25 --- /dev/null +++ b/test/index_fetch_penalty_crossover.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# +# pgColumnar: a fetching index scan on a correlated key is priced below the +# custom scan through ~50,000 rows, while it does about 27x the work (#913). +# +# The penalty term exists for this. The measurement says it is too small. This +# suite asserts the PLAN, not a cost number: costs drift with the constants, the +# chosen node is the property. Split from #766, which closed on the opposite +# question (custom scan vs heap). Raising the custom-scan cost would enlarge the +# wrong-plan region. +# +# Independent of test/pytest/test_index_fetch_penalty_crossover.py: same public +# seam, own fixture, own observations. +# +# Usage: test/index_fetch_penalty_crossover.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +N=1000000 +K_RANGE=50000 + +psql_run "CREATE TABLE ifc (id int, a int, b int) USING pgcolumnar;" +psql_run "INSERT INTO ifc SELECT g, g % 10, g % 100 FROM generate_series(1,$N) g;" +psql_run "CREATE INDEX ifc_id ON ifc(id);" +psql_run "ANALYZE ifc;" + +q1() { + 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 +} + +SETS="SET enable_seqscan=off; +SET enable_bitmapscan=off; +SET enable_indexonlyscan=off; +SET max_parallel_workers_per_gather=0; +SET jit=off; +SET pgcolumnar.enable_vectorization=off; +SET pgcolumnar.enable_ungrouped_vector_agg=off; +SET pgcolumnar.enable_group_vectorization=off;" + +plan_of() { + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "$SETS $1" 2>&1 +} + +top_node() { + # First scan/join node in COSTS OFF text, which is the chosen path. + grep -m1 -oE 'Index Scan|Index Only Scan|Bitmap Heap Scan|Custom Scan|Seq Scan' <<<"$1" +} + +check "premise: the table holds all $N rows" "$(q1 "SELECT count(*) FROM ifc")" "$N" +check "premise: the btree on id exists" \ + "$(q1 "SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = 'ifc'::regclass AND c.relname = 'ifc_id'")" \ + "1" + +PLAN_PT="$(plan_of "EXPLAIN (COSTS OFF) SELECT sum(a) FROM ifc WHERE id = 1")" +echo "-- point lookup: $(printf '%s\n' "$PLAN_PT" | grep -m1 -E 'Scan')" +check "a selective point lookup still uses the index" \ + "$(top_node "$PLAN_PT")" "Index Scan" + +PLAN_50="$(plan_of "EXPLAIN (COSTS OFF) SELECT sum(a) FROM ifc WHERE id <= $K_RANGE")" +echo "-- ${K_RANGE}-row range: $(printf '%s\n' "$PLAN_50" | grep -m1 -E 'Scan')" +check "a ${K_RANGE}-row correlated range uses the custom scan, not a fetching index" \ + "$(top_node "$PLAN_50")" "Custom Scan" + +IDX_SUM="$(q1 "SET enable_seqscan=off; SET enable_bitmapscan=off; SET pgcolumnar.enable_custom_scan=off; SELECT sum(a) FROM ifc WHERE id <= $K_RANGE")" +CS_SUM="$(q1 "SET enable_indexscan=off; SET enable_bitmapscan=off; SELECT sum(a) FROM ifc WHERE id <= $K_RANGE")" +check "both paths return the same aggregate at $K_RANGE" "$IDX_SUM" "$CS_SUM" + +# #355: a clustered ORDER BY of the whole table must stay on the index. A +# per-row term that grows with rows costs this path off onto a Sort. The +# 50,000-row range above projects two ints; this table carries a payload so +# SELECT * is the same shape that #355 must not over-fire on. +N_ORD=300000 +psql_run "CREATE TABLE ifc_cl (id int, payload text) USING pgcolumnar;" +psql_run "INSERT INTO ifc_cl SELECT g, repeat('x', 48) FROM generate_series(1,$N_ORD) g;" +psql_run "CREATE INDEX ifc_cl_id ON ifc_cl(id);" +psql_run "ANALYZE ifc_cl;" + +SETS_ORD="SET max_parallel_workers_per_gather=0; SET random_page_cost=1.0;" +PLAN_ORD="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "$SETS_ORD EXPLAIN (COSTS OFF) SELECT * FROM ifc_cl ORDER BY id" 2>&1)" +echo "-- clustered ORDER BY: $(printf '%s\n' "$PLAN_ORD" | grep -m1 -E 'Scan|Sort')" +# THIS ARM IS THE CAP'S REMOVAL PROOF, and nothing else in the suite is. +# +# The per-row term is capped at half a group. Nothing here NAMES the cap, so a +# reader asking "is that cap load-bearing, or can it be simplified away?" finds +# no arm mentioning it and concludes nothing protects it. That conclusion is +# wrong, and it was reached in writing during review of this PR before anyone +# mutated the code. +# +# Measured, deleting the cap and leaving everything else: +# +# as written 6 passed + 0 failed +# uncapped FAIL the fetch penalty leaves a clustered ORDER BY on its +# index: got [no (Sort)] want [yes] +# +# per_row = cpu_tuple_cost * rows * decodeUnits grows with the whole table on an +# ordered scan, so this IS the saturation case: uncapped it costs the ordered +# scan off its index, which is the #355 regression the cap exists to prevent. +# +# The arm above it is the other side. Together they bound the cap in both +# directions -- too small and the 50,000-row range stays on the index, too large +# and the ordered scan leaves it. Removing either leaves the cap pinned on one +# side only, which is the easy miss. +check "the fetch penalty leaves a clustered ORDER BY on its index" \ + "$(grep -q 'Index Scan using ifc_cl_id' <<<"$PLAN_ORD" && echo yes \ + || echo "no ($(printf '%s' "$PLAN_ORD" | grep -m1 -E 'Scan|Sort'))")" \ + "yes" + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 369f00a0..2a2400ae 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -92,6 +92,7 @@ behaviour, the source of that number is named. - [44. test_native_chunk_length_bound.py: a truncated chunk length cannot fetch](#44-test_native_chunk_length_boundpy-a-truncated-chunk-length-cannot-fetch) - [45. test_native_fetch_coalesce.py: index fetch I/O is not per-column](#45-test_native_fetch_coalescepy-index-fetch-io-is-not-per-column) - [46. test_parallel_am_scan.py: a table-AM parallel scan must share work](#46-test_parallel_am_scanpy-a-table-am-parallel-scan-must-share-work) +- [47. test_index_fetch_penalty_crossover.py: the correlated range must not fetch](#47-test_index_fetch_penalty_crossoverpy-the-correlated-range-must-not-fetch) ## 1. How to read a test in here @@ -4354,11 +4355,6 @@ This file asserts the SQLSTATE, not a cost number. The poison is a catalog UPDATE; the property is that a fetch raises XX001 and the backend survives. Independent of `test/native_chunk_length_bound.sh`. Same public seam, own -fixture, own observations. Assertion names match the shell suite. - -| test | what it asserts | -| --- | --- | -| `test_native_chunk_length_bound` | a point lookup uses the index and returns the row; after `page_length` grows by 2^32, both the fetch and a sequential scan raise XX001 and the backend survives each | ## 45. test_native_fetch_coalesce.py: index fetch I/O is not per-column Index fetch used to pin once per column: validity bitmap, then the value stream, @@ -4400,3 +4396,21 @@ this file uses `ampar`, 80000 rows, groups of 200. Assertion names match. The load-bearing assertion is `workers share the table-AM scan, it is not a single claimer`. It is unreachable while `phs_nallocated` is first-wins, and reachable only when each worker claims its own row groups. +| `test_index_fetch_penalty_crossover` | a 50,000-row correlated range uses the custom scan; a point lookup still uses the index; both paths agree on the aggregate; a clustered ORDER BY stays on the index | +## 47. test_index_fetch_penalty_crossover.py: the correlated range must not fetch + +#913. A fetching index scan on a correlated key is priced below the custom scan +through ~50,000 rows, while it does about 27x the work. The penalty term exists +for this; the measurement says it is too small. Split from #766, which closed +on the opposite question. + +This file asserts the PLAN, not a cost number. Costs drift with the constants. +The chosen node is the property. + +Independent of `test/index_fetch_penalty_crossover.sh`. Same public seam, own +fixture, own observations. Assertion names match the shell suite. + +| test | what it asserts | +| --- | --- | +| `test_native_chunk_length_bound` | a point lookup uses the index and returns the row; after `page_length` grows by 2^32, both the fetch and a sequential scan raise XX001 and the backend survives each | + diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 099600a5..4d634da6 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -238,4 +238,4 @@ guard_tests 346 # the expected answer for a file that needs a cluster, and checking it was the point # rather than assuming it. # cluster_tests re-derived by collection on the rebased tree, never by adding a delta measured on another tree. -cluster_tests 415 +cluster_tests 416 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 5d7b8930..2d18f785 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -89,7 +89,8 @@ "native_chunk_length_bound", "native_fetch_coalesce", "native_ownership", "native_projection", "parallel_am_scan", "projection_privilege", "projections", - "sorted_pathkeys", "stats_privilege", "zonemap_boundaries"] + "sorted_pathkeys", "stats_privilege", + "index_fetch_penalty_crossover", "zonemap_boundaries"] # stem -> why it does not yet reach zero. Empty today, and an entry here is a claim # about the PORT rather than a licence: the standing arm does not grade it, so the diff --git a/test/pytest/test_index_fetch_penalty_crossover.py b/test/pytest/test_index_fetch_penalty_crossover.py new file mode 100644 index 00000000..96c4820a --- /dev/null +++ b/test/pytest/test_index_fetch_penalty_crossover.py @@ -0,0 +1,110 @@ +"""A fetching index scan on a correlated key is priced below the custom scan +through ~50,000 rows, while it does about 27x the work (#913). + +The penalty term exists for this. The measurement says it is too small. This +file asserts the PLAN, not a cost number: costs drift with the constants, the +chosen node is the property. + +Independent of test/index_fetch_penalty_crossover.sh: same public seam, own +fixture, own observations. Assertion names match the shell suite so the two +can be compared by name, not by importing each other. +""" + + +def _scan_node(plan): + """First scan node in the tree, matching the shell suite's grep.""" + stack = [plan[0]["Plan"]] + while stack: + node = stack.pop(0) + t = node["Node Type"] + if t in ( + "Index Scan", + "Index Only Scan", + "Bitmap Heap Scan", + "Custom Scan", + "Seq Scan", + ): + return t + stack.extend(node.get("Plans", ())) + return "" + + +def _plan(conn, sql): + with conn.cursor() as cur: + cur.execute("SET enable_seqscan=off") + cur.execute("SET enable_bitmapscan=off") + cur.execute("SET enable_indexonlyscan=off") + cur.execute("SET max_parallel_workers_per_gather=0") + cur.execute("SET jit=off") + cur.execute("SET pgcolumnar.enable_vectorization=off") + cur.execute("SET pgcolumnar.enable_ungrouped_vector_agg=off") + cur.execute("SET pgcolumnar.enable_group_vectorization=off") + cur.execute(f"EXPLAIN (FORMAT JSON, COSTS OFF) {sql}") + return cur.fetchone()[0] + + +def test_index_fetch_penalty_crossover(pgc_conn, expect): + n = 1_000_000 + k_range = 50_000 + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE ifc (id int, a int, b int) USING pgcolumnar") + cur.execute( + f"INSERT INTO ifc SELECT g, g % 10, g % 100 FROM generate_series(1, {n}) g" + ) + cur.execute("CREATE INDEX ifc_id ON ifc(id)") + cur.execute("ANALYZE ifc") + cur.execute("SELECT count(*) FROM ifc") + expect.num(cur.fetchone()[0], n, f"premise: the table holds all {n} rows") + cur.execute( + "SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid " + "WHERE i.indrelid = 'ifc'::regclass AND c.relname = 'ifc_id'" + ) + expect.num(cur.fetchone()[0], 1, "premise: the btree on id exists") + + point = _plan(pgc_conn, "SELECT sum(a) FROM ifc WHERE id = 1") + expect.text( + _scan_node(point), + "Index Scan", + "a selective point lookup still uses the index", + ) + + ranged = _plan(pgc_conn, f"SELECT sum(a) FROM ifc WHERE id <= {k_range}") + expect.text( + _scan_node(ranged), + "Custom Scan", + f"a {k_range}-row correlated range uses the custom scan, not a fetching index", + ) + + with pgc_conn.cursor() as cur: + cur.execute("SET enable_seqscan=off") + cur.execute("SET enable_bitmapscan=off") + cur.execute("SET pgcolumnar.enable_custom_scan=off") + cur.execute(f"SELECT sum(a) FROM ifc WHERE id <= {k_range}") + idx_sum = cur.fetchone()[0] + cur.execute("SET pgcolumnar.enable_custom_scan=on") + cur.execute("SET enable_indexscan=off") + cur.execute(f"SELECT sum(a) FROM ifc WHERE id <= {k_range}") + cs_sum = cur.fetchone()[0] + expect.num(idx_sum, cs_sum, f"both paths return the same aggregate at {k_range}") + + n_ord = 300_000 + with pgc_conn.cursor() as cur: + cur.execute("RESET ALL") + cur.execute("CREATE TABLE ifc_cl (id int, payload text) USING pgcolumnar") + cur.execute( + "INSERT INTO ifc_cl SELECT g, repeat('x', 48) " + f"FROM generate_series(1, {n_ord}) g" + ) + cur.execute("CREATE INDEX ifc_cl_id ON ifc_cl(id)") + cur.execute("ANALYZE ifc_cl") + cur.execute("SET max_parallel_workers_per_gather=0") + cur.execute("SET random_page_cost=1.0") + cur.execute( + "EXPLAIN (FORMAT JSON, COSTS OFF) SELECT * FROM ifc_cl ORDER BY id" + ) + ordered = cur.fetchone()[0] + expect.text( + _scan_node(ordered), + "Index Scan", + "the fetch penalty leaves a clustered ORDER BY on its index", + ) diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index d755f8ef..fdc45ef1 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -115,6 +115,7 @@ SUITES=( import_exclusion import_export_privilege index_delete_liveness + index_fetch_penalty_crossover index_fetch_penalty_width index_only inheritance