Repository navigation
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core ORCA index predicate extraction and costing behavior and should receive a final domain-expert review despite the added regression coverage.
Pull request overview
This PR fixes an ORCA planning bug where predicates on INCLUDE-only index columns could incorrectly become index quals, leading to executor failures (e.g., “bogus index qualification”), and adds regression coverage for the affected predicate forms.
Changes:
- Reintroduce a key-only vs key+INCLUDE column selector in
CXformUtilsso index-qual construction only considers key columns. - Keep cost model behavior unchanged by explicitly requesting key+INCLUDE columns for unused-index-column weighting.
- Add regression tests (and expected outputs) covering boolean column refs and
IS [NOT] NULLpredicates on INCLUDE-only columns.
File summaries
| File | Description |
|---|---|
| src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp | Make index-key APIs key-only and add selectable index column retrieval (key vs key+INCLUDE). |
| src/backend/gporca/libgpopt/include/gpopt/xforms/CXformUtils.h | Add EIndexCols selector and update function signatures/docs accordingly. |
| src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp | Request key+INCLUDE columns for costing to preserve prior cost behavior. |
| src/test/regress/sql/gp_covering_index.sql | Add regression queries ensuring INCLUDE-only predicates do not become index conditions. |
| src/test/regress/expected/gp_covering_index.out | Update expected output for Postgres planner variant. |
| src/test/regress/expected/gp_covering_index_optimizer.out | Update expected output for ORCA variant. |
Review details
Suppressed comments (1)
src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp:1845
- The comment for PcrsIndexReturnableColumns contains a typo: "retunable" should be "returnable".
// CXformUtils::PcrsIndexReturnableColumns
//
// @doc:
// Return the set of columns from the given array of columns which are
// retunable through the index (to determine index-only scan capable)
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yjhjstz
added a commit
to quantumiodb/pgorca
that referenced
this pull request
Sep 6, 2026
A predicate on a column that appears only in an index's INCLUDE list was
turned into an index condition, and the executor rejected the plan:
create table t (k int, pay int, flag boolean);
insert into t select i, case when i % 997 = 0 then null else i % 50 end,
i % 7 = 0 from generate_series(1, 100000) i;
create index t_i on t (k) include (pay, flag);
analyze t;
set pg_orca.enable_orca = on;
explain (costs off) select * from t where k < 30 and flag;
ERROR: bogus varattno for INDEX_VAR var: 0
select * from t where k < 30 and flag;
ERROR: bogus index qualification
An IS [NOT] NULL test on an INCLUDE column fails the same way with
"btree index keys must be ordered by attribute", and both shapes reach the
executor through the plain index scan and the bitmap paths alike.
CXformUtils::PdrgpcrIndexKeys was a straight wrapper around
PdrgpcrIndexColumns, which appends pmdindex->Keys() followed by
pmdindex->IncludedCols(). Its three index-predicate call sites --
PexprBuildIndexPlan and the two bitmap plan builders -- therefore passed
INCLUDE columns to CPredicateUtils::ExtractIndexPredicates as if they were
keys. Most predicates are still filtered out there by the opfamily lookup
and FCompatiblePredicates, which is why a plain `pay = 7` never broke; the
boolean-column-reference and NULL-test shortcuts skip that validation and
put the predicate straight into the index conditions. The executor only
accepts index quals on key attributes (ExecIndexBuildScanKeys), so the
INCLUDE column's attno, which has no scan-key slot, is what blows up.
Reintroduce a key-only vs. key+INCLUDE selector on the column accessors:
PdrgpcrIndexColumns and PcrsIndexColumns take an EIndexCols, and the
PdrgpcrIndexKeys / PcrsIndexKeys wrappers pass EicKey. INCLUDE columns are
not keys, and a caller building index conditions must never see them.
CCostModelGPDB::GetCommonIndexData is the one caller that wanted the old
key+INCLUDE list: ComputeUnusedIndexWeight has always weighed every column
physically stored in the index, so it now asks for EicKeyAndIncluded
explicitly and plan costs are unaffected -- cost_align.sh is unchanged at
311 / 183 OK / 5 off / 123 diff-plan.
Index-only-scan coverage is decided by PcrsIndexReturnableColumns, which
reads pmdindex->ReturnableCols() and is untouched, so INCLUDE columns keep
serving index-only scans; the fix only stops them from carrying quals.
FIndexApplicable's disjointness check now considers key columns alone,
which is the intended reading: an index whose keys none of the predicate
columns touch offers nothing to an index condition.
Tests: test/sql/covering_index.sql asserts that only the key predicate
becomes an Index Cond while boolean, IS NULL, IS NOT NULL and equality
predicates on INCLUDE columns stay a Filter, and checks results for
INCLUDE-only predicates with enable_seqscan (and then enable_indexscan)
off, where the index is the only access path left. Reverting the source
change turns all of them into the three executor errors above.
--orca-tests 23/23; --pg-tests --ignore-plans keeps the pre-existing six
failures.
Port of apache/cloudberry#1957, fixing apache/cloudberry#1948.
1 of 2 tasks
leborchuk
approved these changes
Sep 29, 2026
leborchuk
left a comment
leborchuk
left a comment
Contributor
There was a problem hiding this comment.
LGTM, we inherited this bug from GP7. No-one fixed it since then.
Alena0704
reviewed
Oct 21, 2026
Since 37224a3 CXformUtils::PdrgpcrIndexKeys() has been a thin wrapper around PdrgpcrIndexColumns(), which appends the index's INCLUDE columns after its key columns. CPredicateUtils::ExtractIndexPredicates() therefore treated a predicate on an INCLUDE-only column as indexable. Ordinary comparisons were still rejected later by CMDIndexGPDB::IsCompatible(), but the shortcuts for boolean column references (col -> col = true, NOT col -> col = false) and for IS [NOT] NULL bypass that check and went straight into the index condition list. The DXL-to-plan translator then maps the column through GetKeyPos(), which returns ulong_max for a non-key column, so the Var's varattno wrapped to 0 and execution failed with ERROR: bogus index qualification (nodeIndexscan.c:1250) or, for IS [NOT] NULL, ERROR: btree index keys must be ordered by attribute (nbtutils.c:777) Restore the key-only semantics: reintroduce the EIndexCols selector (EicKey / EicKeyAndIncluded) on PdrgpcrIndexColumns()/PcrsIndexColumns() and make PdrgpcrIndexKeys()/PcrsIndexKeys() request key columns only. The cost model keeps asking for key + INCLUDE columns so that ComputeUnusedIndexWeight() costing is unchanged. Index-only-scan coverage checks are unaffected: they already use PcrsIndexReturnableColumns(). Add regression coverage to gp_covering_index. Fixes apache#1948
memoize_optimizer.out expects an Index Scan on expr_key_idx_x_t for the expr_key join, but ORCA chooses an Index Only Scan whenever the preceding VACUUM managed to mark expr_key all-visible. Whether it can depends on the snapshots held by tests running concurrently in the same parallel group, so the test intermittently fails with - -> Index Scan using expr_key_idx_x_t on expr_key t2 ... + -> Index Only Scan using expr_key_idx_x_t on expr_key t2 ... + Heap Fetches: N The scan type is irrelevant to what the test checks (memoize cache hits and misses on an expression key), so disable optimizer_enable_indexonlyscan around that query. The GUC only affects ORCA; the planner output merely gains the echoed SET/RESET lines.
PexprBitmapSelectBestIndex() breaks ties between equally selective bitmap index candidates by the number of index columns, as a proxy for how big the index is on disk. It got that number from PdrgpcrIndexKeys(), which the previous commit made key-only, so an index with INCLUDE columns now looked exactly as narrow as one without. Two indexes on the same key then tie, and the first one in pg_index order wins - which can be the much wider one: CREATE INDEX wide ON t(a) INCLUDE (payload); -- 12 MB CREATE INDEX narrow ON t(a); -- 720 kB SELECT * FROM t WHERE a IN (1, 100, 1000) OR b = 17; -> Bitmap Index Scan on wide -- was: narrow INCLUDE columns are stored in every index tuple, so the width comparison must count them; only index *conditions* have to stay key-only. Ask for EicKeyAndIncluded there. Key columns still come first in the array, so the adjacent first-index-key check is unaffected. Also fix a typo in the PcrsIndexReturnableColumns comment. Reported-by: Alena0704 <apache#1957>
yjhjstz
force-pushed
the
fix/orca-include-col-indexqual-1948
branch
from
October 21, 2026 19:54
2f37ded to
a6c1cc9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since 37224a3 CXformUtils::PdrgpcrIndexKeys() has been a thin wrapper around PdrgpcrIndexColumns(), which appends the index's INCLUDE columns after its key columns. CPredicateUtils::ExtractIndexPredicates() therefore treated a predicate on an INCLUDE-only column as indexable. Ordinary comparisons were still rejected later by CMDIndexGPDB::IsCompatible(), but the shortcuts for boolean column references (col -> col = true, NOT col -> col = false) and for IS [NOT] NULL bypass that check and went straight into the index condition list.
The DXL-to-plan translator then maps the column through GetKeyPos(), which returns ulong_max for a non-key column, so the Var's varattno wrapped to 0 and execution failed with
ERROR: bogus index qualification (nodeIndexscan.c:1250)
or, for IS [NOT] NULL,
ERROR: btree index keys must be ordered by attribute (nbtutils.c:777)
Restore the key-only semantics: reintroduce the EIndexCols selector (EicKey / EicKeyAndIncluded) on PdrgpcrIndexColumns()/PcrsIndexColumns() and make PdrgpcrIndexKeys()/PcrsIndexKeys() request key columns only. The cost model keeps asking for key + INCLUDE columns so that ComputeUnusedIndexWeight() costing is unchanged.
Index-only-scan coverage checks are unaffected: they already use PcrsIndexReturnableColumns().
Add regression coverage to gp_covering_index.
Fixes #1948
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions