回复：[Internet]Re: [PATCH] Prevent replacement of a function if it's used in an index expression and is not IMMUTABLE

From 0970a04a012686ddba64c4ebd38295656dbaada2 Mon Sep 17 00:00:00 2001
From: xiaojiluo <xiaojiluo@tencent.com>
Date: Wed, 25 Jun 2025 12:04:05 +0800
Subject: [PATCH v2] Prevent replacement of a function if it's used in an index
 expression and is not IMMUTABLE

In ProcedureCreate(), add a check to disallow replacing an existing function
if it is referenced by an index expression and is not marked as IMMUTABLE.
Replacing such a function could break index semantics or lead to inconsistent
behavior at runtime, especially if the function's output is not guaranteed
to be stable for the same input.

IMMUTABLE functions are assumed to always return the same output for the same
input and thus are considered safe to replace even when referenced in indexes.

This check adds a dependency scan on pg_depend to detect any usage of the
function in indexes. If the function is found to be in use and is not
IMMUTABLE, an error is thrown.

This refinement ensures safer function replacement behavior by blocking
redefinition only in cases where semantic consistency of indexed expressions
could be compromised.

In v2:
- Limit the error to the case where the original function is IMMUTABLE and
  the new definition is not IMMUTABLE.
- Use errcode FEATURE_NOT_SUPPORTED instead of DEPENDENT_OBJECTS_STILL_EXIST,
  which better reflects the nature of this restriction.
- Adjust indentation and code style according to reviewer suggestions.
- Add a regression test in create_function_sql.sql to verify the behavior.

Author: xiaojiluo <xiaojiluo@tencent.com>
---
 src/backend/catalog/pg_proc.c                 | 57 +++++++++++++++++++
 .../regress/expected/create_function_sql.out  | 17 +++++-
 src/test/regress/sql/create_function_sql.sql  | 13 +++++
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..fd831058c32 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -26,6 +26,8 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_transform.h"
 #include "catalog/pg_type.h"
+#include "catalog/pg_depend.h"
+#include "utils/fmgroids.h"
 #include "executor/functions.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -420,6 +422,61 @@ ProcedureCreate(const char *procedureName,
 					  errdetail("\"%s\" is a window function.", procedureName) :
 					  0)));
 
+		if (oldproc->prokind == PROKIND_FUNCTION &&
+			oldproc->provolatile == PROVOLATILE_IMMUTABLE &&
+			volatility != PROVOLATILE_IMMUTABLE)
+		{
+			Relation depRel = table_open(DependRelationId, AccessShareLock);
+			bool index_found = false;
+			SysScanDesc scan;
+			ScanKeyData key;
+			HeapTuple dtup;
+
+			/* refobjid = oldproc->oid */
+			ScanKeyInit(&key,
+						Anum_pg_depend_refobjid,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(oldproc->oid));
+
+			scan = systable_beginscan(depRel,
+									DependReferenceIndexId,
+									true,
+									NULL,
+									1, &key);
+
+			while (HeapTupleIsValid(dtup = systable_getnext(scan)))
+			{
+				Form_pg_depend d = (Form_pg_depend) GETSTRUCT(dtup);
+
+				if (d->classid == RelationRelationId && d->objsubid == 0)
+				{
+					/* query relkind */
+					HeapTuple reltup = SearchSysCache1(RELOID, ObjectIdGetDatum(d->objid));
+					if (HeapTupleIsValid(reltup))
+					{
+						Form_pg_class classForm = (Form_pg_class) GETSTRUCT(reltup);
+						if (classForm->relkind == RELKIND_INDEX)
+						{
+							index_found = true;
+							ReleaseSysCache(reltup);
+							break;
+						}
+						ReleaseSysCache(reltup);
+					}
+				}
+			}
+
+			systable_endscan(scan);
+			table_close(depRel, AccessShareLock);
+
+			if (index_found)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("cannot replace function \"%s\" with a non-IMMUTABLE function because it is used by an index",
+								procedureName)));
+
+		}
+
 		dropcmd = (prokind == PROKIND_PROCEDURE ? "DROP PROCEDURE" :
 				   prokind == PROKIND_AGGREGATE ? "DROP AGGREGATE" :
 				   "DROP FUNCTION");
diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out
index 963b6f863ff..6da599e22e2 100644
--- a/src/test/regress/expected/create_function_sql.out
+++ b/src/test/regress/expected/create_function_sql.out
@@ -771,9 +771,22 @@ ERROR:  return type mismatch in function declared to return integer[]
 DETAIL:  Function's final statement must be SELECT or INSERT/UPDATE/DELETE/MERGE RETURNING.
 CONTEXT:  SQL function "test1" during startup
 RESET check_function_bodies;
+-- Test: prevent replacing an IMMUTABLE function used in index with a non-IMMUTABLE one
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+IMMUTABLE LANGUAGE SQL AS $$ SELECT $1 + 1 $$;
+-- Create a table and an index using that function
+CREATE TABLE test_idx(a int);
+CREATE INDEX idx_f ON test_idx((fidx(a)));
+-- This should be allowed
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+IMMUTABLE LANGUAGE SQL AS $$ SELECT $1 + 2 $$;
+-- Try to replace it with a STABLE function (should fail)
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+STABLE LANGUAGE SQL AS $$ SELECT $1 + 1 $$;
+ERROR:  cannot replace function "fidx" with a non-IMMUTABLE function because it is used by an index
 -- Cleanup
 DROP SCHEMA temp_func_test CASCADE;
-NOTICE:  drop cascades to 35 other objects
+NOTICE:  drop cascades to 37 other objects
 DETAIL:  drop cascades to function functest_a_1(text,date)
 drop cascades to function functest_a_2(text[])
 drop cascades to function functest_a_3()
@@ -809,5 +822,7 @@ drop cascades to table ddl_test
 drop cascades to function alter_and_insert()
 drop cascades to function double_append(anyarray,anyelement)
 drop cascades to function test1(anyelement)
+drop cascades to function fidx(integer)
+drop cascades to table test_idx
 DROP USER regress_unpriv_user;
 RESET search_path;
diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql
index 6d1c102d780..2307eee0160 100644
--- a/src/test/regress/sql/create_function_sql.sql
+++ b/src/test/regress/sql/create_function_sql.sql
@@ -459,6 +459,19 @@ CREATE FUNCTION test1 (anyelement) RETURNS anyarray LANGUAGE SQL
 SELECT test1(0);
 RESET check_function_bodies;
 
+-- Test: prevent replacing an IMMUTABLE function used in index with a non-IMMUTABLE one
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+IMMUTABLE LANGUAGE SQL AS $$ SELECT $1 + 1 $$;
+-- Create a table and an index using that function
+CREATE TABLE test_idx(a int);
+CREATE INDEX idx_f ON test_idx((fidx(a)));
+-- This should be allowed
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+IMMUTABLE LANGUAGE SQL AS $$ SELECT $1 + 2 $$;
+-- Try to replace it with a STABLE function (should fail)
+CREATE OR REPLACE FUNCTION fidx(int) RETURNS int
+STABLE LANGUAGE SQL AS $$ SELECT $1 + 1 $$;
+
 -- Cleanup
 DROP SCHEMA temp_func_test CASCADE;
 DROP USER regress_unpriv_user;
-- 
2.43.0