Merge #70952 #71303

70952: sql: refactored diff tool strucutres to remove JSON complexity r=RichardJCai a=mnovelodou

Previously, diff tool shared the same structures for table metadata
and diffs
This was inadequate because they have different concerns
To address this, this patch refactored the code so every concern or
process have their own data types

Release note: None

71303: storage: always close iterators, batches in unit tests r=sumeerbhola a=jbowens

This fixes a handful of instances where iterators and batches either
weren't closed, or weren't closed if the test failed because the Close
wasn't deferred.

There are more in the codebase, but I wanted to break this up a bit
since some of them are trickier to track down.

Release note: None

Co-authored-by: mnovelodou <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

pkg/cmd/generate-metadata-tables/main.go

@@ -15,11 +15,15 @@
 //
 // This accepts the following arguments:
 //
-// -user: to change default pg username of `postgres`
-// -addr: to change default pg address of `localhost:5432`
+// --user:    to change default pg username of `postgres`
+// --addr:    to change default pg address of `localhost:5432`
+// --catalog: can be pg_catalog or information_schema. Default is pg_catalog
+// --rdbms:   can be postgres or mysql. Default is postgres
+// --stdout:  for testing purposes, use this flag to send the output to the
+//            console
 //
-// Output of this file should generate:
-// pkg/sql/testdata/pg_catalog_tables
+// Output of this file should generate (If not using --stout):
+// pkg/sql/testdata/<catalog>_tables_from_<rdbms>.json
 package main
 
 import (
@@ -59,7 +63,7 @@ func main() {
 		panic(err)
 	}
 	pgCatalogFile := &sql.PGMetadataFile{
-		PGVersion:  dbVersion,
+		Version:    dbVersion,
 		PGMetadata: sql.PGMetadataTables{},
 	}
 
@@ -70,16 +74,13 @@ func main() {
 
 	rows.ForEachRow(func(tableName, columnName, dataTypeName string, dataTypeOid uint32) {
 		pgCatalogFile.PGMetadata.AddColumnMetadata(tableName, columnName, dataTypeName, dataTypeOid)
-		columnType := pgCatalogFile.PGMetadata[tableName][columnName]
-		if dataTypeOid != 0 && !columnType.IsImplemented() {
-			pgCatalogFile.AddUnimplementedType(columnType)
-		}
 	})
 
-	writer, err := getWriter()
+	writer, closeFn, err := getWriter()
 	if err != nil {
 		panic(err)
 	}
+	defer closeFn()
 	pgCatalogFile.Save(writer)
 }
 
@@ -102,11 +103,12 @@ func testdata() string {
 	return testdataDir
 }
 
-func getWriter() (io.Writer, error) {
+func getWriter() (io.Writer, func(), error) {
 	if *flagStdout {
-		return os.Stdout, nil
+		return os.Stdout, func() {}, nil
 	}
 
 	filename := sql.TablesMetadataFilename(testdata(), *flagRDBMS, *flagSchema)
-	return os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
+	file, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
+	return file, func() { file.Close() }, err
 }

pkg/kv/kvserver/rangefeed/catchup_scan_bench_test.go

@@ -44,24 +44,27 @@ func runCatchUpBenchmark(b *testing.B, emk engineMaker, opts benchOptions) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		iter := rangefeed.NewCatchUpIterator(eng, &roachpb.RangeFeedRequest{
-			Header: roachpb.Header{
-				Timestamp: opts.ts,
-			},
-			WithDiff: opts.withDiff,
-			Span:     span,
-		}, opts.useTBI, func() {})
-		counter := 0
-		err := iter.CatchUpScan(storage.MakeMVCCMetadataKey(startKey), storage.MakeMVCCMetadataKey(endKey), opts.ts, opts.withDiff, func(*roachpb.RangeFeedEvent) error {
-			counter++
-			return nil
-		})
-		if err != nil {
-			b.Fatalf("failed catchUp scan: %+v", err)
-		}
-		if counter < 1 {
-			b.Fatalf("didn't emit any events!")
-		}
+		func() {
+			iter := rangefeed.NewCatchUpIterator(eng, &roachpb.RangeFeedRequest{
+				Header: roachpb.Header{
+					Timestamp: opts.ts,
+				},
+				WithDiff: opts.withDiff,
+				Span:     span,
+			}, opts.useTBI, func() {})
+			defer iter.Close()
+			counter := 0
+			err := iter.CatchUpScan(storage.MakeMVCCMetadataKey(startKey), storage.MakeMVCCMetadataKey(endKey), opts.ts, opts.withDiff, func(*roachpb.RangeFeedEvent) error {
+				counter++
+				return nil
+			})
+			if err != nil {
+				b.Fatalf("failed catchUp scan: %+v", err)
+			}
+			if counter < 1 {
+				b.Fatalf("didn't emit any events!")
+			}
+		}()
 	}
 }