opt: do not add unnecessary columns to routine deps

DrewKimball · DrewKimball · commit e9cebce38bb8 · 2025-06-12T15:36:35.000-06:00
Previously, computed columns would be added to the depended-on columns list for a routine that performs an INSERT. This is unnecessary, since values for computed columns are automatically generated when the routine is invoked depending on the table schema at the time of invocation. Because of this behavior, a routine with an INSERT statement previously prevented dropping computed columns from the target table, including the implicit column used for hash-sharded indexes. This commit skips computed columns when adding insert target columns to the routine's dependency list. Note that there was already logic to do this for columns with default values. This commit also adds a session variable `use_improved_routine_dependency_tracking` to control the new behavior, default off for backports. The follow-up will flip the variable on for master. Fixes #145098 Release note (sql change): Fixed a bug that caused a routine with an INSERT statement to unnecessarily block dropping a hash-sharded index or computed column on the target table. Note that the fix applies only to newly-created routines. In releases prior to v25.3, the fix will be off by default, and can be enabled by setting the session variable `use_improved_routine_dependency_tracking`.
diff --git a/pkg/sql/logictest/testdata/logic_test/drop_index b/pkg/sql/logictest/testdata/logic_test/drop_index
@@ -563,29 +563,26 @@ CREATE TABLE tab_145100 (
 )
 
 statement ok
-CREATE PROCEDURE proc_insert_145100(in_id UUID, in_i INT) LANGUAGE SQL AS $$
-  INSERT INTO tab_145100 (id, i) VALUES (in_id, in_i);
+CREATE PROCEDURE proc_select_145100() LANGUAGE SQL AS $$
+  SELECT *, crdb_internal_i_shard_16 FROM tab_145100;
 $$;
 
-# Note: Due to https://github.com/cockroachdb/cockroach/issues/145098, the
-# procedure has a dependency on the shard column. When that issue is resolved,
-# this DROP statement should succeed.
-statement error cannot drop column "crdb_internal_i_shard_16" because function "proc_insert_145100" depends on it
+statement error cannot drop column "crdb_internal_i_shard_16" because function "proc_select_145100" depends on it
 DROP INDEX tab_145100@tab_145100_i_idx
 
 statement ok
-DROP PROCEDURE proc_insert_145100
+DROP PROCEDURE proc_select_145100
 
 statement ok
-CREATE PROCEDURE proc_select_145100() LANGUAGE SQL AS $$
-  SELECT *, crdb_internal_i_shard_16 FROM tab_145100;
+CREATE PROCEDURE proc_insert_145100(in_id UUID, in_i INT) LANGUAGE SQL AS $$
+  INSERT INTO tab_145100 (id, i) VALUES (in_id, in_i);
 $$;
 
-statement error cannot drop column "crdb_internal_i_shard_16" because function "proc_select_145100" depends on it
+statement ok
 DROP INDEX tab_145100@tab_145100_i_idx
 
 statement ok
-DROP INDEX tab_145100@tab_145100_i_idx CASCADE
+DROP PROCEDURE proc_insert_145100
 
 skipif config local-schema-locked
 query T
diff --git a/pkg/sql/logictest/testdata/logic_test/udf_delete b/pkg/sql/logictest/testdata/logic_test/udf_delete
@@ -324,3 +324,40 @@ statement ok
 SELECT f146414()
 
 subtest end
+
+# Make sure that the routine does not add unnecessary columns as dependencies.
+subtest drop_column
+
+statement ok
+CREATE TABLE table_drop (
+  a INT NOT NULL,
+  b INT NOT NULL,
+  c INT NOT NULL,
+  d INT AS (a + b) STORED,
+  -- Hash-sharded indexes generate a hidden computed column.
+  INDEX i (b ASC) USING HASH
+);
+INSERT INTO table_drop VALUES (1,2,3), (4,5,6), (7,8,9);
+
+statement ok
+CREATE FUNCTION f_delete() RETURNS INT LANGUAGE SQL AS $$
+  DELETE FROM table_drop WHERE a = b - 1;
+  SELECT 1;
+$$;
+
+statement ok
+DROP INDEX i;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN d;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN c;
+
+statement error pgcode 2BP01 pq: cannot drop column "b" because function "f_delete" depends on it
+ALTER TABLE table_drop DROP COLUMN b;
+
+statement error pgcode 2BP01 pq: cannot drop column "a" because function "f_delete" depends on it
+ALTER TABLE table_drop DROP COLUMN a;
+
+subtest end
diff --git a/pkg/sql/logictest/testdata/logic_test/udf_insert b/pkg/sql/logictest/testdata/logic_test/udf_insert
@@ -286,3 +286,105 @@ statement ok
 SELECT f146414()
 
 subtest end
+
+subtest regression_145098
+
+statement ok
+CREATE TABLE t_computed (
+  a INT NOT NULL,
+  b INT AS (a + 1) STORED,
+  c INT AS (a * 2) VIRTUAL,
+  INDEX i (a ASC) USING HASH
+)
+
+statement ok
+CREATE FUNCTION f145098() RETURNS INT LANGUAGE SQL AS $$
+  INSERT INTO t_computed VALUES (100);
+  SELECT 1;
+$$;
+
+statement ok
+SELECT f145098();
+
+query III
+SELECT * FROM t_computed;
+----
+100  101  200
+
+# The internal computed column used by the hash-sharded index should not be
+# included in the routine's dependencies, and the drop should succeed.
+statement ok
+DROP INDEX i;
+
+statement ok
+SELECT f145098();
+
+query III
+SELECT * FROM t_computed;
+----
+100  101  200
+100  101  200
+
+statement ok
+ALTER TABLE t_computed DROP COLUMN c;
+
+statement ok
+SELECT f145098();
+
+query II
+SELECT * FROM t_computed;
+----
+100  101
+100  101
+100  101
+
+statement ok
+ALTER TABLE t_computed DROP COLUMN b;
+
+statement ok
+SELECT f145098();
+
+query I
+SELECT * FROM t_computed;
+----
+100
+100
+100
+100
+
+statement error pgcode 2BP01 pq: cannot drop column "a" because function "f145098" depends on it
+ALTER TABLE t_computed DROP COLUMN a;
+
+statement ok
+DROP FUNCTION f145098;
+
+statement ok
+DROP TABLE t_computed;
+
+# Case where the INSERT statement has a RETURNING clause that references the
+# hash-sharded index column. In this case, a dependency *should* be added.
+statement ok
+CREATE TABLE t_hash_sharded (
+  a INT NOT NULL,
+  INDEX i (a ASC) USING HASH
+)
+
+statement ok
+CREATE FUNCTION f145098() RETURNS INT LANGUAGE SQL AS $$
+  INSERT INTO t_hash_sharded VALUES (100) RETURNING crdb_internal_a_shard_16;
+  SELECT 1;
+$$;
+
+statement error pgcode 2BP01 pq: cannot drop column "crdb_internal_a_shard_16" because function "f145098" depends on it
+DROP INDEX i;
+
+statement ok
+SELECT f145098();
+
+statement ok
+DROP FUNCTION f145098;
+
+statement ok
+DROP TABLE t_hash_sharded;
+
+subtest end
diff --git a/pkg/sql/logictest/testdata/logic_test/udf_update b/pkg/sql/logictest/testdata/logic_test/udf_update
@@ -348,3 +348,41 @@ statement ok
 SELECT f146414()
 
 subtest end
+
+# Make sure that the routine does not add unnecessary columns as dependencies.
+subtest drop_column
+
+statement ok
+CREATE TABLE table_drop (
+  a INT NOT NULL,
+  b INT NOT NULL,
+  c INT NOT NULL,
+  d INT AS (a + b) STORED,
+  -- Hash-sharded indexes generate a hidden computed column.
+  INDEX i (b ASC) USING HASH
+);
+INSERT INTO table_drop VALUES (1,2,3), (4,5,6), (7,8,9);
+
+statement ok
+DROP FUNCTION f_update;
+CREATE FUNCTION f_update() RETURNS INT LANGUAGE SQL AS $$
+  UPDATE table_drop SET b = b + 1 WHERE a = 1;
+  SELECT 1;
+$$;
+
+statement ok
+DROP INDEX i;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN d;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN c;
+
+statement error pgcode 2BP01 pq: cannot drop column "b" because function "f_update" depends on it
+ALTER TABLE table_drop DROP COLUMN b;
+
+statement error pgcode 2BP01 pq: cannot drop column "a" because function "f_update" depends on it
+ALTER TABLE table_drop DROP COLUMN a;
+
+subtest end
diff --git a/pkg/sql/logictest/testdata/logic_test/udf_upsert b/pkg/sql/logictest/testdata/logic_test/udf_upsert
@@ -260,3 +260,41 @@ statement ok
 SELECT f146414()
 
 subtest end
+
+# Make sure that the routine does not add unnecessary columns as dependencies.
+subtest drop_column
+
+statement ok
+CREATE TABLE table_drop (
+  a INT NOT NULL,
+  b INT NOT NULL,
+  c INT NOT NULL,
+  d INT AS (a + b) STORED,
+  -- Hash-sharded indexes generate a hidden computed column.
+  INDEX i (b ASC) USING HASH
+);
+INSERT INTO table_drop VALUES (1,2,3), (4,5,6), (7,8,9);
+
+statement ok
+DROP FUNCTION f_upsert;
+CREATE FUNCTION f_upsert() RETURNS INT LANGUAGE SQL AS $$
+  UPSERT INTO table_drop (a, b) VALUES (100, 200);
+  SELECT 1;
+$$;
+
+statement ok
+DROP INDEX i;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN d;
+
+statement ok
+ALTER TABLE table_drop DROP COLUMN c;
+
+statement error pgcode 2BP01 pq: cannot drop column "b" because function "f_upsert" depends on it
+ALTER TABLE table_drop DROP COLUMN b;
+
+statement error pgcode 2BP01 pq: cannot drop column "a" because function "f_upsert" depends on it
+ALTER TABLE table_drop DROP COLUMN a;
+
+subtest end
diff --git a/pkg/sql/opt/optbuilder/mutation_builder.go b/pkg/sql/opt/optbuilder/mutation_builder.go
@@ -848,6 +848,11 @@ func (mb *mutationBuilder) addSynthesizedComputedCols(colIDs opt.OptionalColList
 		// Remember id of newly synthesized column.
 		colIDs[i] = newCol
 
+		// Track columns that were not explicitly set in the insert statement.
+		if mb.b.trackSchemaDeps {
+			mb.implicitInsertCols.Add(newCol)
+		}
+
 		// Add corresponding target column.
 		mb.targetColList = append(mb.targetColList, tabColID)
 		mb.targetColSet.Add(tabColID)
