From 4aba1a446df1233cdc433ae44095d3a0c57d3b5b Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@openpie.com>
Date: Wed, 7 Feb 2024 19:05:18 +0800
Subject: [PATCH] Fix parallel index build failed.

We will check whether it is safe to build btree index parallelly. But the
index expression or predicate which we check is modified not raw expression
or predicate. For example, function in expression may be optimized to const.
This optimization will miss function PARALLEL SAFE option, which will make
planner choose to build index parallelly. If function includes EXCEPTION, that
will report error.

Function containing an EXCEPTION clause effectively forms a subtransaction.
Now we cannot start subtransactions during a parallel operation

For whether to parallely building index, we need the raw index expression.
The RelationGetIndexExpressions() actually returns optimized expression not
raw expression. We choose to add a bool argument to RelationGetIndexExpressions()
not to write a new function which just return index raw expression.
---
 src/backend/access/spgist/spgutils.c      |  2 +-
 src/backend/catalog/index.c               |  4 ++--
 src/backend/commands/matview.c            |  2 +-
 src/backend/commands/tablecmds.c          |  4 ++--
 src/backend/executor/execIndexing.c       |  2 +-
 src/backend/optimizer/plan/planner.c      |  4 ++--
 src/backend/optimizer/util/plancat.c      |  8 ++++----
 src/backend/parser/parse_utilcmd.c        |  4 ++--
 src/backend/utils/cache/relcache.c        | 12 ++++++++++--
 src/include/utils/relcache.h              |  4 ++--
 src/test/regress/expected/btree_index.out | 15 +++++++++++++++
 src/test/regress/sql/btree_index.sql      | 18 ++++++++++++++++++
 12 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 5b5e6e82d3..7acdc3a416 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -137,7 +137,7 @@ GetIndexInputType(Relation index, AttrNumber indexcol)
 	if (index->rd_indexprs)
 		indexprs = index->rd_indexprs;
 	else
-		indexprs = RelationGetIndexExpressions(index);
+		indexprs = RelationGetIndexExpressions(index, true);
 	indexpr_item = list_head(indexprs);
 	for (int i = 1; i <= index->rd_index->indnkeyatts; i++)
 	{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 4b88a9cb87..d49b86a7c8 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2455,8 +2455,8 @@ BuildIndexInfo(Relation index)
 	ii = makeIndexInfo(indexStruct->indnatts,
 					   indexStruct->indnkeyatts,
 					   index->rd_rel->relam,
-					   RelationGetIndexExpressions(index),
-					   RelationGetIndexPredicate(index),
+					   RelationGetIndexExpressions(index, true),
+					   RelationGetIndexPredicate(index, true),
 					   indexStruct->indisunique,
 					   indexStruct->indnullsnotdistinct,
 					   indexStruct->indisready,
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 59920ced83..847f34b021 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -898,7 +898,7 @@ is_usable_unique_index(Relation indexRel)
 		indexStruct->indimmediate &&
 		indexRel->rd_rel->relam == BTREE_AM_OID &&
 		indexStruct->indisvalid &&
-		RelationGetIndexPredicate(indexRel) == NIL &&
+		RelationGetIndexPredicate(indexRel, true) == NIL &&
 		indexStruct->indnatts > 0)
 	{
 		/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9f51696740..d9bc9b7693 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17164,13 +17164,13 @@ ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode
 				 errmsg("cannot use non-immediate index \"%s\" as replica identity",
 						RelationGetRelationName(indexRel))));
 	/* Expression indexes aren't supported. */
-	if (RelationGetIndexExpressions(indexRel) != NIL)
+	if (RelationGetIndexExpressions(indexRel, true) != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot use expression index \"%s\" as replica identity",
 						RelationGetRelationName(indexRel))));
 	/* Predicate indexes aren't supported. */
-	if (RelationGetIndexPredicate(indexRel) != NIL)
+	if (RelationGetIndexPredicate(indexRel, true) != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot use partial index \"%s\" as replica identity",
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 9f05b3654c..ad6e5ab23a 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -1045,7 +1045,7 @@ index_unchanged_by_update(ResultRelInfo *resultRelInfo, EState *estate,
 	 * If we find any matching Vars, don't pass hint for index.  Otherwise
 	 * pass hint.
 	 */
-	idxExprs = RelationGetIndexExpressions(indexRelation);
+	idxExprs = RelationGetIndexExpressions(indexRelation, true);
 	hasexpression = index_expression_changed_walker((Node *) idxExprs,
 													allUpdatedCols);
 	list_free(idxExprs);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index acc324122f..9a733e3148 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -6670,8 +6670,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 *) RelationGetIndexExpressions(index, false)) ||
+		!is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index, false)))
 	{
 		parallel_workers = 0;
 		goto done;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index b933eefa64..0a503269e1 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -434,8 +434,8 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 			 * correct varno for the parent relation, so that they match up
 			 * correctly against qual clauses.
 			 */
-			info->indexprs = RelationGetIndexExpressions(indexRelation);
-			info->indpred = RelationGetIndexPredicate(indexRelation);
+			info->indexprs = RelationGetIndexExpressions(indexRelation, true);
+			info->indpred = RelationGetIndexPredicate(indexRelation, true);
 			if (info->indexprs && varno != 1)
 				ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
 			if (info->indpred && varno != 1)
@@ -847,7 +847,7 @@ infer_arbiter_indexes(PlannerInfo *root)
 			goto next;
 
 		/* Expression attributes (if any) must match */
-		idxExprs = RelationGetIndexExpressions(idxRel);
+		idxExprs = RelationGetIndexExpressions(idxRel, true);
 		foreach(el, onconflict->arbiterElems)
 		{
 			InferenceElem *elem = (InferenceElem *) lfirst(el);
@@ -898,7 +898,7 @@ infer_arbiter_indexes(PlannerInfo *root)
 		 * If it's a partial index, its predicate must be implied by the ON
 		 * CONFLICT's WHERE clause.
 		 */
-		predExprs = RelationGetIndexPredicate(idxRel);
+		predExprs = RelationGetIndexPredicate(idxRel, true);
 
 		if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false))
 			goto next;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 56ac4f516e..928a0d0869 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2413,14 +2413,14 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 					 errdetail("Cannot create a primary key or unique constraint using such an index."),
 					 parser_errposition(cxt->pstate, constraint->location)));
 
-		if (RelationGetIndexExpressions(index_rel) != NIL)
+		if (RelationGetIndexExpressions(index_rel, true) != NIL)
 			ereport(ERROR,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("index \"%s\" contains expressions", index_name),
 					 errdetail("Cannot create a primary key or unique constraint using such an index."),
 					 parser_errposition(cxt->pstate, constraint->location)));
 
-		if (RelationGetIndexPredicate(index_rel) != NIL)
+		if (RelationGetIndexPredicate(index_rel, true) != NIL)
 			ereport(ERROR,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("\"%s\" is a partial index", index_name),
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ac106b40e3..2b5a131038 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5007,7 +5007,7 @@ RelationGetReplicaIndex(Relation relation)
  * disappear due to relcache invalidation.)
  */
 List *
-RelationGetIndexExpressions(Relation relation)
+RelationGetIndexExpressions(Relation relation, bool need_modify)
 {
 	List	   *result;
 	Datum		exprsDatum;
@@ -5038,6 +5038,10 @@ RelationGetIndexExpressions(Relation relation)
 	result = (List *) stringToNode(exprsString);
 	pfree(exprsString);
 
+	/* If we need raw index expression, return */
+	if (!need_modify)
+		return result;
+
 	/*
 	 * Run the expressions through eval_const_expressions. This is not just an
 	 * optimization, but is necessary, because the planner will be comparing
@@ -5120,7 +5124,7 @@ RelationGetDummyIndexExpressions(Relation relation)
  * disappear due to relcache invalidation.)
  */
 List *
-RelationGetIndexPredicate(Relation relation)
+RelationGetIndexPredicate(Relation relation, bool need_modify)
 {
 	List	   *result;
 	Datum		predDatum;
@@ -5151,6 +5155,10 @@ RelationGetIndexPredicate(Relation relation)
 	result = (List *) stringToNode(predString);
 	pfree(predString);
 
+	/* If we need a raw index predicate, return */
+	if (!need_modify)
+		return result;
+
 	/*
 	 * Run the expression through const-simplification and canonicalization.
 	 * This is not just an optimization, but is necessary, because the planner
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 18c32ea700..d1529f7a69 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -48,9 +48,9 @@ extern List *RelationGetIndexList(Relation relation);
 extern List *RelationGetStatExtList(Relation relation);
 extern Oid	RelationGetPrimaryKeyIndex(Relation relation);
 extern Oid	RelationGetReplicaIndex(Relation relation);
-extern List *RelationGetIndexExpressions(Relation relation);
+extern List *RelationGetIndexExpressions(Relation relation, bool need_modify);
 extern List *RelationGetDummyIndexExpressions(Relation relation);
-extern List *RelationGetIndexPredicate(Relation relation);
+extern List *RelationGetIndexPredicate(Relation relation, bool need_modify);
 extern bytea **RelationGetIndexAttOptions(Relation relation, bool copy);
 
 /*
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
index 8311a03c3d..c96c908d41 100644
--- a/src/test/regress/expected/btree_index.out
+++ b/src/test/regress/expected/btree_index.out
@@ -434,3 +434,18 @@ 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);
+INSERT INTO btree_para_bld SELECT g FROM generate_series(1, 300000) g;
+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();
+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..ac687a8ad7 100644
--- a/src/test/regress/sql/btree_index.sql
+++ b/src/test/regress/sql/btree_index.sql
@@ -267,3 +267,21 @@ 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);
+INSERT INTO btree_para_bld SELECT g FROM generate_series(1, 300000) g;
+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();
+DROP TABLE btree_para_bld;
+DROP FUNCTION para_unsafe_f;
-- 
2.25.1

