sql: change SetZoneConfig to return type Ack

This commit changes the return type for `tree.SetZoneConfig` to `Ack`
from `RowsAffected`. This mirrors similar statements, and the row count
has been broken (always zero) ever since this statement was implemented
in the declarative schema changer, anyway. Removing the need to track
row count allows a few related functions to be simplified.

Epic: None

Release note: None

Author: DrewKimball

pkg/cli/interactive_tests/test_missing_log_output.tcl

@@ -56,7 +56,7 @@ interrupt_and_wait
 # Disable replication so as to avoid spurious purgatory errors.
 start_server $argv
 send "$argv sql --insecure -e \"ALTER RANGE default CONFIGURE ZONE USING num_replicas = 1\"\r"
-eexpect {CONFIGURE ZONE [0-9]}
+eexpect "CONFIGURE ZONE"
 eexpect ":/# "
 stop_server $argv
 

pkg/sql/database_region_change_finalizer.go

@@ -115,7 +115,7 @@ func (r *databaseRegionChangeFinalizer) preDrop(ctx context.Context, txn descs.T
 		return err
 	}
 	for _, update := range zoneConfigUpdates {
-		if _, err := writeZoneConfigUpdate(
+		if err = writeZoneConfigUpdate(
 			ctx, txn,
 			r.localPlanner.ExtendedEvalContext().Tracing.KVTracingEnabled(),
 			update,

pkg/sql/plan.go

@@ -272,7 +272,6 @@ var _ planNode = &zeroNode{}
 var _ planNodeFastPath = &deleteRangeNode{}
 var _ planNodeFastPath = &rowCountNode{}
 var _ planNodeFastPath = &serializeNode{}
-var _ planNodeFastPath = &setZoneConfigNode{}
 var _ planNodeFastPath = &controlJobsNode{}
 var _ planNodeFastPath = &controlSchedulesNode{}
 

pkg/sql/region_util.go

@@ -534,8 +534,7 @@ func ApplyZoneConfigForMultiRegionTable(
 	if update == nil || err != nil {
 		return err
 	}
-	_, err = writeZoneConfigUpdate(ctx, txn, kvTrace, update)
-	return err
+	return writeZoneConfigUpdate(ctx, txn, kvTrace, update)
 }
 
 // generateAndValidateZoneConfigForMultiRegionDatabase returns a validated
@@ -654,15 +653,14 @@ func applyZoneConfigForMultiRegionDatabase(
 	)
 	// If the new zone config is the same as a blank zone config, delete it.
 	if newZoneConfig.Equal(zonepb.NewZoneConfig()) {
-		_, err = writeZoneConfigUpdate(
+		return writeZoneConfigUpdate(
 			ctx,
 			txn,
 			kvTrace,
 			&zoneConfigUpdate{id: dbID, zoneConfig: nil},
 		)
-		return err
 	}
-	if _, err := writeZoneConfig(
+	return writeZoneConfig(
 		ctx,
 		txn,
 		dbID,
@@ -672,10 +670,7 @@ func applyZoneConfigForMultiRegionDatabase(
 		execConfig,
 		false, /* hasNewSubzones */
 		kvTrace,
-	); err != nil {
-		return err
-	}
-	return nil
+	)
 }
 
 type refreshZoneConfigOptions struct {
@@ -787,7 +782,7 @@ func (p *planner) refreshZoneConfigsForTablesWithValidation(
 	// TODO(janexing): if any write failed, do we roll back? Same question to the
 	// original p.forEachMutableTableInDatabase().
 	for _, update := range zoneConfigUpdates {
-		if _, err := writeZoneConfigUpdate(
+		if err = writeZoneConfigUpdate(
 			ctx,
 			p.InternalSQLTxn(),
 			p.ExtendedEvalContext().Tracing.KVTracingEnabled(),

pkg/sql/schema_changer.go

@@ -2256,7 +2256,7 @@ func maybeUpdateZoneConfigsForPKChange(
 
 	// Write the zone back. This call regenerates the index spans that apply
 	// to each partition in the index.
-	_, err = writeZoneConfig(
+	err = writeZoneConfig(
 		ctx, txn, table.ID, table,
 		zoneWithRaw.ZoneConfigProto(), zoneWithRaw.GetRawBytesInStorage(),
 		execCfg, false, kvTrace,

pkg/sql/sem/tree/stmt.go

@@ -1690,7 +1690,7 @@ func (*SetTracing) StatementTag() string { return "SET TRACING" }
 func (*SetTracing) observerStatement() {}
 
 // StatementReturnType implements the Statement interface.
-func (*SetZoneConfig) StatementReturnType() StatementReturnType { return RowsAffected }
+func (*SetZoneConfig) StatementReturnType() StatementReturnType { return Ack }
 
 // StatementType implements the Statement interface.
 func (*SetZoneConfig) StatementType() StatementType { return TypeDCL }

pkg/sql/set_zone_config.go

@@ -44,8 +44,6 @@ type setZoneConfigNode struct {
 	yamlConfig    tree.TypedExpr
 	options       map[tree.Name]zone.OptionValue
 	setDefault    bool
-
-	run setZoneConfigRun
 }
 
 func (p *planner) getUpdatedZoneConfigYamlConfig(
@@ -230,11 +228,6 @@ func checkPrivilegeForSetZoneConfig(ctx context.Context, p *planner, zs tree.Zon
 		[]privilege.Kind{privilege.ZONECONFIG, privilege.CREATE}, string(tableDesc.DescriptorType()), tableDesc.GetName())
 }
 
-// setZoneConfigRun contains the run-time state of setZoneConfigNode during local execution.
-type setZoneConfigRun struct {
-	numAffected int
-}
-
 // ReadingOwnWrites implements the planNodeReadingOwnWrites interface.
 // This is because CONFIGURE ZONE performs multiple KV operations on descriptors
 // and expects to see its own writes.
@@ -747,7 +740,7 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
 		zoneToWrite := partialZone
 		// TODO(ajwerner): This is extremely fragile because we accept a nil table
 		// all the way down here.
-		n.run.numAffected, err = writeZoneConfig(
+		err = writeZoneConfig(
 			params.ctx,
 			params.p.InternalSQLTxn(),
 			targetID,
@@ -796,8 +789,6 @@ func (n *setZoneConfigNode) Next(runParams) (bool, error) { return false, nil }
 func (n *setZoneConfigNode) Values() tree.Datums          { return nil }
 func (*setZoneConfigNode) Close(context.Context)          {}
 
-func (n *setZoneConfigNode) FastPathResults() (int, bool) { return n.run.numAffected, true }
-
 type nodeGetter func(context.Context, *serverpb.NodesRequest) (*serverpb.NodesResponse, error)
 type regionsGetter func(context.Context) (*serverpb.RegionsResponse, error)
 
@@ -1058,41 +1049,38 @@ func writeZoneConfig(
 	execCfg *ExecutorConfig,
 	hasNewSubzones bool,
 	kvTrace bool,
-) (numAffected int, err error) {
+) error {
 	update, err := prepareZoneConfigWrites(ctx, execCfg, targetID, table, zone, expectedExistingRawBytes, hasNewSubzones)
 	if err != nil {
-		return 0, err
+		return err
 	}
 	return writeZoneConfigUpdate(ctx, txn, kvTrace, update)
 }
 
 func writeZoneConfigUpdate(
 	ctx context.Context, txn descs.Txn, kvTrace bool, update *zoneConfigUpdate,
-) (numAffected int, err error) {
+) error {
 	b := txn.KV().NewBatch()
 	if update.zoneConfig == nil {
-		err = txn.Descriptors().DeleteZoneConfigInBatch(ctx, kvTrace, b, update.id)
+		err := txn.Descriptors().DeleteZoneConfigInBatch(ctx, kvTrace, b, update.id)
+		if err != nil {
+			return err
+		}
 	} else {
-		numAffected = 1
-		err = txn.Descriptors().WriteZoneConfigToBatch(ctx, kvTrace, b, update.id, update.zoneConfig)
-	}
-	if err != nil {
-		return 0, err
+		err := txn.Descriptors().WriteZoneConfigToBatch(ctx, kvTrace, b, update.id, update.zoneConfig)
+		if err != nil {
+			return err
+		}
 	}
 
 	if err := txn.KV().Run(ctx, b); err != nil {
-		return 0, err
+		return err
 	}
 	r := b.Results[0]
 	if r.Err != nil {
 		panic("run succeeded even through the result has an error")
 	}
-	// We don't really care how many keys are affected since this function always
-	// write one single zone config.
-	if len(r.Keys) > 0 {
-		numAffected = 1
-	}
-	return numAffected, err
+	return nil
 }
 
 // RemoveIndexZoneConfigs removes the zone configurations for some
@@ -1139,7 +1127,7 @@ func RemoveIndexZoneConfigs(
 
 	if zcRewriteNecessary {
 		// Ignore CCL required error to allow schema change to progress.
-		_, err = writeZoneConfig(
+		err = writeZoneConfig(
 			ctx, txn, tableDesc.GetID(), tableDesc, zone,
 			zoneWithRaw.GetRawBytesInStorage(), execCfg,
 			false /* hasNewSubzones */, kvTrace,

pkg/sql/zone_config.go

@@ -512,6 +512,5 @@ func deleteRemovedPartitionZoneConfigs(
 	if update == nil || err != nil {
 		return err
 	}
-	_, err = writeZoneConfigUpdate(ctx, txn, kvTrace, update)
-	return err
+	return writeZoneConfigUpdate(ctx, txn, kvTrace, update)
 }