From 2f37ded8d551122c9429d07c3622648ced8a16c5 Mon Sep 17 00:00:00 2001 From: Jianghua Yang Date: Fri, 4 Sep 2026 06:48:14 +0800 Subject: [PATCH] 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 575f6473ff0..76cc04c4a23 100644 --- a/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp +++ b/src/backend/gporca/libgpdbcost/src/CCostModelGPDB.cpp @@ -1823,8 +1823,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;