From fd4ccf6d87c66b35e87c68a6e988bbc6ec46350a Mon Sep 17 00:00:00 2001 From: Jianghua Yang Date: Fri, 4 Sep 2026 06:48:14 +0800 Subject: [PATCH 1/3] fix(orca): never build index conditions on INCLUDE-only index columns Since 37224a3d171 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 --- .../gporca/libgpdbcost/src/CCostModelGPDB.cpp | 8 +- .../include/gpopt/xforms/CXformUtils.h | 37 +++++-- .../libgpopt/src/xforms/CXformUtils.cpp | 28 ++++-- .../regress/expected/gp_covering_index.out | 98 +++++++++++++++++++ .../expected/gp_covering_index_optimizer.out | 98 +++++++++++++++++++ src/test/regress/sql/gp_covering_index.sql | 36 +++++++ 6 files changed, 284 insertions(+), 21 deletions(-) diff --git a/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp b/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp index f4dad6c59df..b42b75aa65c 100644 --- a/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp +++ b/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp @@ -1824,8 +1824,12 @@ CCostModelGPDB::GetCommonIndexData(T *ptr, ULONG &ulIndexKeys, const IMDIndex *pmdindex = md_accessor->RetrieveIndex(ptr->Pindexdesc()->MDId()); - pdrgpcrIndexColumns = CXformUtils::PdrgpcrIndexKeys( - mp, ptr->PdrgpcrOutput(), pmdindex, pmdrel); + // Costing has always weighed every column stored in the index (keys and + // INCLUDE columns alike) in ComputeUnusedIndexWeight; keep that behavior + // here so plan costs are unaffected by PdrgpcrIndexKeys becoming key-only. + pdrgpcrIndexColumns = CXformUtils::PdrgpcrIndexColumns( + mp, ptr->PdrgpcrOutput(), pmdindex, pmdrel, + CXformUtils::EicKeyAndIncluded); stats = ptr->PstatsBaseTable(); } diff --git a/src/backend/gporca/libgpopt/include/gpopt/xforms/CXformUtils.h b/src/backend/gporca/libgpopt/include/gpopt/xforms/CXformUtils.h index 6494cfc992a..92f41a5530c 100644 --- a/src/backend/gporca/libgpopt/include/gpopt/xforms/CXformUtils.h +++ b/src/backend/gporca/libgpopt/include/gpopt/xforms/CXformUtils.h @@ -68,6 +68,19 @@ using CExpressionArrays = CDynamicPtrArray; //--------------------------------------------------------------------------- class CXformUtils { +public: + // which columns of an index to consider + enum EIndexCols + { + // KEY columns only: these are the only columns an index condition + // (index qual) may be built on + EicKey, + + // KEY columns followed by INCLUDE ("payload") columns: everything + // physically stored in the index + EicKeyAndIncluded + }; + private: // create a logical assert for the not nullable columns of the given table // on top of the given child expression @@ -83,11 +96,12 @@ class CXformUtils CColRefArray *colref_array); // return the set of columns from the given array of columns which appear - // in the index included / key columns + // in the index columns of the specified type (key only, or key + included) static CColRefSet *PcrsIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, - const IMDRelation *pmdrel); + const IMDRelation *pmdrel, + EIndexCols eic); // return the set of columns from the given array of columns which are // returnable through the index (to determine index-only scan capable) @@ -96,13 +110,6 @@ class CXformUtils const IMDIndex *pmdindex, const IMDRelation *pmdrel); - // return the ordered array of columns from the given array of columns which appear - // in the index included / key columns - static CColRefArray *PdrgpcrIndexColumns(CMemoryPool *mp, - CColRefArray *colref_array, - const IMDIndex *pmdindex, - const IMDRelation *pmdrel); - // lookup join keys in scalar child group static void LookupJoinKeys(CMemoryPool *mp, CExpression *pexpr, CExpressionArray **ppdrgpexprOuter, @@ -369,8 +376,18 @@ class CXformUtils static CWStringConst *PstrErrorMessage(CMemoryPool *mp, ULONG major, ULONG minor, ...); + // return the ordered array of columns from the given array of columns which + // appear in the index columns of the specified type (key only, or + // key + included) + static CColRefArray *PdrgpcrIndexColumns(CMemoryPool *mp, + CColRefArray *colref_array, + const IMDIndex *pmdindex, + const IMDRelation *pmdrel, + EIndexCols eic); + // return the array of key columns from the given array of columns which appear - // in the index key columns + // in the index key columns (INCLUDE columns are deliberately excluded: + // a predicate on them must never become an index condition) static CColRefArray *PdrgpcrIndexKeys(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, diff --git a/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp b/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp index b810b058f4d..3f8b8005586 100644 --- a/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp +++ b/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp @@ -1784,7 +1784,9 @@ CXformUtils::PstrErrorMessage(CMemoryPool *mp, ULONG major, ULONG minor, ...) // // @doc: // Return the array of columns from the given array of columns which appear -// in the index key columns +// in the index key columns. INCLUDE columns are not keys: the executor +// only accepts index quals on key attributes (see ExecIndexBuildScanKeys), +// so callers building index conditions must never see them here. // //--------------------------------------------------------------------------- CColRefArray * @@ -1792,7 +1794,7 @@ CXformUtils::PdrgpcrIndexKeys(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, const IMDRelation *pmdrel) { - return PdrgpcrIndexColumns(mp, colref_array, pmdindex, pmdrel); + return PdrgpcrIndexColumns(mp, colref_array, pmdindex, pmdrel, EicKey); } //--------------------------------------------------------------------------- @@ -1808,7 +1810,7 @@ CColRefSet * CXformUtils::PcrsIndexKeys(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, const IMDRelation *pmdrel) { - return PcrsIndexColumns(mp, colref_array, pmdindex, pmdrel); + return PcrsIndexColumns(mp, colref_array, pmdindex, pmdrel, EicKey); } //--------------------------------------------------------------------------- @@ -1817,16 +1819,16 @@ CXformUtils::PcrsIndexKeys(CMemoryPool *mp, CColRefArray *colref_array, // // @doc: // Return the set of columns from the given array of columns which appear -// in the index columns of the specified type (included / key) +// in the index columns of the specified type (key only / key + included) // //--------------------------------------------------------------------------- CColRefSet * CXformUtils::PcrsIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, - const IMDRelation *pmdrel) + const IMDRelation *pmdrel, EIndexCols eic) { CColRefArray *pdrgpcrIndexColumns = - PdrgpcrIndexColumns(mp, colref_array, pmdindex, pmdrel); + PdrgpcrIndexColumns(mp, colref_array, pmdindex, pmdrel, eic); CColRefSet *pcrsCols = GPOS_NEW(mp) CColRefSet(mp, pdrgpcrIndexColumns); pdrgpcrIndexColumns->Release(); @@ -1836,7 +1838,7 @@ CXformUtils::PcrsIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, //--------------------------------------------------------------------------- // @function: -// CXformUtils::PdrgpcrIndexColumns +// CXformUtils::PcrsIndexReturnableColumns // // @doc: // Return the set of columns from the given array of columns which are @@ -1872,14 +1874,17 @@ CXformUtils::PcrsIndexReturnableColumns(CMemoryPool *mp, // // @doc: // Return the ordered list of columns from the given array of columns which -// appear in the index columns of the specified type (included / key) +// appear in the index columns of the specified type: key columns only +// (EicKey), or key columns followed by INCLUDE columns (EicKeyAndIncluded) // //--------------------------------------------------------------------------- CColRefArray * CXformUtils::PdrgpcrIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, const IMDIndex *pmdindex, - const IMDRelation *pmdrel) + const IMDRelation *pmdrel, EIndexCols eic) { + GPOS_ASSERT(EicKey == eic || EicKeyAndIncluded == eic); + CColRefArray *pdrgpcrIndex = GPOS_NEW(mp) CColRefArray(mp); // key columns @@ -1894,6 +1899,11 @@ CXformUtils::PdrgpcrIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, pdrgpcrIndex->Append(colref); } + if (EicKey == eic) + { + return pdrgpcrIndex; + } + // included columns for (ULONG ul = 0; ul < pmdindex->IncludedCols(); ul++) { diff --git a/src/test/regress/expected/gp_covering_index.out b/src/test/regress/expected/gp_covering_index.out index e67cd8d7ad1..b904d0db43c 100644 --- a/src/test/regress/expected/gp_covering_index.out +++ b/src/test/regress/expected/gp_covering_index.out @@ -837,5 +837,103 @@ SELECT b FROM test_combined_index_scan WHERE a < 42 OR b < 42; Optimizer: Postgres query optimizer (6 rows) +-- Test predicates on INCLUDE-only columns (https://github.com/apache/cloudberry/issues/1948) +-- +-- A predicate that only references INCLUDE ("payload") columns must never be +-- turned into an index condition: the executor rejects index quals on non-key +-- attributes ("bogus index qualification"). Boolean column references and +-- IS [NOT] NULL tests used to slip through ORCA's index predicate extraction, +-- so cover them explicitly. +CREATE TABLE test_include_col_pred(c0 boolean, c1 boolean, c2 int) DISTRIBUTED BY (c2); +CREATE INDEX i_test_include_col_pred ON test_include_col_pred(c0) INCLUDE (c1); +INSERT INTO test_include_col_pred VALUES (true, true, 1), (false, true, 2), (true, false, 3), (true, NULL, 4); +VACUUM ANALYZE test_include_col_pred; +-- KEYS: [c0] INCLUDED: [c1] +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: c1 + Optimizer: Postgres query optimizer +(4 rows) + +SELECT * FROM test_include_col_pred WHERE c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | t | 1 + f | t | 2 +(2 rows) + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE NOT c1; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: (NOT c1) + Optimizer: Postgres query optimizer +(4 rows) + +SELECT * FROM test_include_col_pred WHERE NOT c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | f | 3 +(1 row) + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1 IS NULL; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: (c1 IS NULL) + Optimizer: Postgres query optimizer +(4 rows) + +SELECT * FROM test_include_col_pred WHERE c1 IS NULL ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | | 4 +(1 row) + +EXPLAIN (COSTS OFF) +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL; + QUERY PLAN +------------------------------------------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Index Only Scan using i_test_include_col_pred on test_include_col_pred + Filter: (c1 IS NOT NULL) + Optimizer: Postgres query optimizer +(4 rows) + +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL ORDER BY 1; + c1 +---- + f + t + t +(3 rows) + +-- key predicate combined with an INCLUDE-column predicate: only the key +-- predicate may become an index condition, the rest must stay a filter +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c0 AND c1; + QUERY PLAN +------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) + -> Index Scan using i_test_include_col_pred on test_include_col_pred + Index Cond: (c0 = true) + Filter: c1 + Optimizer: Postgres query optimizer +(5 rows) + +SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | t | 1 +(1 row) + reset optimizer_trace_fallback; reset enable_seqscan; diff --git a/src/test/regress/expected/gp_covering_index_optimizer.out b/src/test/regress/expected/gp_covering_index_optimizer.out index d709e659256..a543af2d140 100644 --- a/src/test/regress/expected/gp_covering_index_optimizer.out +++ b/src/test/regress/expected/gp_covering_index_optimizer.out @@ -795,5 +795,103 @@ SELECT b FROM test_combined_index_scan WHERE a < 42 OR b < 42; Optimizer: Pivotal Optimizer (GPORCA) (5 rows) +-- Test predicates on INCLUDE-only columns (https://github.com/apache/cloudberry/issues/1948) +-- +-- A predicate that only references INCLUDE ("payload") columns must never be +-- turned into an index condition: the executor rejects index quals on non-key +-- attributes ("bogus index qualification"). Boolean column references and +-- IS [NOT] NULL tests used to slip through ORCA's index predicate extraction, +-- so cover them explicitly. +CREATE TABLE test_include_col_pred(c0 boolean, c1 boolean, c2 int) DISTRIBUTED BY (c2); +CREATE INDEX i_test_include_col_pred ON test_include_col_pred(c0) INCLUDE (c1); +INSERT INTO test_include_col_pred VALUES (true, true, 1), (false, true, 2), (true, false, 3), (true, NULL, 4); +VACUUM ANALYZE test_include_col_pred; +-- KEYS: [c0] INCLUDED: [c1] +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: c1 + Optimizer: GPORCA +(4 rows) + +SELECT * FROM test_include_col_pred WHERE c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | t | 1 + f | t | 2 +(2 rows) + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE NOT c1; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: (NOT c1) + Optimizer: GPORCA +(4 rows) + +SELECT * FROM test_include_col_pred WHERE NOT c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | f | 3 +(1 row) + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1 IS NULL; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: (c1 IS NULL) + Optimizer: GPORCA +(4 rows) + +SELECT * FROM test_include_col_pred WHERE c1 IS NULL ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | | 4 +(1 row) + +EXPLAIN (COSTS OFF) +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL; + QUERY PLAN +------------------------------------------ + Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on test_include_col_pred + Filter: (NOT (c1 IS NULL)) + Optimizer: GPORCA +(4 rows) + +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL ORDER BY 1; + c1 +---- + f + t + t +(3 rows) + +-- key predicate combined with an INCLUDE-column predicate: only the key +-- predicate may become an index condition, the rest must stay a filter +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c0 AND c1; + QUERY PLAN +------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) + -> Index Scan using i_test_include_col_pred on test_include_col_pred + Index Cond: (c0 = true) + Filter: c1 + Optimizer: GPORCA +(5 rows) + +SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; + c0 | c1 | c2 +----+----+---- + t | t | 1 +(1 row) + reset optimizer_trace_fallback; reset enable_seqscan; diff --git a/src/test/regress/sql/gp_covering_index.sql b/src/test/regress/sql/gp_covering_index.sql index defdbb2a12d..5585b4cc60d 100644 --- a/src/test/regress/sql/gp_covering_index.sql +++ b/src/test/regress/sql/gp_covering_index.sql @@ -443,5 +443,41 @@ EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT b FROM test_combined_index_scan WHERE a < 42 OR b < 42; +-- Test predicates on INCLUDE-only columns (https://github.com/apache/cloudberry/issues/1948) +-- +-- A predicate that only references INCLUDE ("payload") columns must never be +-- turned into an index condition: the executor rejects index quals on non-key +-- attributes ("bogus index qualification"). Boolean column references and +-- IS [NOT] NULL tests used to slip through ORCA's index predicate extraction, +-- so cover them explicitly. +CREATE TABLE test_include_col_pred(c0 boolean, c1 boolean, c2 int) DISTRIBUTED BY (c2); +CREATE INDEX i_test_include_col_pred ON test_include_col_pred(c0) INCLUDE (c1); +INSERT INTO test_include_col_pred VALUES (true, true, 1), (false, true, 2), (true, false, 3), (true, NULL, 4); +VACUUM ANALYZE test_include_col_pred; + +-- KEYS: [c0] INCLUDED: [c1] +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1; +SELECT * FROM test_include_col_pred WHERE c1 ORDER BY c2; + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE NOT c1; +SELECT * FROM test_include_col_pred WHERE NOT c1 ORDER BY c2; + +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c1 IS NULL; +SELECT * FROM test_include_col_pred WHERE c1 IS NULL ORDER BY c2; + +EXPLAIN (COSTS OFF) +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL; +SELECT c1 FROM test_include_col_pred WHERE c1 IS NOT NULL ORDER BY 1; + +-- key predicate combined with an INCLUDE-column predicate: only the key +-- predicate may become an index condition, the rest must stay a filter +EXPLAIN (COSTS OFF) +SELECT * FROM test_include_col_pred WHERE c0 AND c1; +SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; + + reset optimizer_trace_fallback; reset enable_seqscan; From 9f3586134434fb18dde7e5fdc7e340f5e4013afc Mon Sep 17 00:00:00 2001 From: Jianghua Yang Date: Fri, 4 Sep 2026 10:03:18 +0800 Subject: [PATCH 2/3] test: pin index-only scan choice in memoize expr_key case for ORCA 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. --- src/test/regress/expected/memoize.out | 5 +++++ src/test/regress/expected/memoize_optimizer.out | 5 +++++ src/test/regress/sql/memoize.sql | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index 96e6737be27..afebea20104 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -118,6 +118,10 @@ SELECT d1::numeric, d1::text FROM ( INSERT INTO expr_key SELECT * FROM expr_key; CREATE INDEX expr_key_idx_x_t ON expr_key (x, t); VACUUM ANALYZE expr_key; +-- ORCA may pick either an Index Scan or an Index Only Scan for the inner +-- side depending on how much of expr_key the VACUUM above could mark +-- all-visible, which varies with concurrently running tests. Pin the plan. +SET optimizer_enable_indexonlyscan TO off; -- Ensure we get we get a cache miss and hit for each of the 20 distinct values SELECT explain_memoize(' SELECT * FROM expr_key t1 INNER JOIN expr_key t2 @@ -141,6 +145,7 @@ ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false); Optimizer: Postgres query optimizer (15 rows) +RESET optimizer_enable_indexonlyscan; DROP TABLE expr_key; -- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions SET work_mem TO '64kB'; diff --git a/src/test/regress/expected/memoize_optimizer.out b/src/test/regress/expected/memoize_optimizer.out index 08ac4f997ef..6dd4bc6ce5c 100644 --- a/src/test/regress/expected/memoize_optimizer.out +++ b/src/test/regress/expected/memoize_optimizer.out @@ -115,6 +115,10 @@ SELECT d1::numeric, d1::text FROM ( INSERT INTO expr_key SELECT * FROM expr_key; CREATE INDEX expr_key_idx_x_t ON expr_key (x, t); VACUUM ANALYZE expr_key; +-- ORCA may pick either an Index Scan or an Index Only Scan for the inner +-- side depending on how much of expr_key the VACUUM above could mark +-- all-visible, which varies with concurrently running tests. Pin the plan. +SET optimizer_enable_indexonlyscan TO off; -- Ensure we get we get a cache miss and hit for each of the 20 distinct values SELECT explain_memoize(' SELECT * FROM expr_key t1 INNER JOIN expr_key t2 @@ -133,6 +137,7 @@ ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false); Optimizer: GPORCA (10 rows) +RESET optimizer_enable_indexonlyscan; DROP TABLE expr_key; -- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions SET work_mem TO '64kB'; diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql index 683889d8814..2829a7b52f4 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -79,11 +79,16 @@ INSERT INTO expr_key SELECT * FROM expr_key; CREATE INDEX expr_key_idx_x_t ON expr_key (x, t); VACUUM ANALYZE expr_key; +-- ORCA may pick either an Index Scan or an Index Only Scan for the inner +-- side depending on how much of expr_key the VACUUM above could mark +-- all-visible, which varies with concurrently running tests. Pin the plan. +SET optimizer_enable_indexonlyscan TO off; -- Ensure we get we get a cache miss and hit for each of the 20 distinct values SELECT explain_memoize(' SELECT * FROM expr_key t1 INNER JOIN expr_key t2 ON t1.x = t2.t::numeric AND t1.t::numeric = t2.x;', false); +RESET optimizer_enable_indexonlyscan; DROP TABLE expr_key; -- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions From a6c1cc90cd92db4a00a3df882ad2c086aec58864 Mon Sep 17 00:00:00 2001 From: Jianghua Yang Date: Thu, 22 Oct 2026 03:54:13 +0800 Subject: [PATCH 3/3] fix(orca): keep INCLUDE columns in the bitmap index width comparison 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 --- .../libgpopt/src/xforms/CXformUtils.cpp | 13 +++++++-- .../regress/expected/gp_covering_index.out | 28 +++++++++++++++++++ .../expected/gp_covering_index_optimizer.out | 28 +++++++++++++++++++ src/test/regress/sql/gp_covering_index.sql | 17 +++++++++++ 4 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp b/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp index 3f8b8005586..83d8662bc51 100644 --- a/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp +++ b/src/backend/gporca/libgpopt/src/xforms/CXformUtils.cpp @@ -1842,7 +1842,7 @@ CXformUtils::PcrsIndexColumns(CMemoryPool *mp, CColRefArray *colref_array, // // @doc: // Return the set of columns from the given array of columns which are -// retunable through the index (to determine index-only scan capable) +// returnable through the index (to determine index-only scan capable) // //--------------------------------------------------------------------------- CColRefSet * @@ -3036,8 +3036,15 @@ CXformUtils::PexprBitmapSelectBestIndex( continue; } - CColRefArray *indexColumns = CXformUtils::PdrgpcrIndexKeys( - mp, pdrgpcrOutput, pmdindex, pmdrel); + // This array is used below for two things: to check the first key + // of the index, and to compare candidate indexes by width. The + // width comparison is a proxy for how big the index is on disk, so + // it must count the INCLUDE columns as well - they are stored in + // every index tuple. Key columns still come first, so looking at + // element 0 for the first key stays correct. + CColRefArray *indexColumns = CXformUtils::PdrgpcrIndexColumns( + mp, pdrgpcrOutput, pmdindex, pmdrel, + CXformUtils::EicKeyAndIncluded); // make sure the first key of index is included in the scalar predicate // (except for BRIN, which are symmetrical) diff --git a/src/test/regress/expected/gp_covering_index.out b/src/test/regress/expected/gp_covering_index.out index b904d0db43c..f3bf8903654 100644 --- a/src/test/regress/expected/gp_covering_index.out +++ b/src/test/regress/expected/gp_covering_index.out @@ -935,5 +935,33 @@ SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; t | t | 1 (1 row) +-- When two indexes have the same keys, the bitmap candidate with fewer stored +-- columns must still win: INCLUDE columns make the index wider on disk even +-- though they can never be used in an index condition. i_bmidx_wide is created +-- first on purpose, so a tie on width would hand it the plan. +CREATE TABLE test_bitmap_include_width(a int, b int, payload text) DISTRIBUTED BY (a); +CREATE INDEX i_bmidx_wide ON test_bitmap_include_width(a) INCLUDE (payload); +CREATE INDEX i_bmidx_narrow ON test_bitmap_include_width(a); +CREATE INDEX i_bmidx_b ON test_bitmap_include_width(b); +INSERT INTO test_bitmap_include_width SELECT i, i, repeat('x', 100) FROM generate_series(1, 2000) i; +VACUUM ANALYZE test_bitmap_include_width; +-- KEYS: [a] INCLUDED: [payload] +-- KEYS: [a] +-- KEYS: [b] +EXPLAIN (COSTS OFF) +SELECT * FROM test_bitmap_include_width WHERE a IN (1, 100, 1000) OR b = 17; + QUERY PLAN +--------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) + -> Bitmap Heap Scan on test_bitmap_include_width + Recheck Cond: ((a = ANY ('{1,100,1000}'::integer[])) OR (b = 17)) + -> BitmapOr + -> Bitmap Index Scan on i_bmidx_narrow + Index Cond: (a = ANY ('{1,100,1000}'::integer[])) + -> Bitmap Index Scan on i_bmidx_b + Index Cond: (b = 17) + Optimizer: Postgres query optimizer +(9 rows) + reset optimizer_trace_fallback; reset enable_seqscan; diff --git a/src/test/regress/expected/gp_covering_index_optimizer.out b/src/test/regress/expected/gp_covering_index_optimizer.out index a543af2d140..a27737cc2c3 100644 --- a/src/test/regress/expected/gp_covering_index_optimizer.out +++ b/src/test/regress/expected/gp_covering_index_optimizer.out @@ -893,5 +893,33 @@ SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; t | t | 1 (1 row) +-- When two indexes have the same keys, the bitmap candidate with fewer stored +-- columns must still win: INCLUDE columns make the index wider on disk even +-- though they can never be used in an index condition. i_bmidx_wide is created +-- first on purpose, so a tie on width would hand it the plan. +CREATE TABLE test_bitmap_include_width(a int, b int, payload text) DISTRIBUTED BY (a); +CREATE INDEX i_bmidx_wide ON test_bitmap_include_width(a) INCLUDE (payload); +CREATE INDEX i_bmidx_narrow ON test_bitmap_include_width(a); +CREATE INDEX i_bmidx_b ON test_bitmap_include_width(b); +INSERT INTO test_bitmap_include_width SELECT i, i, repeat('x', 100) FROM generate_series(1, 2000) i; +VACUUM ANALYZE test_bitmap_include_width; +-- KEYS: [a] INCLUDED: [payload] +-- KEYS: [a] +-- KEYS: [b] +EXPLAIN (COSTS OFF) +SELECT * FROM test_bitmap_include_width WHERE a IN (1, 100, 1000) OR b = 17; + QUERY PLAN +--------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) + -> Bitmap Heap Scan on test_bitmap_include_width + Recheck Cond: ((a = ANY ('{1,100,1000}'::integer[])) OR (b = 17)) + -> BitmapOr + -> Bitmap Index Scan on i_bmidx_narrow + Index Cond: (a = ANY ('{1,100,1000}'::integer[])) + -> Bitmap Index Scan on i_bmidx_b + Index Cond: (b = 17) + Optimizer: GPORCA +(9 rows) + reset optimizer_trace_fallback; reset enable_seqscan; diff --git a/src/test/regress/sql/gp_covering_index.sql b/src/test/regress/sql/gp_covering_index.sql index 5585b4cc60d..b927da0e022 100644 --- a/src/test/regress/sql/gp_covering_index.sql +++ b/src/test/regress/sql/gp_covering_index.sql @@ -478,6 +478,23 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_include_col_pred WHERE c0 AND c1; SELECT * FROM test_include_col_pred WHERE c0 AND c1 ORDER BY c2; +-- When two indexes have the same keys, the bitmap candidate with fewer stored +-- columns must still win: INCLUDE columns make the index wider on disk even +-- though they can never be used in an index condition. i_bmidx_wide is created +-- first on purpose, so a tie on width would hand it the plan. +CREATE TABLE test_bitmap_include_width(a int, b int, payload text) DISTRIBUTED BY (a); +CREATE INDEX i_bmidx_wide ON test_bitmap_include_width(a) INCLUDE (payload); +CREATE INDEX i_bmidx_narrow ON test_bitmap_include_width(a); +CREATE INDEX i_bmidx_b ON test_bitmap_include_width(b); +INSERT INTO test_bitmap_include_width SELECT i, i, repeat('x', 100) FROM generate_series(1, 2000) i; +VACUUM ANALYZE test_bitmap_include_width; + +-- KEYS: [a] INCLUDED: [payload] +-- KEYS: [a] +-- KEYS: [b] +EXPLAIN (COSTS OFF) +SELECT * FROM test_bitmap_include_width WHERE a IN (1, 100, 1000) OR b = 17; + reset optimizer_trace_fallback; reset enable_seqscan;