From f5a4e1129ac42026a9fb4992e0f21823062e89d9 Mon Sep 17 00:00:00 2001 From: "tender.wang" Date: Tue, 5 Mar 2024 19:20:13 +0800 Subject: [PATCH v3] Fix CREATE INDEX failed due to parallel unsafe function. Currently, the expression passed to is_parallel_safe() is created from RelationGetIndexExpressions(). In RelationGetIndexExpressions(), it will flatten the raw index expression if possible. It will ok in most cases, because index expression usually only include column reference. If index expressions include function, which is defined with PARALLEL UNSAFE label. And the UDF is so simple that RelationGetIndexExpressions() will flatten it. It will report error if building index in parallel mode. For example, the UDF contain EXCEPTION statement. The solution is to pass the raw index expression to is_parallel_safe(). The raw expression should get from syscache not relcache. --- src/backend/optimizer/plan/planner.c | 4 +-- src/backend/utils/cache/relcache.c | 43 +++++++++++++++++++++++ src/include/utils/relcache.h | 1 + src/test/regress/expected/btree_index.out | 17 +++++++++ src/test/regress/sql/btree_index.sql | 21 +++++++++++ 5 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index ac97575453..2ab06f7573 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -6669,8 +6669,8 @@ plan_create_index_workers(Oid tableOid, Oid indexOid) * safe. */ if (heap->rd_rel->relpersistence == RELPERSISTENCE_TEMP || - !is_parallel_safe(root, (Node *) RelationGetIndexExpressions(index)) || - !is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index))) + !is_parallel_safe(root, (Node *) RelationGetIndexClause(index, Anum_pg_index_indexprs)) || + !is_parallel_safe(root, (Node *) RelationGetIndexClause(index, Anum_pg_index_indpred))) { parallel_workers = 0; goto done; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 8cb88454c1..23a47e247a 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5057,6 +5057,49 @@ RelationGetIndexExpressions(Relation relation) return result; } +/* + * get_raw_index_clause + * Return the raw index expression or predicate + * + * The att_num should be Anum_pg_index_indexprs or Anum_pg_index_indpred. + */ +List * +RelationGetIndexClause(Relation relation, int attnum) +{ + List *result; + HeapTuple tuple; + Datum datum; + bool isnull; + char *attString; + + Assert(attnum == Anum_pg_index_indexprs || + attnum == Anum_pg_index_indpred); + + tuple = SearchSysCache1(INDEXRELID, + ObjectIdGetDatum(RelationGetRelid(relation))); + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for index %u", + RelationGetRelid(relation)); + + /* Extract raw node tree(s) from index tuple. */ + datum = SysCacheGetAttr(INDEXRELID, tuple, attnum, &isnull); + + if (isnull) + { + ReleaseSysCache(tuple); + return NIL; + } + + attString = TextDatumGetCString(datum); + result = (List *) stringToNode(attString); + pfree(attString); + + ReleaseSysCache(tuple); + + return result; +} + /* * RelationGetDummyIndexExpressions -- get dummy expressions for an index * diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 18c32ea700..83dfdc0dda 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -52,6 +52,7 @@ extern List *RelationGetIndexExpressions(Relation relation); extern List *RelationGetDummyIndexExpressions(Relation relation); extern List *RelationGetIndexPredicate(Relation relation); extern bytea **RelationGetIndexAttOptions(Relation relation, bool copy); +extern List *RelationGetIndexClause(Relation relation, int attnum); /* * Which set of columns to return by RelationGetIndexAttrBitmap. diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out index 8311a03c3d..29a2903d80 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -434,3 +434,20 @@ ALTER INDEX btree_part_idx ALTER COLUMN id SET (n_distinct=100); ERROR: ALTER action ALTER COLUMN ... SET cannot be performed on relation "btree_part_idx" DETAIL: This operation is not supported for partitioned indexes. DROP TABLE btree_part; +-- Test index expression includes parallel unsafe function +CREATE FUNCTION para_unsafe_f() RETURNS int IMMUTABLE PARALLEL UNSAFE +AS $$ +BEGIN + RETURN 0; +EXCEPTION WHEN OTHERS THEN + RETURN 1; +END$$ LANGUAGE plpgsql; +CREATE TABLE btree_para_bld(i int); +ALTER TABLE btree_para_bld SET (parallel_workers = 4); +SET max_parallel_maintenance_workers TO 4; +CREATE INDEX ON btree_para_bld((i + para_unsafe_f())); +-- Test index predicate includes parallel unsafe function +CREATE INDEX ON btree_para_bld(i) WHERE i > para_unsafe_f(); +RESET max_parallel_maintenance_workers; +DROP TABLE btree_para_bld; +DROP FUNCTION para_unsafe_f; diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql index ef84354234..a054d106fb 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -267,3 +267,24 @@ CREATE TABLE btree_part (id int4) PARTITION BY RANGE (id); CREATE INDEX btree_part_idx ON btree_part(id); ALTER INDEX btree_part_idx ALTER COLUMN id SET (n_distinct=100); DROP TABLE btree_part; + +-- Test index expression includes parallel unsafe function +CREATE FUNCTION para_unsafe_f() RETURNS int IMMUTABLE PARALLEL UNSAFE +AS $$ +BEGIN + RETURN 0; +EXCEPTION WHEN OTHERS THEN + RETURN 1; +END$$ LANGUAGE plpgsql; + +CREATE TABLE btree_para_bld(i int); +ALTER TABLE btree_para_bld SET (parallel_workers = 4); +SET max_parallel_maintenance_workers TO 4; +CREATE INDEX ON btree_para_bld((i + para_unsafe_f())); + +-- Test index predicate includes parallel unsafe function +CREATE INDEX ON btree_para_bld(i) WHERE i > para_unsafe_f(); + +RESET max_parallel_maintenance_workers; +DROP TABLE btree_para_bld; +DROP FUNCTION para_unsafe_f; -- 2.25.1