schemachanger: add old zone config in zone config logs in DSC

This patch fills the `OldZoneConfig` field in
`eventpb.SetZoneConfig` to log the old zone configuration
in the declarative schema changer.

Informs: #127176

Release note: None

docs/generated/eventlog.md

@@ -3380,6 +3380,9 @@ An event of type `remove_zone_config` is recorded when a zone config is removed.
 An event of type `set_zone_config` is recorded when a zone config is changed.
 
 
+| Field | Description | Sensitive |
+|--|--|--|
+| `ResolvedOldConfig` | The string representation of the resolved old zone config. This is not necessarily the same as the zone config that was previously set -- as it includes the resolved values of the zone config options. In other words, a zone config that hasn't been properly "set" yet (and inherits from its parent) will have a resolved_old_config that has details of the values it inherits from its parent. This is particularly useful to get a proper diff between the old and new zone config. | yes |
 
 
 #### Common fields

pkg/ccl/logictestccl/testdata/logic_test/event_log

@@ -39,5 +39,5 @@ ORDER BY "timestamp", info
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.transaction_statistics CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.transaction_statistics", "User": "node"}
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.statement_activity CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.statement_activity", "User": "node"}
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.transaction_activity CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.transaction_activity", "User": "node"}
-1  {"EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 13000"], "Statement": "ALTER INDEX test.public.a@a_pkey CONFIGURE ZONE USING \"gc.ttlseconds\" = 13000", "Tag": "CONFIGURE ZONE", "Target": "INDEX test.public.a@a_pkey", "User": "root"}
-1  {"EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 15000"], "Statement": "ALTER INDEX test.public.a@bar CONFIGURE ZONE USING \"gc.ttlseconds\" = 15000", "Tag": "CONFIGURE ZONE", "Target": "INDEX test.public.a@bar", "User": "root"}
+1  {"EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 13000"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:3 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER INDEX test.public.a@a_pkey CONFIGURE ZONE USING \"gc.ttlseconds\" = 13000", "Tag": "CONFIGURE ZONE", "Target": "INDEX test.public.a@a_pkey", "User": "root"}
+1  {"EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 15000"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:3 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER INDEX test.public.a@bar CONFIGURE ZONE USING \"gc.ttlseconds\" = 15000", "Tag": "CONFIGURE ZONE", "Target": "INDEX test.public.a@bar", "User": "root"}

pkg/ccl/schemachangerccl/testdata/end_to_end/alter_index_configure_zone/alter_index_configure_zone.side_effects

@@ -11,14 +11,18 @@ ALTER INDEX t@foo CONFIGURE ZONE USING num_replicas = 7, gc.ttlseconds = 10000;
 begin transaction #1
 # begin StatementPhase
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - '"gc.ttlseconds" = 10000'
+    - num_replicas = 7
+    target: INDEX defaultdb.public.t@foo
+  resolvedOldConfig: 'range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER INDEX ‹defaultdb›.‹public›.‹t›@‹foo› CONFIGURE ZONE USING ‹num_replicas› = ‹7›, ‹"gc.ttlseconds"› = ‹10000›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 # end StatementPhase

pkg/ccl/schemachangerccl/testdata/end_to_end/alter_index_configure_zone_multiple/alter_index_configure_zone_multiple.side_effects

@@ -13,36 +13,45 @@ ALTER INDEX t@foo CONFIGURE ZONE USING num_replicas = 10;
 begin transaction #1
 # begin StatementPhase
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - num_replicas = 7
+    target: INDEX defaultdb.public.t@foo
+  resolvedOldConfig: 'range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER INDEX ‹defaultdb›.‹public›.‹t›@‹foo› CONFIGURE ZONE USING ‹num_replicas› = ‹7›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - '"gc.ttlseconds" = 10000'
+    target: INDEX defaultdb.public.t@foo
+  resolvedOldConfig: 'range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:7 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER INDEX ‹defaultdb›.‹public›.‹t›@‹foo› CONFIGURE ZONE USING ‹"gc.ttlseconds"› = ‹10000›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - num_replicas = 10
+    target: INDEX defaultdb.public.t@foo
+  resolvedOldConfig: 'range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:10000 > num_replicas:7 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER INDEX ‹defaultdb›.‹public›.‹t›@‹foo› CONFIGURE ZONE USING ‹num_replicas› = ‹10›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 # end StatementPhase

pkg/ccl/schemachangerccl/testdata/end_to_end/alter_partition_configure_zone/alter_partition_configure_zone.side_effects

@@ -16,14 +16,18 @@ ALTER PARTITION p1 OF INDEX t@t_pkey CONFIGURE ZONE USING num_replicas = 7, gc.t
 begin transaction #1
 # begin StatementPhase
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - '"gc.ttlseconds" = 10000'
+    - num_replicas = 7
+    target: PARTITION p1 OF INDEX defaultdb.public.t@t_pkey
+  resolvedOldConfig: 'inherited_constraints:false null_voter_constraints_is_empty:false inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER PARTITION ‹p1› OF INDEX ‹defaultdb›.‹public›.‹t›@‹t_pkey› CONFIGURE ZONE USING ‹num_replicas› = ‹7›, ‹"gc.ttlseconds"› = ‹10000›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 # end StatementPhase

pkg/ccl/schemachangerccl/testdata/end_to_end/alter_partition_configure_zone_multiple/alter_partition_configure_zone_multiple.side_effects

@@ -23,47 +23,59 @@ ALTER PARTITION p3 OF INDEX t@t_pkey CONFIGURE ZONE USING num_replicas = 10;
 begin transaction #1
 # begin StatementPhase
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - num_replicas = 7
+    target: PARTITION p3 OF INDEX defaultdb.public.t@t_pkey
+  resolvedOldConfig: 'inherited_constraints:false null_voter_constraints_is_empty:false inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER PARTITION ‹p3› OF INDEX ‹defaultdb›.‹public›.‹t›@‹t_pkey› CONFIGURE ZONE USING ‹num_replicas› = ‹7›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - '"gc.ttlseconds" = 10000'
+    target: PARTITION p3 OF INDEX defaultdb.public.t@t_pkey
+  resolvedOldConfig: 'inherited_constraints:false null_voter_constraints_is_empty:false inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER PARTITION ‹p3› OF INDEX ‹defaultdb›.‹public›.‹t›@‹t_pkey› CONFIGURE ZONE USING ‹"gc.ttlseconds"› = ‹10000›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - '"gc.ttlseconds" = 12000'
+    target: PARTITION p1 OF INDEX defaultdb.public.t@idx
+  resolvedOldConfig: 'inherited_constraints:false null_voter_constraints_is_empty:false inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER PARTITION ‹p1› OF INDEX ‹defaultdb›.‹public›.‹t›@‹idx› CONFIGURE ZONE USING ‹"gc.ttlseconds"› = ‹12000›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 checking for feature: CONFIGURE ZONE
-write *eventpb.AlterTable to event log:
-  mutationId: 1
+write *eventpb.SetZoneConfig to event log:
+  config:
+    options:
+    - num_replicas = 10
+    target: PARTITION p3 OF INDEX defaultdb.public.t@t_pkey
+  resolvedOldConfig: 'inherited_constraints:false null_voter_constraints_is_empty:false inherited_lease_preferences:false '
   sql:
     descriptorId: 104
     statement: ALTER PARTITION ‹p3› OF INDEX ‹defaultdb›.‹public›.‹t›@‹t_pkey› CONFIGURE ZONE USING ‹num_replicas› = ‹10›
     tag: CONFIGURE ZONE
     user: root
-  tableName: defaultdb.public.t
 ## StatementPhase stage 1 of 1 with 1 MutationType op
 upsert zone config for #104
 # end StatementPhase

pkg/sql/logictest/testdata/logic_test/event_log

@@ -537,8 +537,8 @@ ORDER BY "timestamp", info
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.transaction_statistics CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.transaction_statistics", "User": "node"}
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.statement_activity CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.statement_activity", "User": "node"}
 1  {"ApplicationName": "$ internal-set-SQLStatsTables-TTL", "EventType": "set_zone_config", "Options": ["\"gc.ttlseconds\" = 3600"], "PlaceholderValues": ["3600"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:5 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE \"\".system.transaction_activity CONFIGURE ZONE USING \"gc.ttlseconds\" = $1", "Tag": "CONFIGURE ZONE", "Target": "TABLE system.public.transaction_activity", "User": "node"}
-1  {"EventType": "set_zone_config", "Options": ["range_max_bytes = 67108865", "range_min_bytes = 16777216"], "Statement": "ALTER DATABASE test CONFIGURE ZONE USING range_max_bytes = 67108865, range_min_bytes = 16777216", "Tag": "CONFIGURE ZONE", "Target": "DATABASE test", "User": "root"}
-1  {"EventType": "set_zone_config", "Options": ["range_max_bytes = 67108865", "range_min_bytes = 16777216"], "Statement": "ALTER TABLE test.public.a CONFIGURE ZONE USING range_max_bytes = 67108865, range_min_bytes = 16777216", "Tag": "CONFIGURE ZONE", "Target": "TABLE test.public.a", "User": "root"}
+1  {"EventType": "set_zone_config", "Options": ["range_max_bytes = 67108865", "range_min_bytes = 16777216"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:3 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER DATABASE test CONFIGURE ZONE USING range_max_bytes = 67108865, range_min_bytes = 16777216", "Tag": "CONFIGURE ZONE", "Target": "DATABASE test", "User": "root"}
+1  {"EventType": "set_zone_config", "Options": ["range_max_bytes = 67108865", "range_min_bytes = 16777216"], "ResolvedOldConfig": "range_min_bytes:134217728 range_max_bytes:536870912 gc:<ttl_seconds:14400 > num_replicas:3 inherited_constraints:false null_voter_constraints_is_empty:true inherited_lease_preferences:false ", "Statement": "ALTER TABLE test.public.a CONFIGURE ZONE USING range_max_bytes = 67108865, range_min_bytes = 16777216", "Tag": "CONFIGURE ZONE", "Target": "TABLE test.public.a", "User": "root"}
 
 skipif config 3node-tenant-default-configs
 query IT

pkg/sql/schemachanger/scbuild/event_log.go

@@ -465,15 +465,18 @@ func (pb payloadBuilder) build(b buildCtx) logpb.EventPayload {
 		*scpb.PartitionZoneConfig:
 		if pb.TargetStatus == scpb.Status_PUBLIC {
 			var zcDetails eventpb.CommonZoneConfigDetails
+			var oldConfig string
 			if pb.maybePayload != nil {
 				payload := pb.maybePayload.(*eventpb.SetZoneConfig)
 				zcDetails = eventpb.CommonZoneConfigDetails{
 					Target:  payload.Target,
 					Options: payload.Options,
 				}
+				oldConfig = payload.ResolvedOldConfig
 			}
 			return &eventpb.SetZoneConfig{
 				CommonZoneConfigDetails: zcDetails,
+				ResolvedOldConfig:       oldConfig,
 			}
 		}
 	case *scpb.Trigger:

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/configure_zone.go

@@ -71,7 +71,8 @@ func SetZoneConfig(b BuildCtx, n *tree.SetZoneConfig) {
 		sqltelemetry.SchemaChangeAlterCounterWithExtra(zs.TelemetryName(), "configure_zone"),
 	)
 
-	if err = zco.applyZoneConfig(b, n, copyFromParentList, setters); err != nil {
+	oldZone, err := zco.applyZoneConfig(b, n, copyFromParentList, setters)
+	if err != nil {
 		panic(err)
 	}
 
@@ -87,7 +88,8 @@ func SetZoneConfig(b BuildCtx, n *tree.SetZoneConfig) {
 		Target:  tree.AsString(&n.ZoneSpecifier),
 		Options: optionsStr,
 	}
-	info := &eventpb.SetZoneConfig{CommonZoneConfigDetails: eventDetails}
+	info := &eventpb.SetZoneConfig{CommonZoneConfigDetails: eventDetails,
+		ResolvedOldConfig: oldZone.String()}
 	b.LogEventForExistingPayload(elem, info)
 }
 

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/database_zone_config.go

@@ -194,8 +194,8 @@ func (dzo *databaseZoneConfigObj) applyZoneConfig(
 	n *tree.SetZoneConfig,
 	copyFromParentList []tree.Name,
 	setters []func(c *zonepb.ZoneConfig),
-) error {
-	partialZone, err := prepareZoneConfig(b, n, copyFromParentList, setters, dzo)
+) (*zonepb.ZoneConfig, error) {
+	oldZone, partialZone, err := prepareZoneConfig(b, n, copyFromParentList, setters, dzo)
 	dzo.setZoneConfigToWrite(partialZone)
-	return err
+	return oldZone, err
 }

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/index_zone_config.go

@@ -178,7 +178,7 @@ func (izo *indexZoneConfigObj) applyZoneConfig(
 	n *tree.SetZoneConfig,
 	copyFromParentList []tree.Name,
 	setters []func(c *zonepb.ZoneConfig),
-) error {
+) (*zonepb.ZoneConfig, error) {
 	// TODO(annie): once we allow configuring zones for named zones/system ranges,
 	// we will need to guard against secondary tenants from configuring such
 	// ranges.
@@ -218,15 +218,15 @@ func (izo *indexZoneConfigObj) applyZoneConfig(
 	completeZone, completeSubZone, err := izo.retrieveCompleteZoneConfig(b,
 		n.SetDefault /* getInheritedDefault */)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// We need to inherit zone configuration information from the correct zone,
 	// not completeZone.
 	{
 		zoneInheritedFields, err := izo.getInheritedFieldsForPartialSubzone(b, partialZone)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		partialSubzone.Config.CopyFromZone(*zoneInheritedFields, copyFromParentList)
 	}
@@ -240,22 +240,25 @@ func (izo *indexZoneConfigObj) applyZoneConfig(
 	// If an existing subzone is being modified, finalZone is overridden.
 	finalZone := partialSubzone.Config
 
+	// Clone our zone config to log the old zone config as well as the new one.
+	oldZone := protoutil.Clone(&newZone).(*zonepb.ZoneConfig)
+
 	if n.SetDefault {
 		finalZone = *zonepb.NewZoneConfig()
 	}
 
 	// Fill in our zone configs with var = val assignments.
 	if err := loadSettingsToZoneConfigs(setters, &newZone, &finalZone); err != nil {
-		return err
+		return nil, err
 	}
 
 	// Validate that there are no conflicts in the zone setup.
 	if err := zonepb.ValidateNoRepeatKeysInZone(&newZone); err != nil {
-		return err
+		return nil, err
 	}
 
 	if err := validateZoneAttrsAndLocalities(b, currentZone, &newZone); err != nil {
-		return err
+		return nil, err
 	}
 
 	// Fill in the final zone config with subzones.
@@ -264,7 +267,7 @@ func (izo *indexZoneConfigObj) applyZoneConfig(
 
 	// Finally, revalidate everything. Validate only the completeZone config.
 	if err := completeZone.Validate(); err != nil {
-		return pgerror.Wrap(err, pgcode.CheckViolation, "could not validate zone config")
+		return nil, pgerror.Wrap(err, pgcode.CheckViolation, "could not validate zone config")
 	}
 
 	// Finally, check for the extra protection partial zone configs would
@@ -282,10 +285,10 @@ func (izo *indexZoneConfigObj) applyZoneConfig(
 		err = errors.WithHint(err,
 			"try ALTER ... CONFIGURE ZONE USING <field_name> = COPY FROM PARENT [, ...] to "+
 				"populate the field")
-		return err
+		return nil, err
 	}
 	izo.setZoneConfigToWrite(partialZone)
-	return err
+	return oldZone, err
 }
 
 // fillIndexFromZoneSpecifier fills out the index id in the zone