From b011c3518bb068a4c886d2898b50664f640eec5e Mon Sep 17 00:00:00 2001 From: David Kimura Date: Thu, 10 Sep 2020 17:36:42 -0700 Subject: [PATCH] Align Orca relhasindex behavior with Planner (#10788) Function `RelationGetIndexList()` does not filter out invalid indexes. That responsiblity is left to the caller (e.g. `get_relation_info()`). Issue is that Orca was not checking index validity. This commit also introduces an optimization to Orca that is already used in Planner whereby we first check relhasindex before checking pg_index. --- src/backend/gpopt/gpdbwrappers.cpp | 7 +++++-- .../translate/CTranslatorRelcacheToDXL.cpp | 1 + src/test/regress/expected/bfv_index.out | 18 ++++++++++++++++++ .../regress/expected/bfv_index_optimizer.out | 18 ++++++++++++++++++ src/test/regress/sql/bfv_index.sql | 10 ++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/backend/gpopt/gpdbwrappers.cpp b/src/backend/gpopt/gpdbwrappers.cpp index 92ba48fb04..f90ae43e17 100644 --- a/src/backend/gpopt/gpdbwrappers.cpp +++ b/src/backend/gpopt/gpdbwrappers.cpp @@ -2484,8 +2484,11 @@ gpdb::GetRelationIndexes { GP_WRAP_START; { - /* catalog tables: from relcache */ - return RelationGetIndexList(relation); + if (relation->rd_rel->relhasindex) + { + /* catalog tables: from relcache */ + return RelationGetIndexList(relation); + } } GP_WRAP_END; return NIL; diff --git a/src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp b/src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp index b6af52b2d6..11a0eb2daa 100644 --- a/src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp +++ b/src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp @@ -3301,6 +3301,7 @@ CTranslatorRelcacheToDXL::IsIndexSupported // index expressions and index constraints not supported return gpdb::HeapAttIsNull(tup, Anum_pg_index_indexprs) && gpdb::HeapAttIsNull(tup, Anum_pg_index_indpred) && + index_rel->rd_index->indisvalid && (BTREE_AM_OID == index_rel->rd_rel->relam || BITMAP_AM_OID == index_rel->rd_rel->relam || GIST_AM_OID == index_rel->rd_rel->relam || GIN_AM_OID == index_rel->rd_rel->relam); } diff --git a/src/test/regress/expected/bfv_index.out b/src/test/regress/expected/bfv_index.out index 4d127816a3..bcef52ed64 100644 --- a/src/test/regress/expected/bfv_index.out +++ b/src/test/regress/expected/bfv_index.out @@ -48,6 +48,24 @@ explain select * from bfv_tab1, (values(147, 'RFAAAA'), (931, 'VJAAAA')) as v (i Optimizer status: Postgres query optimizer (8 rows) +-- Test that we do not choose to perform an index scan if indisvalid=false. +create table bfv_tab1_with_invalid_index (like bfv_tab1 including indexes); +NOTICE: table doesn't have 'DISTRIBUTED BY' clause, defaulting to distribution columns from LIKE table +set allow_system_table_mods=on; +update pg_index set indisvalid=false where indrelid='bfv_tab1_with_invalid_index'::regclass; +reset allow_system_table_mods; +explain select * from bfv_tab1_with_invalid_index where unique1>42; + QUERY PLAN +----------------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) (cost=0.00..0.05 rows=3 width=244) + -> Seq Scan on bfv_tab1_with_invalid_index (cost=0.00..0.01 rows=1 width=244) + Filter: (unique1 > 42) + Optimizer: Postgres query optimizer +(4 rows) + +-- Cannot currently upgrade table with invalid index +-- (see https://github.com/greenplum-db/gpdb/issues/10805). +drop table bfv_tab1_with_invalid_index; reset gp_enable_relsize_collection; --start_ignore DROP TABLE IF EXISTS bfv_tab2_facttable1; diff --git a/src/test/regress/expected/bfv_index_optimizer.out b/src/test/regress/expected/bfv_index_optimizer.out index f106dbbaab..7fbd3589e9 100644 --- a/src/test/regress/expected/bfv_index_optimizer.out +++ b/src/test/regress/expected/bfv_index_optimizer.out @@ -51,6 +51,24 @@ explain select * from bfv_tab1, (values(147, 'RFAAAA'), (931, 'VJAAAA')) as v (i Optimizer: Pivotal Optimizer (GPORCA) version 2.64.0 (9 rows) +-- Test that we do not choose to perform an index scan if indisvalid=false. +create table bfv_tab1_with_invalid_index (like bfv_tab1 including indexes); +NOTICE: table doesn't have 'DISTRIBUTED BY' clause, defaulting to distribution columns from LIKE table +set allow_system_table_mods=on; +update pg_index set indisvalid=false where indrelid='bfv_tab1_with_invalid_index'::regclass; +reset allow_system_table_mods; +explain select * from bfv_tab1_with_invalid_index where unique1>42; + QUERY PLAN +----------------------------------------------------------------------------------- + Gather Motion 3:1 (slice1; segments: 3) (cost=0.00..431.00 rows=1 width=244) + -> Seq Scan on bfv_tab1_with_invalid_index (cost=0.00..431.00 rows=1 width=244) + Filter: (unique1 > 42) + Optimizer: Pivotal Optimizer (GPORCA) +(4 rows) + +-- Cannot currently upgrade table with invalid index +-- (see https://github.com/greenplum-db/gpdb/issues/10805). +drop table bfv_tab1_with_invalid_index; reset gp_enable_relsize_collection; --start_ignore DROP TABLE IF EXISTS bfv_tab2_facttable1; diff --git a/src/test/regress/sql/bfv_index.sql b/src/test/regress/sql/bfv_index.sql index 38f18f81c3..048b3ba1f5 100644 --- a/src/test/regress/sql/bfv_index.sql +++ b/src/test/regress/sql/bfv_index.sql @@ -30,6 +30,16 @@ set gp_enable_relsize_collection=on; explain select * from bfv_tab1, (values(147, 'RFAAAA'), (931, 'VJAAAA')) as v (i, j) WHERE bfv_tab1.unique1 = v.i and bfv_tab1.stringu1 = v.j; +-- Test that we do not choose to perform an index scan if indisvalid=false. +create table bfv_tab1_with_invalid_index (like bfv_tab1 including indexes); +set allow_system_table_mods=on; +update pg_index set indisvalid=false where indrelid='bfv_tab1_with_invalid_index'::regclass; +reset allow_system_table_mods; +explain select * from bfv_tab1_with_invalid_index where unique1>42; +-- Cannot currently upgrade table with invalid index +-- (see https://github.com/greenplum-db/gpdb/issues/10805). +drop table bfv_tab1_with_invalid_index; + reset gp_enable_relsize_collection; --start_ignore -- GitLab