Merge #71670 #71684

71670: tree: make "rename" stmt tags match postgres r=otan a=rafiss This makes ALTER statements that rename objects use the correct statement tag to match Postgres. Release note: None 71684: server: add assertion around binary versions r=irfansharif a=irfansharif Makes for a more helpful error message in #69828; there we're dealing with tests that override the binary version that now fail seeing as how the min supported version is advanced. Release note: None Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com> Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
cockroachdb · Oct 19, 2021 · 0e2e645 · 0e2e645
3 parents 5e001e2 + 25a51e1 + 475a110
commit 0e2e645
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 14 deletions.
diff --git a/pkg/server/init.go b/pkg/server/init.go
@@ -616,14 +616,21 @@ type initServerCfg struct {
 	testingKnobs base.TestingKnobs
 }
 
-func newInitServerConfig(cfg Config, dialOpts []grpc.DialOption) initServerCfg {
+func newInitServerConfig(
+	ctx context.Context, cfg Config, dialOpts []grpc.DialOption,
+) initServerCfg {
 	binaryVersion := cfg.Settings.Version.BinaryVersion()
 	if knobs := cfg.TestingKnobs.Server; knobs != nil {
 		if ov := knobs.(*TestingKnobs).BinaryVersionOverride; ov != (roachpb.Version{}) {
 			binaryVersion = ov
 		}
 	}
 	binaryMinSupportedVersion := cfg.Settings.Version.BinaryMinSupportedVersion()
+	if binaryVersion.Less(binaryMinSupportedVersion) {
+		log.Fatalf(ctx, "binary version (%s) less than min supported version (%s)",
+			binaryVersion, binaryMinSupportedVersion)
+	}
+
 	bootstrapAddresses := cfg.FilterGossipBootstrapAddresses(context.Background())
 	return initServerCfg{
 		advertiseAddr:             cfg.AdvertiseAddr,

diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -1277,7 +1277,7 @@ func (s *Server) PreStart(ctx context.Context) error {
 			return err
 		}
 
-		initConfig := newInitServerConfig(s.cfg, dialOpts)
+		initConfig := newInitServerConfig(ctx, s.cfg, dialOpts)
 		inspectedDiskState, err := inspectEngines(
 			ctx,
 			s.engines,

diff --git a/pkg/server/settings_cache_test.go b/pkg/server/settings_cache_test.go
@@ -59,6 +59,7 @@ func TestCachedSettingsServerRestart(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	ctx := context.Background()
 	stickyEngineRegistry := NewStickyInMemEnginesRegistry()
 	defer stickyEngineRegistry.CloseAllStickyInMemEngines()
 
@@ -105,7 +106,7 @@ func TestCachedSettingsServerRestart(t *testing.T) {
 		dialOpts, err := s.rpcContext.GRPCDialOptions()
 		require.NoError(t, err)
 
-		initConfig := newInitServerConfig(s.cfg, dialOpts)
+		initConfig := newInitServerConfig(ctx, s.cfg, dialOpts)
 		inspectState, err := inspectEngines(
 			context.Background(),
 			s.engines,

diff --git a/pkg/sql/logictest/testdata/logic_test/event_log b/pkg/sql/logictest/testdata/logic_test/event_log
@@ -305,7 +305,7 @@ FROM system.eventlog
 WHERE "eventType" = 'rename_table'
   AND info::JSONB->>'Statement' LIKE 'ALTER TABLE %toberenamed% RENAME TO %renamedtable%'
 ----
-1  {"EventType": "rename_table", "NewTableName": "test.public.renamedtable", "Statement": "ALTER TABLE toberenamed RENAME TO renamedtable", "TableName": "test.public.toberenamed", "Tag": "RENAME TABLE", "User": "root"}
+1  {"EventType": "rename_table", "NewTableName": "test.public.renamedtable", "Statement": "ALTER TABLE toberenamed RENAME TO renamedtable", "TableName": "test.public.toberenamed", "Tag": "ALTER TABLE", "User": "root"}
 
 
 ##################
@@ -413,7 +413,7 @@ FROM system.eventlog
 WHERE "eventType" = 'rename_database'
   AND info::JSONB->>'Statement' LIKE 'ALTER DATABASE %eventlogtorename% RENAME TO %eventlogtonewname%'
 ----
-1  {"DatabaseName": "eventlogtorename", "EventType": "rename_database", "NewDatabaseName": "eventlogtonewname", "Statement": "ALTER DATABASE eventlogtorename RENAME TO eventlogtonewname", "Tag": "RENAME DATABASE", "User": "root"}
+1  {"DatabaseName": "eventlogtorename", "EventType": "rename_database", "NewDatabaseName": "eventlogtonewname", "Statement": "ALTER DATABASE eventlogtorename RENAME TO eventlogtonewname", "Tag": "ALTER DATABASE", "User": "root"}
 
 statement ok
 SET DATABASE = test

diff --git a/pkg/sql/logictest/testdata/logic_test/rename_database b/pkg/sql/logictest/testdata/logic_test/rename_database
@@ -187,7 +187,7 @@ EXPLAIN ALTER DATABASE v RENAME TO x
 distribution: local
 vectorized: true
 ·
-• rename database
+• alter database
 
 statement ok
 RESET vectorize

diff --git a/pkg/sql/logictest/testdata/logic_test/rename_index b/pkg/sql/logictest/testdata/logic_test/rename_index
@@ -157,7 +157,7 @@ EXPLAIN ALTER INDEX users@bar RENAME TO woo
 distribution: local
 vectorized: true
 ·
-• rename index
+• alter index
 
 statement ok
 RESET vectorize

diff --git a/pkg/sql/logictest/testdata/logic_test/rename_table b/pkg/sql/logictest/testdata/logic_test/rename_table
@@ -260,7 +260,7 @@ EXPLAIN ALTER TABLE kv RENAME TO kv2
 distribution: local
 vectorized: true
 ·
-• rename table
+• alter table
 
 statement ok
 RESET vectorize

diff --git a/pkg/sql/sem/tree/stmt.go b/pkg/sql/sem/tree/stmt.go
@@ -916,7 +916,7 @@ func (*RenameColumn) StatementReturnType() StatementReturnType { return DDL }
 func (*RenameColumn) StatementType() StatementType { return TypeDDL }
 
 // StatementTag returns a short string identifying the type of statement.
-func (*RenameColumn) StatementTag() string { return "RENAME COLUMN" }
+func (*RenameColumn) StatementTag() string { return "ALTER TABLE" }
 
 // StatementReturnType implements the Statement interface.
 func (*RenameDatabase) StatementReturnType() StatementReturnType { return DDL }
@@ -925,7 +925,7 @@ func (*RenameDatabase) StatementReturnType() StatementReturnType { return DDL }
 func (*RenameDatabase) StatementType() StatementType { return TypeDDL }
 
 // StatementTag returns a short string identifying the type of statement.
-func (*RenameDatabase) StatementTag() string { return "RENAME DATABASE" }
+func (*RenameDatabase) StatementTag() string { return "ALTER DATABASE" }
 
 // StatementReturnType implements the Statement interface.
 func (*ReparentDatabase) StatementReturnType() StatementReturnType { return DDL }
@@ -943,7 +943,7 @@ func (*RenameIndex) StatementReturnType() StatementReturnType { return DDL }
 func (*RenameIndex) StatementType() StatementType { return TypeDDL }
 
 // StatementTag returns a short string identifying the type of statement.
-func (*RenameIndex) StatementTag() string { return "RENAME INDEX" }
+func (*RenameIndex) StatementTag() string { return "ALTER INDEX" }
 
 // StatementReturnType implements the Statement interface.
 func (*RenameTable) StatementReturnType() StatementReturnType { return DDL }
@@ -954,11 +954,11 @@ func (*RenameTable) StatementType() StatementType { return TypeDDL }
 // StatementTag returns a short string identifying the type of statement.
 func (n *RenameTable) StatementTag() string {
 	if n.IsView {
-		return "RENAME VIEW"
+		return "ALTER VIEW"
 	} else if n.IsSequence {
-		return "RENAME SEQUENCE"
+		return "ALTER SEQUENCE"
 	}
-	return "RENAME TABLE"
+	return "ALTER TABLE"
 }
 
 // StatementReturnType implements the Statement interface.