Merge pull request #110750 from ecwall/backport22.2-110252

release-22.2: ttl: do not remove all table TTL settings on RESET (ttl_expire_after)

pkg/sql/alter_table.go

@@ -1952,14 +1952,8 @@ func handleTTLStorageParamChange(
 		}
 	}
 
-	// Add TTL mutation if TTL is newly SET.
-	addTTLMutation := before == nil && after != nil
-
 	// Create new column.
 	if (before == nil || !before.HasDurationExpr()) && (after != nil && after.HasDurationExpr()) {
-		// Adding a TTL requires adding the automatic column and deferring the TTL
-		// addition to after the column is successfully added.
-		addTTLMutation = true
 		if _, err := tableDesc.FindColumnWithName(catpb.TTLDefaultExpirationColumnName); err == nil {
 			return false, pgerror.Newf(
 				pgcode.InvalidTableDefinition,
@@ -1994,21 +1988,9 @@ func handleTTLStorageParamChange(
 		}
 	}
 
-	// Add TTL mutation so that job is scheduled in SchemaChanger.
-	if addTTLMutation {
-		tableDesc.AddModifyRowLevelTTLMutation(
-			&descpb.ModifyRowLevelTTL{RowLevelTTL: after},
-			descpb.DescriptorMutation_ADD,
-		)
-	}
-
-	dropTTLMutation := before != nil && after == nil
-
 	// Remove existing column.
 	if (before != nil && before.HasDurationExpr()) && (after == nil || !after.HasDurationExpr()) {
 		telemetry.Inc(sqltelemetry.RowLevelTTLDropped)
-		// Create the DROP COLUMN job and the associated mutation.
-		dropTTLMutation = true
 		droppedViews, err := dropColumnImpl(params, tn, tableDesc, after, &tree.AlterTableDropColumn{
 			Column: catpb.TTLDefaultExpirationColumnName,
 		})
@@ -2021,10 +2003,22 @@ func handleTTLStorageParamChange(
 		}
 	}
 
-	if dropTTLMutation {
+	// Adding TTL requires adding the TTL job before adding the TTL fields.
+	// Removing TTL requires removing the TTL job before removing the TTL fields.
+	var direction descpb.DescriptorMutation_Direction
+	switch {
+	case before == nil && after != nil:
+		direction = descpb.DescriptorMutation_ADD
+	case before != nil && after == nil:
+		direction = descpb.DescriptorMutation_DROP
+	default:
+		descriptorChanged = true
+	}
+	if !descriptorChanged {
+		// Add TTL mutation so that job is scheduled in SchemaChanger.
 		tableDesc.AddModifyRowLevelTTLMutation(
 			&descpb.ModifyRowLevelTTL{RowLevelTTL: after},
-			descpb.DescriptorMutation_DROP,
+			direction,
 		)
 	}
 
@@ -2035,7 +2029,7 @@ func handleTTLStorageParamChange(
 		}
 	}
 
-	descriptorChanged = !addTTLMutation && !dropTTLMutation
+	// Modify the TTL fields here because it will not be done in a mutation.
 	if descriptorChanged {
 		tableDesc.RowLevelTTL = after
 	}

pkg/sql/logictest/testdata/logic_test/row_level_ttl

@@ -503,65 +503,92 @@ ALTER TABLE test.public.tbl_ttl_on_noop WITH (ttl = 'on', ...)
 
 subtest end
 
-subtest ttl_job_cron
+subtest ttl_job_cron_invalid
 
 statement error invalid cron expression for "ttl_job_cron"
 CREATE TABLE tbl () WITH (ttl_expire_after = '10 seconds', ttl_job_cron = 'bad expr')
 
+subtest end
+
+subtest create_ttl_job_cron
+
 statement ok
-CREATE TABLE tbl_ttl_job_cron (
+CREATE TABLE tbl_create_ttl_job_cron (
   id INT PRIMARY KEY
 ) WITH (ttl_expire_after = '10 minutes', ttl_job_cron = '@daily')
 
 query T
-SELECT create_statement FROM [SHOW CREATE TABLE tbl_ttl_job_cron]
+SELECT create_statement FROM [SHOW CREATE TABLE tbl_create_ttl_job_cron]
 ----
-CREATE TABLE public.tbl_ttl_job_cron (
+CREATE TABLE public.tbl_create_ttl_job_cron (
   id INT8 NOT NULL,
   crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
-  CONSTRAINT tbl_ttl_job_cron_pkey PRIMARY KEY (id ASC)
+  CONSTRAINT tbl_create_ttl_job_cron_pkey PRIMARY KEY (id ASC)
 ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@daily')
 
 let $table_id
-SELECT oid FROM pg_class WHERE relname = 'tbl_ttl_job_cron'
+SELECT oid FROM pg_class WHERE relname = 'tbl_create_ttl_job_cron'
 
 query TTT
 SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
 WHERE label = 'row-level-ttl-$table_id'
 ----
 ACTIVE  @daily  root
 
+subtest end
+
+subtest set_ttl_job_cron
+
 statement ok
-ALTER TABLE tbl_ttl_job_cron SET (ttl_job_cron = '@weekly')
+CREATE TABLE tbl_set_ttl_job_cron (
+  id INT PRIMARY KEY
+) WITH (ttl_expire_after = '10 minutes', ttl_job_cron = '@daily')
+
+statement ok
+ALTER TABLE tbl_set_ttl_job_cron SET (ttl_job_cron = '@weekly')
 
 query T
-SELECT create_statement FROM [SHOW CREATE TABLE tbl_ttl_job_cron]
+SELECT create_statement FROM [SHOW CREATE TABLE tbl_set_ttl_job_cron]
 ----
-CREATE TABLE public.tbl_ttl_job_cron (
+CREATE TABLE public.tbl_set_ttl_job_cron (
   id INT8 NOT NULL,
   crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
-  CONSTRAINT tbl_ttl_job_cron_pkey PRIMARY KEY (id ASC)
+  CONSTRAINT tbl_set_ttl_job_cron_pkey PRIMARY KEY (id ASC)
 ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@weekly')
 
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_set_ttl_job_cron'
 
 query TTT
 SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
 WHERE label = 'row-level-ttl-$table_id'
 ----
 ACTIVE  @weekly  root
 
+subtest end
+
+subtest reset_ttl_job_cron
+
 statement ok
-ALTER TABLE tbl_ttl_job_cron RESET (ttl_job_cron)
+CREATE TABLE tbl_reset_ttl_job_cron (
+  id INT PRIMARY KEY
+) WITH (ttl_expire_after = '10 minutes', ttl_job_cron = '@daily')
+
+statement ok
+ALTER TABLE tbl_reset_ttl_job_cron RESET (ttl_job_cron)
 
 query T
-SELECT create_statement FROM [SHOW CREATE TABLE tbl_ttl_job_cron]
+SELECT create_statement FROM [SHOW CREATE TABLE tbl_reset_ttl_job_cron]
 ----
-CREATE TABLE public.tbl_ttl_job_cron (
+CREATE TABLE public.tbl_reset_ttl_job_cron (
   id INT8 NOT NULL,
   crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
-  CONSTRAINT tbl_ttl_job_cron_pkey PRIMARY KEY (id ASC)
+  CONSTRAINT tbl_reset_ttl_job_cron_pkey PRIMARY KEY (id ASC)
 ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly')
 
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_reset_ttl_job_cron'
+
 query TTT
 SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
 WHERE label = 'row-level-ttl-$table_id'
@@ -810,19 +837,14 @@ CREATE TABLE public.tbl_add_ttl_expiration_expression_to_ttl_expire_after (
   FAMILY fam_0_id_expire_at_crdb_internal_expiration (id, expire_at, crdb_internal_expiration)
 ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_expiration_expression = 'crdb_internal_expiration', ttl_job_cron = '@hourly')
 
-statement ok
-ALTER TABLE tbl_add_ttl_expiration_expression_to_ttl_expire_after RESET (ttl_expiration_expression)
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_add_ttl_expiration_expression_to_ttl_expire_after'
 
-query T
-SELECT create_statement FROM [SHOW CREATE TABLE tbl_add_ttl_expiration_expression_to_ttl_expire_after]
+query TTT
+SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
+WHERE label = 'row-level-ttl-$table_id'
 ----
-CREATE TABLE public.tbl_add_ttl_expiration_expression_to_ttl_expire_after (
-  id INT8 NOT NULL,
-  expire_at TIMESTAMPTZ NULL,
-  crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
-  CONSTRAINT tbl_add_ttl_expiration_expression_to_ttl_expire_after_pkey PRIMARY KEY (id ASC),
-  FAMILY fam_0_id_expire_at_crdb_internal_expiration (id, expire_at, crdb_internal_expiration)
-) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly')
+ACTIVE  @hourly  root
 
 subtest end
 
@@ -849,18 +871,14 @@ CREATE TABLE public.tbl_add_ttl_expire_after_to_ttl_expiration_expression (
   FAMILY fam_0_id_expire_at (id, expire_at, crdb_internal_expiration)
 ) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_expiration_expression = 'expire_at', ttl_job_cron = '@hourly')
 
-statement ok
-ALTER TABLE tbl_add_ttl_expire_after_to_ttl_expiration_expression RESET (ttl_expire_after)
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_add_ttl_expire_after_to_ttl_expiration_expression'
 
-query T
-SELECT create_statement FROM [SHOW CREATE TABLE tbl_add_ttl_expire_after_to_ttl_expiration_expression]
+query TTT
+SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
+WHERE label = 'row-level-ttl-$table_id'
 ----
-CREATE TABLE public.tbl_add_ttl_expire_after_to_ttl_expiration_expression (
-  id INT8 NOT NULL,
-  expire_at TIMESTAMPTZ NULL,
-  CONSTRAINT tbl_add_ttl_expire_after_to_ttl_expiration_expression_pkey PRIMARY KEY (id ASC),
-  FAMILY fam_0_id_expire_at (id, expire_at)
-)
+ACTIVE  @hourly  root
 
 subtest end
 
@@ -1089,3 +1107,78 @@ statement ok
 DROP TABLE "Table-Name"
 
 subtest end
+
+# sets both ttl_exipire_after and ttl_expiration_expression
+subtest set_both_reset_ttl_expire_after
+
+statement ok
+CREATE TABLE tbl_set_both_reset_ttl_expire_after (
+  id INT PRIMARY KEY,
+  expire_at TIMESTAMPTZ,
+  FAMILY (id, expire_at)
+) WITH (
+  ttl_expire_after = '10m',
+  ttl_expiration_expression = 'expire_at'
+)
+
+statement ok
+ALTER TABLE tbl_set_both_reset_ttl_expire_after RESET (ttl_expire_after)
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE tbl_set_both_reset_ttl_expire_after]
+----
+CREATE TABLE public.tbl_set_both_reset_ttl_expire_after (
+  id INT8 NOT NULL,
+  expire_at TIMESTAMPTZ NULL,
+  CONSTRAINT tbl_set_both_reset_ttl_expire_after_pkey PRIMARY KEY (id ASC),
+  FAMILY fam_0_id_expire_at_crdb_internal_expiration (id, expire_at)
+) WITH (ttl = 'on', ttl_expiration_expression = 'expire_at', ttl_job_cron = '@hourly')
+
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_set_both_reset_ttl_expire_after'
+
+query TTT
+SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
+WHERE label = 'row-level-ttl-$table_id'
+----
+ACTIVE  @hourly  root
+
+subtest end
+
+# sets both ttl_exipire_after and ttl_expiration_expression
+subtest set_both_reset_ttl_expiration_expression
+
+statement ok
+CREATE TABLE tbl_set_both_reset_ttl_expiration_expression (
+  id INT PRIMARY KEY,
+  expire_at TIMESTAMPTZ,
+  FAMILY (id, expire_at)
+) WITH (
+  ttl_expire_after = '10m',
+  ttl_expiration_expression = 'expire_at'
+)
+
+statement ok
+ALTER TABLE tbl_set_both_reset_ttl_expiration_expression RESET (ttl_expiration_expression)
+
+query T
+SELECT create_statement FROM [SHOW CREATE TABLE tbl_set_both_reset_ttl_expiration_expression]
+----
+CREATE TABLE public.tbl_set_both_reset_ttl_expiration_expression (
+  id INT8 NOT NULL,
+  expire_at TIMESTAMPTZ NULL,
+  crdb_internal_expiration TIMESTAMPTZ NOT VISIBLE NOT NULL DEFAULT current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL ON UPDATE current_timestamp():::TIMESTAMPTZ + '00:10:00':::INTERVAL,
+  CONSTRAINT tbl_set_both_reset_ttl_expiration_expression_pkey PRIMARY KEY (id ASC),
+  FAMILY fam_0_id_expire_at_crdb_internal_expiration (id, expire_at, crdb_internal_expiration)
+) WITH (ttl = 'on', ttl_expire_after = '00:10:00':::INTERVAL, ttl_job_cron = '@hourly')
+
+let $table_id
+SELECT oid FROM pg_class WHERE relname = 'tbl_set_both_reset_ttl_expiration_expression'
+
+query TTT
+SELECT schedule_status, recurrence, owner FROM [SHOW SCHEDULES]
+WHERE label = 'row-level-ttl-$table_id'
+----
+ACTIVE  @hourly  root
+
+subtest end

pkg/sql/schema_changer.go

@@ -1609,7 +1609,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error {
 							return err
 						}
 					}
-					scTable.RowLevelTTL = nil
+					scTable.RowLevelTTL = modify.RowLevelTTL()
 				}
 			}
 

pkg/sql/show_create_external_connection.go

@@ -120,8 +120,8 @@ func (p *planner) ShowCreateExternalConnection(
 			var rows []tree.Datums
 			for _, conn := range connections {
 				row := tree.Datums{
-					scheduleID: tree.NewDString(conn.ConnectionName()),
-					createStmt: tree.NewDString(conn.UnredactedConnectionStatement()),
+					scheduleIDIdx: tree.NewDString(conn.ConnectionName()),
+					createStmtIdx: tree.NewDString(conn.UnredactedConnectionStatement()),
 				}
 				rows = append(rows, row)
 			}

pkg/sql/show_create_schedule.go

@@ -33,8 +33,8 @@ var showCreateTableColumns = colinfo.ResultColumns{
 }
 
 const (
-	scheduleID = iota
-	createStmt
+	scheduleIDIdx = iota
+	createStmtIdx
 )
 
 func loadSchedules(params runParams, n *tree.ShowCreateSchedules) ([]*jobs.ScheduledJob, error) {
@@ -126,8 +126,8 @@ func (p *planner) ShowCreateSchedule(
 				}
 
 				row := tree.Datums{
-					scheduleID: tree.NewDInt(tree.DInt(sj.ScheduleID())),
-					createStmt: tree.NewDString(createStmtStr),
+					scheduleIDIdx: tree.NewDInt(tree.DInt(sj.ScheduleID())),
+					createStmtIdx: tree.NewDString(createStmtStr),
 				}
 				rows = append(rows, row)
 			}