From 08fc6eaae296de7236d89f7f0000484f21cc3d3d Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Fri, 18 Sep 2026 17:55:45 +0000 Subject: [PATCH] fix: price a covering projection by what it can prune, not what it mentions (#1126) #1107 replaced a constant 0.5 with the selectivity of clauses referencing sortKey[0]. That fixed the case where the selectivity came from a different column. It left a narrower one: a single RestrictInfo that ORs a sort-key range with a predicate on another column REFERENCES the sort key, so the membership test counted it whole and credited the projection with a selectivity its sort order cannot deliver. WHERE sk BETWEEN 1 AND 2000 priced 0.100 earns it WHERE sk BETWEEN 1 AND 2000 OR kind = 'odd' priced 0.101 earns nothing The second prunes nothing and is slower than the base scan it undercuts tenfold: 0 usable skip predicates, 0 vectors skipped, all 20 chunk groups read, 120 vector decodes against 80, 2.653 ms against 1.996 ms. ASK THE FUNCTION THAT DECIDES SKIPPING. pgcolumnar_clause_to_scankey already answers "can this clause prune, and on which column": it returns 0 for a BoolExpr, because a BoolExpr is not an OpExpr and never becomes a scan key, and it records sk_attno per key. Pricing now keeps a clause only when it yields at least one key and every key it yields is on the sort key. One definition of "can skip", shared by the price and the executor, rather than a second one restated in the cost path. NOT GATED ON exact. The batch fold needs exactness because scan keys are its whole row filter (#715); pruning does not. An anchored LIKE (#426) and an IN-list range (#704) prune honestly, and gating on exactness would decline a projection that genuinely wins. That is the silent direction, so its control ships beside the arm: mutating the gate onto exact reddens the IN-list arm exactly as intended. THE FIXTURE HAD TO CLEAR THE ONE-STRIPE FLOOR. The first version of the arm used a 100-row range; at 20 stripes the floor is 0.05 and both the fabricated discount and the honest one price there, so a broken guard and a working one were indistinguishable and the arm passed against the defect. The range is 10% now and a premise asserts it is above the floor, so the arm cannot quietly go vacuous again. Checking only the first scan key rather than every key reddens nothing, because no current clause shape writes keys on two columns. That is recorded in the comment as untested insurance rather than claimed as a property. THE PARITY TOOL CAUGHT THE PORT BEFORE CI DID, on three of the five new names. This suite is declared one-for-one with its pytest twin, so a property has to be asserted under the SAME name on both sides, and I had written three of them differently: MISSING premise: the prunable range is priced above the one-stripe floor, so the arms differ extra premise: the prunable range is priced above the one-stripe floor The bash names are canonical here because the ledger rows were seeded from a bash run, so the port adopts them rather than the reverse. Renaming the ledger side would have meant re-seeding five rows across five majors to fix a typo. NOT A REGRESSION FROM #1107: the old 0.5 also beat the base for this query and the planner also chose the projection. What changed is how confidently. Five ledger rows, seeded from one run per major merged in a single call, all carrying 15;16;17;18;19. Census re-derived by counting on the merged tree: 1396. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- CHANGELOG.md | 50 ++++++++++++++++ docs/ARCHITECTURE.md | 6 +- src/columnar_customscan.c | 64 ++++++++++++++++++-- test/check_ledger.tsv | 5 ++ test/check_ledger_budget.txt | 20 ++++++- test/projection_scan_cost.sh | 60 +++++++++++++++++++ test/pytest/test_projection_scan_cost.py | 74 ++++++++++++++++++++++++ 7 files changed, 272 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2458f65..fe52aad9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,56 @@ true until the next version shipped. ### Fixed +- A covering projection was priced by clauses that merely mention its sort key, + rather than by clauses it can prune on (#1126, the remainder of #1107). + + #1107 replaced a constant `0.5` with the selectivity of the clauses referencing + `sortKey[0]`, which fixed the case where the selectivity came from a different + column entirely. It left a narrower one: **a single `RestrictInfo` that ORs a + sort-key range with a predicate on another column references the sort key**, so + the membership test counted it whole and credited the projection with a + selectivity its sort order cannot deliver. + + WHERE sk BETWEEN 1 AND 2000 priced 0.100 earns it + WHERE sk BETWEEN 1 AND 2000 OR kind = 'odd' priced 0.101 earns nothing + + Measured on 20,000 rows, `stripe_row_limit => 1000`, scrambled physical order, + projection sorted on `sk`, with `kind = 'odd'` where `sk % 1000 = 0` so its rows + are spread across the whole `sk` domain. The second query prunes NOTHING and is + slower than the base scan it undercuts tenfold: + + Columnar Usable Skip Predicates 0 with the projection, 0 without + Columnar Vectors Skipped 0 with the projection, 0 without + Columnar Chunk Groups Read 20 of 20, both ways + Columnar Vector Decodes 120 with the projection, 80 without + Execution Time 2.653 ms with, 1.996 ms without + + ASK THE FUNCTION THAT DECIDES SKIPPING. `pgcolumnar_clause_to_scankey` already + answers "can this clause prune, and on which column" -- it returns 0 for a + `BoolExpr`, because a BoolExpr is not an `OpExpr` and never becomes a scan key -- + and it records `sk_attno` per key. Pricing now keeps a clause only when it yields + at least one key and every key it yields is on the sort key. One definition of + "can skip", shared by the price and the executor, rather than a second one + restated in the cost path. + + NOT GATED ON `exact`. The batch fold needs exactness because scan keys are its + whole row filter (#715); pruning does not. An anchored `LIKE` (#426) and an + IN-list range (#704) are conservative keys that prune honestly, and gating on + exactness would decline a projection that genuinely wins. That is the silent + direction -- a plan not taken reddens nothing -- so the arm for it ships beside + the arm for the defect, and mutation-testing the gate onto `exact` reddens the + IN-list arm exactly as intended. + + THE FIXTURE HAD TO CLEAR THE ONE-STRIPE FLOOR. The first version of this arm used + a 100-row range; at 20 stripes the floor is 0.05 and both the fabricated discount + and the honest one price there, so a broken guard and a working one were + indistinguishable and the arm passed against the defect. The range is 10% now, + and a premise check asserts it is above the floor so the arm cannot quietly + return to being vacuous. + + NOT A REGRESSION FROM #1107: the old constant `0.5` also beat the base for this + query and the planner also chose the projection. What changed is how confidently. + - The union-merge page did not say why rebasing works where merging does not (#1116 follow-up). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2fef8d1d..0b150f92 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -463,7 +463,11 @@ live in `columnar_write_state.c` (each projection stores the base row number as leading column); the planner selection and executor projection scan live in `columnar_customscan.c` (a covering projection with a restricted sort key is scanned instead of the base, pruning chunk groups by the projection's min/max, -with deletes/visibility taken from the base). `pgcolumnar.vacuum` rebuilds +with deletes/visibility taken from the base). "Restricted" means the query carries a +clause that can prune on the sort key. A clause prunes when it becomes a scan +key. A clause that only names the column does not prune. For example, an `OR` of +a sort-key range with a condition on another column names the sort key and prunes +nothing, so it earns no discount (#1126). `pgcolumnar.vacuum` rebuilds projections aligned to the compacted base. ## Data flow summaries diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index 80fc813c..5a931e69 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -1622,29 +1622,82 @@ pgcolumnar_sorted_pathkeys(PlannerInfo *root, RelOptInfo *rel, Oid relid) * reference disqualifies a projection scan. */ static Selectivity -pgcolumnar_sortkey_selectivity(PlannerInfo *root, RelOptInfo *rel, +pgcolumnar_sortkey_selectivity(PlannerInfo *root, RelOptInfo *rel, Oid relid, AttrNumber sortAttno) { List *clauses = NIL; ListCell *lc; + Relation r; + TupleDesc tupdesc; /* * Eligibility tests sortKey[0] against restrictCols. Pricing has to use * the same clauses: rel->rows is the estimate after EVERY restriction, * including columns the projection cannot prune on. + * + * MENTIONING THE SORT KEY IS NOT PRUNING ON IT (#1126). A single + * RestrictInfo that ORs a sort-key range with a predicate on another column + * references the key, so a membership test counts it whole and credits the + * projection with a selectivity the sort order cannot deliver. Measured + * before this changed: `sk BETWEEN 1 AND 2000 OR kind = 'odd'` priced at + * 0.101 of the base while pruning nothing -- no pushed-down filter, no + * usable skip predicate, every chunk group read, 50% more vector decodes and + * 33% slower than the base scan it undercut tenfold. + * + * SO ASK THE FUNCTION THAT DECIDES SKIPPING, rather than restating its rule + * here. `pgcolumnar_clause_to_scankey` returns 0 for a clause it cannot use + * -- a BoolExpr is not an OpExpr and never becomes a key -- and records the + * column each key prunes on. That keeps one definition of "can skip" for the + * price and the executor, which is selftest 320's rule: a check that + * recomputes a rule tests the world instead of the code. + * + * NOT `exact`. The fold needs exactness because scan keys are its whole row + * filter (#715); pruning does not. An anchored LIKE (#426) and an IN-list + * range (#704) are conservative keys that prune honestly, and gating on + * exactness would decline a projection that genuinely wins -- the silent + * direction, since a plan not taken reddens nothing. */ if (sortAttno <= 0) return (Selectivity) 1.0; + r = table_open(relid, AccessShareLock); + tupdesc = RelationGetDescr(r); + foreach(lc, rel->baserestrictinfo) { RestrictInfo *ri = lfirst_node(RestrictInfo, lc); - Bitmapset *cols = NULL; + ScanKeyData scratch[2]; /* an anchored LIKE writes two keys */ + bool exact = false; + int n; + int i; + bool allOnSortKey; - pull_varattnos((Node *) ri->clause, rel->relid, &cols); - if (bms_is_member(sortAttno - FirstLowInvalidHeapAttributeNumber, cols)) + n = pgcolumnar_clause_to_scankey((Node *) ri->clause, rel->relid, + tupdesc, &scratch[0], &exact); + if (n < 1) + continue; + + /* + * EVERY key, not the first. One clause writing keys on two different + * columns would otherwise be credited entirely to the sort key on the + * strength of whichever came first. No current shape does that, which + * is a reason to be cheap about it rather than a reason to assume it. + */ + allOnSortKey = true; + for (i = 0; i < n; i++) + { + if (scratch[i].sk_attno != sortAttno) + { + allOnSortKey = false; + break; + } + } + if (allOnSortKey) clauses = lappend(clauses, ri); } + + table_close(r, AccessShareLock); + if (clauses == NIL) return (Selectivity) 1.0; return clauselist_selectivity(root, clauses, rel->relid, JOIN_INNER, NULL); @@ -2949,7 +3002,8 @@ PgColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, * and a 50% range the same price. */ serialRun = serialTotalCost - serialStartupCost; - sel = (double) pgcolumnar_sortkey_selectivity(root, rel, sortAttno); + sel = (double) pgcolumnar_sortkey_selectivity(root, rel, rte->relid, + sortAttno); if (sel < 0.0) sel = 0.0; if (sel > 1.0) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 8936d866..f8ff057a 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1388,15 +1388,20 @@ parallel_scan_cost parallel_scan_cost premise: the serial plan has no Gather 15; parallel_scan_cost parallel_scan_cost premise: the serial plan is a columnar scan 15;16;17;18;19 never - parallel_scan_cost parallel_scan_cost premise: the table holds every inserted row 15;16;17;18;19 never - parallel_scan_cost parallel_scan_cost the leader-participation branch changes the divisor 15;16;17;18;19 never - +projection_scan_cost projection_scan_cost a clause that mentions the sort key but cannot prune on it does not cheapen a covering projection 15;16;17;18;19 never - projection_scan_cost projection_scan_cost a non-sort-key restriction does not cheapen a covering projection 15;16;17;18;19 2026-09-18 - projection_scan_cost projection_scan_cost a tight covering projection is cheaper relative to the base than a loose one 15;16;17;18;19 2026-09-17 - +projection_scan_cost projection_scan_cost and an IN-list on the sort key keeps its discount, which gating on exactness would lose 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: a covering projection exists 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: every compared scan has a positive run cost 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: every misattributed scan has a positive run cost 15;16;17;18;19 never - +projection_scan_cost projection_scan_cost premise: every unprunable-clause scan has a positive run cost 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: the loose plan uses the covering projection 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: the misattributed query has a covering projection 15;16;17;18;19 never - +projection_scan_cost projection_scan_cost premise: the prunable range is priced above the one-stripe floor, so the arms differ 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: the table holds every inserted row 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: the tight plan uses the covering projection 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: without the projection scan, the loose plan is a base columnar scan 15;16;17;18;19 never - projection_scan_cost projection_scan_cost premise: without the projection scan, the tight plan is a base columnar scan 15;16;17;18;19 never - projection_scan_cost projection_scan_cost tight and loose covering scans are not both priced at half the base 15;16;17;18;19 2026-09-17 - +projection_scan_cost projection_scan_cost while a plain range on the sort key still earns its discount 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 9044ff3d..133df503 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -110,4 +110,22 @@ suites_not_covered 249 # and TESTS.md conflicted loudly -- the usual pairing -- so the union was checked # by KEY: 0 keys lost from either side, 14 added, 0 duplicates. Re-derived by the # command above. -checks_never_observed_red 1391 +# 1391 -> 1396: five arms in projection_scan_cost over the sort-key price (#1126) -- +# the arm for a clause that mentions the sort key it cannot prune on, two controls +# for the silent direction (a plain range and an IN-list must KEEP their discount), +# and two premises, one of which asserts the fixture clears the one-stripe floor. +# Seeded from one run per major, all five merged in a SINGLE call, so every row +# carries 15;16;17;18;19 and the short-major warning (#1071) stayed silent. +# RESEATED onto main carrying #1124. Main states 1391, this branch stated 1382, and +# the merged tree is neither. The ledger auto-merged SILENTLY while this file +# conflicted loudly -- the usual pairing -- so the union was checked by KEY: +# 0 keys lost from either side, 0 duplicates, 1407 rows. Re-derived by COUNTING on +# the merged tree, never by adding five: +# awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l +# +# COUNT THE WHOLE FILE. check_ledger.tsv has NO header row -- line 1 is a data row +# (differential/differential/agg avg) -- so a `tail -n +2` habit drops it and reports +# one short. The census command above was never affected because it skips nothing, +# but a row count taken the other way disagrees with the tool's own `ledger: rows=` +# and reads as an off-by-one in the merge rather than in the command. +checks_never_observed_red 1396 diff --git a/test/projection_scan_cost.sh b/test/projection_scan_cost.sh index 1d9fe75e..2320acb3 100755 --- a/test/projection_scan_cost.sh +++ b/test/projection_scan_cost.sh @@ -148,4 +148,64 @@ check "a non-sort-key restriction does not cheapen a covering projection" \ "$(awk -v r="$m_ratio" "BEGIN{ print (r+0 >= 0.8) ? \"not-cheap\" : \"cheap\" }")" \ "not-cheap" +# ---- a clause that MENTIONS the sort key but cannot prune on it (#1126) ------- +# +# Eligibility and pricing both ask "does this clause reference sortKey[0]". +# Mentioning is not prunability. A single RestrictInfo that ORs a sort-key range +# with a predicate on another column mentions the key, is counted whole, and +# contributes a selectivity the sort order cannot deliver. +# +# THE SAME prsk FIXTURE, because the question is about the clause and not the +# data: `kind = 'odd'` holds where sk % 1000 = 0, so its rows are spread across +# the whole sk domain and no sort order on sk brings them together. +# +# THE RANGE MUST CLEAR THE ONE-STRIPE FLOOR or the arm is vacuous. At +# stripe_row_limit => 1000 over 20000 rows there are 20 stripes, so the floor is +# 0.05; a range of 100 rows prices at the floor whether the arithmetic is right +# or wrong, and a broken guard and a working one are indistinguishable. 2000 rows +# is 10%, well clear of it. The first version of this arm used 100 and passed +# against the defect. +OR_HI=2000 +SQL_ORQ="SELECT sk FROM prsk WHERE sk BETWEEN 1 AND $OR_HI OR kind = 'odd'" +SQL_PRUNE="SELECT sk FROM prsk WHERE sk BETWEEN 1 AND $OR_HI" +SQL_SAOP="SELECT sk FROM prsk WHERE sk = ANY (ARRAY[1,2,3,4,5,6,7,8,9,10])" + +or_run="$(run_of "$(explain_scan on "$SQL_ORQ")")" +or_base="$(run_of "$(explain_scan off "$SQL_ORQ")")" +pr_run="$(run_of "$(explain_scan on "$SQL_PRUNE")")" +pr_base="$(run_of "$(explain_scan off "$SQL_PRUNE")")" +sa_run="$(run_of "$(explain_scan on "$SQL_SAOP")")" +sa_base="$(run_of "$(explain_scan off "$SQL_SAOP")")" +or_ratio="$(awk -v p="$or_run" -v b="$or_base" "BEGIN{ if (b<=0) print 0; else printf \"%.3f\", p/b }")" +pr_ratio="$(awk -v p="$pr_run" -v b="$pr_base" "BEGIN{ if (b<=0) print 0; else printf \"%.3f\", p/b }")" +sa_ratio="$(awk -v p="$sa_run" -v b="$sa_base" "BEGIN{ if (b<=0) print 0; else printf \"%.3f\", p/b }")" +echo "-- unprunable OR ratio=$or_ratio prunable range ratio=$pr_ratio IN-list ratio=$sa_ratio" + +check "premise: every unprunable-clause scan has a positive run cost" \ + "$(awk -v a="$or_run" -v b="$or_base" "BEGIN{ print (a>0 && b>0) ? \"yes\" : \"no\" }")" "yes" + +# PREMISE THAT THE FIXTURE CLEARS THE FLOOR. Without it a pass says nothing: +# at the floor every arm below reads 0.05 and agrees for the wrong reason. +check "premise: the prunable range is priced above the one-stripe floor, so the arms differ" \ + "$(awk -v r="$pr_ratio" "BEGIN{ print (r+0 > 0.051) ? \"above\" : \"at-floor\" }")" "above" + +# THE ARM. The OR rules out nothing by sort order, so it must not be discounted. +check "a clause that mentions the sort key but cannot prune on it does not cheapen a covering projection" \ + "$(awk -v r="$or_ratio" "BEGIN{ print (r+0 >= 0.8) ? \"not-cheap\" : \"cheap\" }")" \ + "not-cheap" + +# THE OTHER DIRECTION, which is the silent one. Declining a projection that would +# have won costs a query plan and reddens nothing, so both controls ship with the +# arm rather than after it. +check "while a plain range on the sort key still earns its discount" \ + "$(awk -v r="$pr_ratio" "BEGIN{ print (r+0 < 0.8) ? \"cheap\" : \"not-cheap\" }")" \ + "cheap" + +# AN IN-LIST PRUNES HONESTLY and must keep its discount. `exact` is false for a +# ScalarArrayOpExpr range key, so a fix that gated on exactness rather than on +# producing a key at all would decline this one. +check "and an IN-list on the sort key keeps its discount, which gating on exactness would lose" \ + "$(awk -v r="$sa_ratio" "BEGIN{ print (r+0 < 0.8) ? \"cheap\" : \"not-cheap\" }")" \ + "cheap" + pgc_summary diff --git a/test/pytest/test_projection_scan_cost.py b/test/pytest/test_projection_scan_cost.py index 7f7c8b12..f2afc5f9 100644 --- a/test/pytest/test_projection_scan_cost.py +++ b/test/pytest/test_projection_scan_cost.py @@ -189,3 +189,77 @@ def test_projection_scan_cost(pgc_conn, expect): "not-cheap", "a non-sort-key restriction does not cheapen a covering projection", ) + + # ---- mentioning the sort key is not pruning on it (#1126) ---------------- + # + # The arm above covers a clause on ANOTHER column. This one covers a clause + # that references the sort key and still cannot prune on it: one RestrictInfo + # ORing a sort-key range with a predicate on another column. A membership test + # counts it whole; a scan-key test does not, because a BoolExpr never becomes + # a key. + # + # THE RANGE MUST CLEAR THE ONE-STRIPE FLOOR. 30000 rows at stripe_row_limit + # 1500 is 20 stripes, so the floor is 0.05 and a small range prices there + # whether the arithmetic is right or wrong. 3000 rows is 10%. The shell twin's + # first version of this arm used a range below the floor and passed against the + # defect; the premise below is what stops that recurring here. + # + # Same psmis fixture, because the question is about the CLAUSE and not the + # data: flag = 'x' holds where ikey % 1500 = 0, so its rows sit in every stripe + # and no sort order on ikey gathers them. + or_hi = 3000 + sql_or = f"SELECT ikey FROM psmis WHERE ikey BETWEEN 1 AND {or_hi} OR flag = 'x'" + sql_prune = f"SELECT ikey FROM psmis WHERE ikey BETWEEN 1 AND {or_hi}" + sql_saop = "SELECT ikey FROM psmis WHERE ikey = ANY (ARRAY[1,2,3,4,5,6,7,8,9,10])" + + def _run_ratio(sql): + pj = _custom_scan(_plan(pgc_conn, sql, True)) + bs = _custom_scan(_plan(pgc_conn, sql, False)) + pr = pj["Total Cost"] - pj["Startup Cost"] + br = bs["Total Cost"] - bs["Startup Cost"] + return pr, br, (pr / br if br > 0 else 0.0) + + or_run, or_base, or_ratio = _run_ratio(sql_or) + pr_run, pr_base, pr_ratio = _run_ratio(sql_prune) + sa_run, sa_base, sa_ratio = _run_ratio(sql_saop) + print(f"-- unprunable OR ratio={or_ratio:.3f} " + f"prunable range ratio={pr_ratio:.3f} IN-list ratio={sa_ratio:.3f}") + + expect.text( + "yes" if min(or_run, or_base, pr_run, pr_base, sa_run, sa_base) > 0 else "no", + "yes", + "premise: every unprunable-clause scan has a positive run cost", + ) + + # Without this the three arms below can all agree at the floor, which is + # agreement for a reason unrelated to what they assert. + expect.text( + "above" if pr_ratio > 0.051 else "at-floor", + "above", + "premise: the prunable range is priced above the one-stripe floor, " + "so the arms differ", + ) + + expect.text( + "not-cheap" if or_ratio >= 0.8 else "cheap", + "not-cheap", + "a clause that mentions the sort key but cannot prune on it does not " + "cheapen a covering projection", + ) + + # The silent direction. Declining a projection that would have won costs a + # plan and reddens nothing, so both controls ship with the arm. + expect.text( + "cheap" if pr_ratio < 0.8 else "not-cheap", + "cheap", + "while a plain range on the sort key still earns its discount", + ) + + # An IN-list range key is conservative, so `exact` is false for it. A fix + # gating on exactness rather than on producing a key would decline this. + expect.text( + "cheap" if sa_ratio < 0.8 else "not-cheap", + "cheap", + "and an IN-list on the sort key keeps its discount, which gating on " + "exactness would lose", + )