Revert "Fix parallel-safety check of expressions and predicate for index builds"

michaelpq · michaelpq · commit c46817ee512a · 2024-03-07T08:31:00.000+09:00
This reverts commit eae7be6, following a discussion with Tom Lane, due to concerns that this impacts the decisions made by the planner for the number of workers spawned based on the inlining and const-folding of index expressions and predicate for cases that would have worked until this commit. Discussion: https://postgr.es/m/162802.1709746091@sss.pgh.pa.us Backpatch-through: 12
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
@@ -6695,18 +6695,10 @@ plan_create_index_workers(Oid tableOid, Oid indexOid)
 	 * Currently, parallel workers can't access the leader's temporary tables.
 	 * Furthermore, any index predicate or index expressions must be parallel
 	 * safe.
-	 *
-	 * Fetch the list of expressions and predicates directly from the
-	 * catalogs.  Retrieving this information from the relcache would cause
-	 * the expressions and predicates to be flattened, losing properties that
-	 * can be important to check if parallel workers can be used.  For
-	 * example, immutable parallel-unsafe functions, that cannot be used in
-	 * parallel workers, would be changed to Const nodes, that are safe in
-	 * parallel workers.
 	 */
 	if (heap->rd_rel->relpersistence == RELPERSISTENCE_TEMP ||
-		!is_parallel_safe(root, (Node *) get_index_expressions(indexOid)) ||
-		!is_parallel_safe(root, (Node *) get_index_predicate(indexOid)))
+		!is_parallel_safe(root, (Node *) RelationGetIndexExpressions(index)) ||
+		!is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index)))
 	{
 		parallel_workers = 0;
 		goto done;
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
@@ -3504,74 +3504,6 @@ get_index_column_opclass(Oid index_oid, int attno)
 	return opclass;
 }
 
-/*
- * get_index_expressions
- *
- *		Given the index OID, its a List of its expressions or NIL if none.
- */
-List *
-get_index_expressions(Oid index_oid)
-{
-	List	   *result;
-	HeapTuple	tuple;
-	Datum		exprDatum;
-	bool		isnull;
-	char	   *exprString;
-
-	tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for index %u", index_oid);
-
-	exprDatum = SysCacheGetAttr(INDEXRELID, tuple,
-								Anum_pg_index_indexprs, &isnull);
-	if (isnull)
-	{
-		ReleaseSysCache(tuple);
-		return NIL;
-	}
-
-	exprString = TextDatumGetCString(exprDatum);
-	result = (List *) stringToNode(exprString);
-	pfree(exprString);
-	ReleaseSysCache(tuple);
-
-	return result;
-}
-
-/*
- * get_index_predicate
- *
- *		Given the index OID, return a List of its predicate or NIL if none.
- */
-List *
-get_index_predicate(Oid index_oid)
-{
-	List	   *result;
-	HeapTuple	tuple;
-	Datum		predDatum;
-	bool		isnull;
-	char	   *predString;
-
-	tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for index %u", index_oid);
-
-	predDatum = SysCacheGetAttr(INDEXRELID, tuple,
-								Anum_pg_index_indpred, &isnull);
-	if (isnull)
-	{
-		ReleaseSysCache(tuple);
-		return NIL;
-	}
-
-	predString = TextDatumGetCString(predDatum);
-	result = (List *) stringToNode(predString);
-	pfree(predString);
-	ReleaseSysCache(tuple);
-
-	return result;
-}
-
 /*
  * get_index_isreplident
  *
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
@@ -195,8 +195,6 @@ extern Oid	get_range_collation(Oid rangeOid);
 extern Oid	get_range_multirange(Oid rangeOid);
 extern Oid	get_multirange_range(Oid multirangeOid);
 extern Oid	get_index_column_opclass(Oid index_oid, int attno);
-extern List *get_index_expressions(Oid index_oid);
-extern List *get_index_predicate(Oid index_oid);
 extern bool get_index_isreplident(Oid index_oid);
 extern bool get_index_isvalid(Oid index_oid);
 extern bool get_index_isclustered(Oid index_oid);
diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out
@@ -387,22 +387,3 @@ 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 with index expression and predicate that include a 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;
--- With parallel-unsafe expression
-CREATE INDEX ON btree_para_bld((i + para_unsafe_f()));
--- With parallel-unsafe predicate
-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
@@ -242,25 +242,3 @@ 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 with index expression and predicate that include a 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;
--- With parallel-unsafe expression
-CREATE INDEX ON btree_para_bld((i + para_unsafe_f()));
--- With parallel-unsafe predicate
-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;
