Merge #146250 #146430

146250: sql: improve routine dependency tracking r=yuzefovich,michae2 a=DrewKimball

#### sql: track routine dependencies on columns in RETURNING clause

Previously, while building a routine, we did not explicitly add dependencies
to columns referenced in the RETURNING clause of mutations within the body
statements. This was not a problem for INSERT or UPSERT due to #145098, but
could cause a routine with UPDATE or DELETE statements to be broken after
a referenced column was dropped. This commit fixes the bug by tracking
column references in the RETURNING clause.

Fixes #146414

Release note (bug fix): Fixed a bug that allowed a column to be dropped
from a table even if it was referenced in the RETURNING clause of an UPDATE
or DELETE statement in a routine. 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`.

#### opt: do not add unnecessary columns to routine deps

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`.

#### sql: add session setting for routine dependency improvements

This commit adds a session var `use_improved_routine_dependency_tracking`
which enables the improvements in the previous two commits (do not add
dependencies on unnecessary columns, and track dependencies in the
RETURNING clause). The var is off by default in this commit for backports.

Informs #145098
Informs #146414

Release note: None

#### opt: enable improved routine dependency tracking

This commit flips `use_improved_routine_dependency_tracking` to on by
default. This means that newly-created routines will not add unnecessary
implicit insert columns to the list of dependencies, and will correctly
track dependencies in the RETURNING clause of UPDATE and DELETE statements.

Informs #145098
Informs #146414

Release note: None

146430: sql: report unit for some session variables r=yuzefovich,drewkimball a=Guilherme1Rocha

Previously, we never populated the `unit` column of `pg_settings`
virtual table deviating from PostgreSQL. We now report the unit for
time-based and memory-based settings.

AFAIU the unit defines the default unit of the value which allows these
settings take in integer numbers with the unit providing the necessary
measurement dimension.

Fixes: #30717
Release note: None

Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Guilherme1Rocha <[email protected]>

Author: 3 people

pkg/sql/exec_util.go

@@ -4190,6 +4190,10 @@ func (m *sessionDataMutator) SetInitialRetryBackoffForReadCommitted(val time.Dur
 	m.data.InitialRetryBackoffForReadCommitted = val
 }
 
+func (m *sessionDataMutator) SetUseImprovedRoutineDependencyTracking(val bool) {
+	m.data.UseImprovedRoutineDependencyTracking = val
+}
+
 // Utility functions related to scrubbing sensitive information on SQL Stats.
 
 // quantizeCounts ensures that the Count field in the

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

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -4133,6 +4133,7 @@ unbounded_parallel_scans                                   off
 unconstrained_non_covering_index_scan_enabled              off
 unsafe_allow_triggers_modifying_cascades                   off
 use_cputs_on_non_unique_indexes                            off
+use_improved_routine_dependency_tracking                   on
 use_pre_25_2_variadic_builtins                             off
 variable_inequality_lookup_join_enabled                    on
 vector_search_beam_size                                    32

pkg/sql/logictest/testdata/logic_test/pg_catalog

@@ -3128,6 +3128,7 @@ unconstrained_non_covering_index_scan_enabled              off                 N
 unsafe_allow_triggers_modifying_cascades                   off                 NULL      NULL        NULL        string
 use_cputs_on_non_unique_indexes                            off                 NULL      NULL        NULL        string
 use_declarative_schema_changer                             on                  NULL      NULL        NULL        string
+use_improved_routine_dependency_tracking                   on                  NULL      NULL        NULL        string
 use_pre_25_2_variadic_builtins                             off                 NULL      NULL        NULL        string
 variable_inequality_lookup_join_enabled                    on                  NULL      NULL        NULL        string
 vector_search_beam_size                                    32                  NULL      NULL        NULL        string
@@ -3181,7 +3182,7 @@ cost_scans_with_default_col_size                           off                 N
 create_table_with_schema_locked                            off                 NULL  user     NULL      off                 off
 database                                                   test                NULL  user     NULL      ·                   test
 datestyle                                                  ISO, MDY            NULL  user     NULL      ISO, MDY            ISO, MDY
-deadlock_timeout                                           0                   NULL  user     NULL      0s                  0s
+deadlock_timeout                                           0                   ms    user     NULL      0s                  0s
 declare_cursor_statement_timeout_enabled                   on                  NULL  user     NULL      on                  on
 default_int_size                                           8                   NULL  user     NULL      8                   8
 default_table_access_method                                heap                NULL  user     NULL      heap                heap
@@ -3232,19 +3233,19 @@ experimental_hash_group_join_enabled                       off                 N
 extra_float_digits                                         1                   NULL  user     NULL      1                   1
 force_savepoint_restart                                    off                 NULL  user     NULL      off                 off
 foreign_key_cascades_limit                                 10000               NULL  user     NULL      10000               10000
-idle_in_transaction_session_timeout                        0                   NULL  user     NULL      0s                  0s
-idle_session_timeout                                       0                   NULL  user     NULL      0s                  0s
-index_join_streamer_batch_size                             8.0 MiB             NULL  user     NULL      8.0 MiB             8.0 MiB
+idle_in_transaction_session_timeout                        0                   ms    user     NULL      0s                  0s
+idle_session_timeout                                       0                   ms    user     NULL      0s                  0s
+index_join_streamer_batch_size                             8.0 MiB             B     user     NULL      8.0 MiB             8.0 MiB
 index_recommendations_enabled                              off                 NULL  user     NULL      on                  false
 initial_retry_backoff_for_read_committed                   2                   NULL  user     NULL      2ms                 2ms
 inject_retry_errors_enabled                                off                 NULL  user     NULL      off                 off
 inject_retry_errors_on_commit_enabled                      off                 NULL  user     NULL      off                 off
 integer_datetimes                                          on                  NULL  user     NULL      on                  on
 intervalstyle                                              postgres            NULL  user     NULL      postgres            postgres
 is_superuser                                               on                  NULL  user     NULL      on                  on
-join_reader_index_join_strategy_batch_size                 4.0 MiB             NULL  user     NULL      4.0 MiB             4.0 MiB
-join_reader_no_ordering_strategy_batch_size                2.0 MiB             NULL  user     NULL      2.0 MiB             2.0 MiB
-join_reader_ordering_strategy_batch_size                   100 KiB             NULL  user     NULL      100 KiB             100 KiB
+join_reader_index_join_strategy_batch_size                 4.0 MiB             B     user     NULL      4.0 MiB             4.0 MiB
+join_reader_no_ordering_strategy_batch_size                2.0 MiB             B     user     NULL      2.0 MiB             2.0 MiB
+join_reader_ordering_strategy_batch_size                   100 KiB             B     user     NULL      100 KiB             100 KiB
 large_full_scan_rows                                       0                   NULL  user     NULL      0                   0
 lc_collate                                                 C.UTF-8             NULL  user     NULL      C.UTF-8             C.UTF-8
 lc_ctype                                                   C.UTF-8             NULL  user     NULL      C.UTF-8             C.UTF-8
@@ -3255,7 +3256,7 @@ lc_time                                                    C.UTF-8             N
 legacy_varchar_typing                                      off                 NULL  user     NULL      off                 off
 locality                                                   region=test,dc=dc1  NULL  user     NULL      region=test,dc=dc1  region=test,dc=dc1
 locality_optimized_partitioned_index_scan                  on                  NULL  user     NULL      on                  on
-lock_timeout                                               0                   NULL  user     NULL      0s                  0s
+lock_timeout                                               0                   ms    user     NULL      0s                  0s
 log_timezone                                               UTC                 NULL  user     NULL      UTC                 UTC
 max_connections                                            -1                  NULL  user     NULL      -1                  -1
 max_identifier_length                                      128                 NULL  user     NULL      128                 128
@@ -3307,7 +3308,7 @@ pg_trgm.similarity_threshold                               0.3                 N
 plan_cache_mode                                            auto                NULL  user     NULL      auto                auto
 plpgsql_use_strict_into                                    off                 NULL  user     NULL      off                 off
 prefer_lookup_joins_for_fks                                off                 NULL  user     NULL      off                 off
-prepared_statements_cache_size                             0 B                 NULL  user     NULL      0 B                 0 B
+prepared_statements_cache_size                             0 B                 B     user     NULL      0 B                 0 B
 propagate_admission_header_to_leaf_transactions            on                  NULL  user     NULL      on                  on
 propagate_input_ordering                                   off                 NULL  user     NULL      off                 off
 recursion_depth_limit                                      1000                NULL  user     NULL      1000                1000
@@ -3326,7 +3327,7 @@ show_primary_key_constraint_on_not_visible_columns         on                  N
 sql_safe_updates                                           off                 NULL  user     NULL      off                 off
 ssl                                                        on                  NULL  user     NULL      on                  on
 standard_conforming_strings                                on                  NULL  user     NULL      on                  on
-statement_timeout                                          0                   NULL  user     NULL      0s                  0s
+statement_timeout                                          0                   ms    user     NULL      0s                  0s
 streamer_always_maintain_ordering                          off                 NULL  user     NULL      off                 off
 streamer_enabled                                           on                  NULL  user     NULL      on                  on
 streamer_head_of_line_only_fraction                        0.8                 NULL  user     NULL      0.8                 0.8
@@ -3352,13 +3353,14 @@ transaction_rows_read_log                                  0                   N
 transaction_rows_written_err                               0                   NULL  user     NULL      0                   0
 transaction_rows_written_log                               0                   NULL  user     NULL      0                   0
 transaction_status                                         NoTxn               NULL  user     NULL      NoTxn               NoTxn
-transaction_timeout                                        0                   NULL  user     NULL      0s                  0s
+transaction_timeout                                        0                   ms    user     NULL      0s                  0s
 troubleshooting_mode                                       off                 NULL  user     NULL      off                 off
 unbounded_parallel_scans                                   off                 NULL  user     NULL      off                 off
 unconstrained_non_covering_index_scan_enabled              off                 NULL  user     NULL      off                 off
 unsafe_allow_triggers_modifying_cascades                   off                 NULL  user     NULL      off                 off
 use_cputs_on_non_unique_indexes                            off                 NULL  user     NULL      off                 off
 use_declarative_schema_changer                             on                  NULL  user     NULL      on                  on
+use_improved_routine_dependency_tracking                   on                  NULL  user     NULL      on                  on
 use_pre_25_2_variadic_builtins                             off                 NULL  user     NULL      off                 off
 variable_inequality_lookup_join_enabled                    on                  NULL  user     NULL      on                  on
 vector_search_beam_size                                    32                  NULL  user     NULL      32                  32
@@ -3582,6 +3584,7 @@ unconstrained_non_covering_index_scan_enabled              NULL    NULL     NULL
 unsafe_allow_triggers_modifying_cascades                   NULL    NULL     NULL     NULL        NULL
 use_cputs_on_non_unique_indexes                            NULL    NULL     NULL     NULL        NULL
 use_declarative_schema_changer                             NULL    NULL     NULL     NULL        NULL
+use_improved_routine_dependency_tracking                   NULL    NULL     NULL     NULL        NULL
 use_pre_25_2_variadic_builtins                             NULL    NULL     NULL     NULL        NULL
 variable_inequality_lookup_join_enabled                    NULL    NULL     NULL     NULL        NULL
 vector_search_beam_size                                    NULL    NULL     NULL     NULL        NULL

pkg/sql/logictest/testdata/logic_test/set

@@ -940,3 +940,22 @@ subtest end
 # Regression test for incorrectly marking this variable as boolean.
 statement ok
 SET copy_num_retries_per_batch = 5;
+
+# Tests for a byte-size setting where integer values are provided (which are
+# treated as number of bytes).
+statement ok
+SET distsql_workmem = 1048576
+
+query T
+SHOW distsql_workmem
+----
+1.0 MiB
+
+statement error distsql_workmem cannot be set to a negative value
+SET distsql_workmem = -1
+
+statement error distsql_workmem can only be set to a value greater than 1
+SET distsql_workmem = 1
+
+statement ok
+RESET distsql_workmem

pkg/sql/logictest/testdata/logic_test/show_source

@@ -242,6 +242,7 @@ unconstrained_non_covering_index_scan_enabled              off
 unsafe_allow_triggers_modifying_cascades                   off
 use_cputs_on_non_unique_indexes                            off
 use_declarative_schema_changer                             on
+use_improved_routine_dependency_tracking                   on
 use_pre_25_2_variadic_builtins                             off
 variable_inequality_lookup_join_enabled                    on
 vector_search_beam_size                                    32

pkg/sql/logictest/testdata/logic_test/udf_delete

@@ -302,3 +302,62 @@ SELECT * FROM u_a;
 2  b  20
 
 subtest end
+
+subtest regression_146414
+
+statement ok
+CREATE TABLE t146414 (
+  a INT NOT NULL,
+  b INT AS (a + 1) VIRTUAL
+)
+
+statement ok
+CREATE FUNCTION f146414() RETURNS INT LANGUAGE SQL AS $$
+  DELETE FROM t146414 WHERE a = 1 RETURNING b;
+  SELECT 1;
+$$;
+
+statement error pgcode 2BP01 pq: cannot drop column "b" because function "f146414" depends on it
+ALTER TABLE t146414 DROP COLUMN b;
+
+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