From 0dc107d03a69bf48a30e37967e8902ebefa1e4a8 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Fri, 10 Feb 2023 16:16:31 -0800
Subject: [PATCH 01/40] Added a check for non-empty table on schema update

---
 go/cmd/dolt/commands/schcmds/import.go | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/go/cmd/dolt/commands/schcmds/import.go b/go/cmd/dolt/commands/schcmds/import.go
index 101fe5aebe..724cce5e0c 100644
--- a/go/cmd/dolt/commands/schcmds/import.go
+++ b/go/cmd/dolt/commands/schcmds/import.go
@@ -145,7 +145,7 @@ func (cmd ImportCmd) ArgParser() *argparser.ArgParser {
 	ap.ArgListHelp = append(ap.ArgListHelp, [2]string{"table", "Name of the table to be created."})
 	ap.ArgListHelp = append(ap.ArgListHelp, [2]string{"file", "The file being used to infer the schema."})
 	ap.SupportsFlag(createFlag, "c", "Create a table with the schema inferred from the {{.LessThan}}file{{.GreaterThan}} provided.")
-	ap.SupportsFlag(updateFlag, "u", "Update a table to match the inferred schema of the {{.LessThan}}file{{.GreaterThan}} provided")
+	ap.SupportsFlag(updateFlag, "u", "Update a table to match the inferred schema of the {{.LessThan}}file{{.GreaterThan}} provided. All previous data will be lost.")
 	ap.SupportsFlag(replaceFlag, "r", "Replace a table with a new schema that has the inferred schema from the {{.LessThan}}file{{.GreaterThan}} provided. All previous data will be lost.")
 	ap.SupportsFlag(dryRunFlag, "", "Print the sql statement that would be run if executed without the flag.")
 	ap.SupportsFlag(keepTypesParam, "", "When a column already exists in the table, and it's also in the {{.LessThan}}file{{.GreaterThan}} provided, use the type from the table.")
@@ -219,6 +219,23 @@ func getSchemaImportArgs(ctx context.Context, apr *argparser.ArgParseResults, dE
 		return nil, errhand.BuildDError("error: failed to create table.").AddDetails("A table named '%s' already exists.", tblName).AddDetails("Use --replace or --update instead of --create.").Build()
 	}
 
+	if op != CreateOp {
+		rows, err := tbl.GetRowData(ctx)
+		if err != nil {
+			return nil, errhand.VerboseErrorFromError(err)
+		}
+
+		rowCnt, err := rows.Count()
+		if err != nil {
+			return nil, errhand.VerboseErrorFromError(err)
+		}
+
+		if rowCnt > 0 {
+			return nil, errhand.BuildDError("This operation will delete all row data. If this is your intent, " +
+				"run dolt sql -q 'delete from %s' to delete all row data, then re-run this command.", tblName).Build()
+		}
+	}
+
 	var existingSch schema.Schema = schema.EmptySchema
 	if tblExists {
 		existingSch, err = tbl.GetSchema(ctx)

From c4ba57e7a5ceac15eb2ee322135487412c190d61 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Fri, 10 Feb 2023 16:33:12 -0800
Subject: [PATCH 02/40] New bats tests

---
 integration-tests/bats/schema-import.bats | 37 +++++++++++++++++++++--
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/integration-tests/bats/schema-import.bats b/integration-tests/bats/schema-import.bats
index 83ffb3e5ae..0c78d8a2a0 100755
--- a/integration-tests/bats/schema-import.bats
+++ b/integration-tests/bats/schema-import.bats
@@ -252,6 +252,8 @@ DELIM
 
 @test "schema-import: --update adds new columns" {
     dolt table import -c -pk=pk test abc.csv
+    dolt sql -q 'delete from test'
+
     dolt add test
     dolt commit -m "added table"
     run dolt schema import -pks=pk -u test abc-xyz.csv
@@ -268,13 +270,26 @@ DELIM
     run dolt sql -r csv -q 'select * from test'
     [ "$status" -eq 0 ]
     [[ "$output" =~ "pk,a,b,c,x,y,z" ]] || false
-    skip "schema import --update is currently deleting table data"
-    [[ "$output" =~ "0,red,1.1,true,,," ]] || false
-    [[ "$output" =~ "1,blue,2.2,false,,," ]] || false
+}
+
+@test "schema-import: --update blocked on non-empty table" {
+    dolt table import -c -pk=pk test abc.csv
+    dolt add test
+    dolt commit -m "added table"
+    
+    run dolt schema import -pks=pk -u test abc-xyz.csv
+    [ "$status" -eq 1 ]
+    [[ "$output" =~ "will delete all row data" ]] || false
+    [[ "$output" =~ "dolt sql -q 'delete from test'" ]] || false
+
+    dolt sql -q 'delete from test'
+    dolt schema import -pks=pk -u test abc-xyz.csv
 }
 
 @test "schema-import: --replace adds new columns" {
     dolt table import -c -pk=pk test abc.csv
+    dolt sql -q 'delete from test'
+
     dolt add test
     dolt commit -m "added table"
     run dolt schema import -pks=pk -r test abc-xyz.csv
@@ -294,6 +309,20 @@ DELIM
     [[ "$output" =~ "0" ]] || false
 }
 
+@test "schema-import: --replace blocked on non-empty table" {
+    dolt table import -c -pk=pk test abc.csv
+    dolt add test
+    dolt commit -m "added table"
+
+    run dolt schema import -pks=pk -r test abc-xyz.csv
+    [ "$status" -eq 1 ]
+    [[ "$output" =~ "will delete all row data" ]] || false
+    [[ "$output" =~ "dolt sql -q 'delete from test'" ]] || false
+
+    dolt sql -q 'delete from test'    
+    dolt schema import -pks=pk -u test abc-xyz.csv
+}
+
 @test "schema-import: --replace drops missing columns" {
     cat <<DELIM > xyz.csv
 pk,x,y,z
@@ -302,6 +331,8 @@ pk,x,y,z
 DELIM
     dolt table import -c -pk=pk test abc-xyz.csv
     dolt add test
+    dolt sql -q 'delete from test'
+
     dolt commit -m "added test"
     run dolt schema import -pks=pk -r test xyz.csv
     [ "$status" -eq 0 ]

From df6a0dd8ab91f8a1eefbe985869a2567d0accc97 Mon Sep 17 00:00:00 2001
From: zachmu <zachmu@users.noreply.github.com>
Date: Sat, 11 Feb 2023 00:35:34 +0000
Subject: [PATCH 03/40] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and
 go/Godeps/update.sh

---
 go/cmd/dolt/commands/schcmds/import.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/cmd/dolt/commands/schcmds/import.go b/go/cmd/dolt/commands/schcmds/import.go
index 724cce5e0c..f2dd62b59a 100644
--- a/go/cmd/dolt/commands/schcmds/import.go
+++ b/go/cmd/dolt/commands/schcmds/import.go
@@ -231,7 +231,7 @@ func getSchemaImportArgs(ctx context.Context, apr *argparser.ArgParseResults, dE
 		}
 
 		if rowCnt > 0 {
-			return nil, errhand.BuildDError("This operation will delete all row data. If this is your intent, " +
+			return nil, errhand.BuildDError("This operation will delete all row data. If this is your intent, "+
 				"run dolt sql -q 'delete from %s' to delete all row data, then re-run this command.", tblName).Build()
 		}
 	}

From 938ea2d4dfa00e45233d59357d6a4011f230f3dc Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Tue, 14 Feb 2023 18:44:21 -0800
Subject: [PATCH 04/40] Started defining new schema for dolt_schemas

---
 go/libraries/doltcore/doltdb/system_table.go  |  14 +-
 .../doltcore/schema/typeinfo/varstring.go     |   4 +
 go/libraries/doltcore/sqle/schema_table.go    | 136 +++++++++++++-----
 3 files changed, 111 insertions(+), 43 deletions(-)

diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go
index b88a0a8fb0..525e696467 100644
--- a/go/libraries/doltcore/doltdb/system_table.go
+++ b/go/libraries/doltcore/doltdb/system_table.go
@@ -224,17 +224,19 @@ const (
 	// SchemasTableName is the name of the dolt schema fragment table
 	SchemasTableName = "dolt_schemas"
 	// SchemasTablesIdCol is an incrementing integer that represents the insertion index.
+	// Deprecated: This column is no longer used and will be removed in a future release.
 	SchemasTablesIdCol = "id"
-	// Currently: `view` or `trigger`.
+	// SchemasTablesTypeCol is the name of the column that stores the type of a schema fragment  in the dolt_schemas table
 	SchemasTablesTypeCol = "type"
-	// The name of the database entity.
+	// SchemasTablesNameCol The name of the column that stores the name of a schema fragment in the dolt_schemas table
 	SchemasTablesNameCol = "name"
-	// The schema fragment associated with the database entity.
-	// For example, the SELECT statement for a CREATE VIEW.
+	// SchemasTablesFragmentCol The name of the column that stores the SQL fragment of a schema element in the 
+	// dolt_schemas table
 	SchemasTablesFragmentCol = "fragment"
-	// The extra information for schema; currently contains creation time for triggers and views
+	// SchemasTablesExtraCol The name of the column that stores extra information about a schema element in the
+	// dolt_schemas table
 	SchemasTablesExtraCol = "extra"
-	// The name of the index that is on the table.
+	// 
 	SchemasTablesIndexName = "fragment_name"
 )
 
diff --git a/go/libraries/doltcore/schema/typeinfo/varstring.go b/go/libraries/doltcore/schema/typeinfo/varstring.go
index eb59d519f2..a6adeae1ba 100644
--- a/go/libraries/doltcore/schema/typeinfo/varstring.go
+++ b/go/libraries/doltcore/schema/typeinfo/varstring.go
@@ -52,6 +52,10 @@ var (
 	StringDefaultType = &varStringType{gmstypes.MustCreateStringWithDefaults(sqltypes.VarChar, 16383)}
 )
 
+func CreateVarStringTypeFromSqlType(stringType sql.StringType) TypeInfo {
+	return &varStringType{stringType}
+}
+
 func CreateVarStringTypeFromParams(params map[string]string) (TypeInfo, error) {
 	var length int64
 	var collation sql.CollationID
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index efc1a8ed0e..a4c9b9ed5c 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/dolthub/go-mysql-server/sql"
 	gmstypes "github.com/dolthub/go-mysql-server/sql/types"
+	"github.com/dolthub/vitess/go/vt/proto/query"
 
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable"
@@ -33,7 +34,6 @@ import (
 )
 
 var errDoltSchemasTableFormat = fmt.Errorf("`%s` schema in unexpected format", doltdb.SchemasTableName)
-var noSchemaIndexDefined = fmt.Errorf("could not find index `%s` on system table `%s`", doltdb.SchemasTablesIndexName, doltdb.SchemasTableName)
 
 const (
 	viewFragment    = "view"
@@ -44,6 +44,46 @@ type Extra struct {
 	CreatedAt int64
 }
 
+func mustNewColWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...schema.ColConstraint) schema.Column {
+	col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo,partOfPK, defaultVal, autoIncrement, comment, constraints...)
+	if err != nil {
+		panic(err)
+	}
+	return col
+}
+
+func mustCreateStringType(baseType query.Type, length int64, collation sql.CollationID) sql.StringType {
+	ti, err := gmstypes.CreateString(baseType, length, collation)
+	if err != nil {
+		panic(err)
+	}
+	return ti
+}
+
+// old dolt_schemas columns, only used for migration
+var schemasTableColsWithId = schema.NewColCollection(
+	mustNewColWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.StringDefaultType, false, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.StringDefaultType, false, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.StringDefaultType, false, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesIdCol, schema.DoltSchemasIdTag, typeinfo.Int64Type, true, "", false, "", schema.NotNullConstraint{}),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), 
+)
+
+var schemaTableWithIdColSchema = schema.MustSchemaFromCols(schemasTableColsWithId)
+
+// dolt_schemas columns
+var schemasTableCols = schema.NewColCollection(
+	mustNewColWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.CreateVarStringTypeFromSqlType(gmstypes.LongText), false, "", false, ""),
+	mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""),
+)
+
+var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols)
+var schemaTableKd = schemaTableSchema.GetKeyDescriptor()
+var schemaTabldVd = schemaTableSchema.GetValueDescriptor()
+
+
 // The fixed dolt schema for the `dolt_schemas` table.
 func SchemasTableSchema() schema.Schema {
 	typeCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.StringDefaultType, false, "", false, "")
@@ -79,8 +119,8 @@ func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 	var rowsToAdd []sql.Row
 	if found {
 		schemasTable := tbl.(*WritableDoltTable)
-		// Old schemas table does not contain the `id` or `extra` column.
-		if !tbl.Schema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) || !tbl.Schema().Contains(doltdb.SchemasTablesExtraCol, doltdb.SchemasTableName) {
+		// Old schemas table contains the `id` column.
+		if tbl.Schema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) {
 			root, err := db.GetRoot(ctx)
 			if err != nil {
 				return nil, err
@@ -165,40 +205,61 @@ func migrateOldSchemasTableToNew(
 	[]sql.Row,
 	error,
 ) {
-	// Copy all of the old data over and add an index column and an extra column
-	var rowsToAdd []sql.Row
+
 	table, err := schemasTable.DoltTable.DoltTable(ctx)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	rowData, err := table.GetNomsRowData(ctx)
+	rowData, err := table.GetRowData(ctx)
 	if err != nil {
 		return nil, nil, err
 	}
-	id := int64(1)
-	err = rowData.IterAll(ctx, func(key, val types.Value) error {
-		dRow, err := row.FromNoms(schemasTable.sch, key.(types.Tuple), val.(types.Tuple))
+
+	// Copy all of the old row data over minus the id column
+	var rowsToAdd []sql.Row
+	if types.IsFormat_DOLT(rowData.Format()) {
+		pm := durable.ProllyMapFromIndex(rowData)
+		iter, err := pm.IterAll(ctx)
 		if err != nil {
-			return err
+			return nil, nil, err
 		}
-		sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, schemasTable.sch)
+		
+		for {
+			_, v, err := iter.Next(ctx)
+			if err != nil {
+				return nil, nil, err
+			}
+			
+			
+		}
+	} else {
+		nomsMap := durable.NomsMapFromIndex(rowData)
+		id := int64(1)
+		err = nomsMap.IterAll(ctx, func(key, val types.Value) error {
+			dRow, err := row.FromNoms(schemasTable.sch, key.(types.Tuple), val.(types.Tuple))
+			if err != nil {
+				return err
+			}
+			sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, schemasTable.sch)
+			if err != nil {
+				return err
+			}
+			// append the new id to row, if missing
+			if !schemasTable.sqlSchema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) {
+				sqlRow = append(sqlRow, id)
+			}
+			// append the extra cols to row
+			sqlRow = append(sqlRow, nil)
+			rowsToAdd = append(rowsToAdd, sqlRow)
+			id++
+			return nil
+		})
 		if err != nil {
-			return err
+			return nil, nil, err
 		}
-		// append the new id to row, if missing
-		if !schemasTable.sqlSchema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) {
-			sqlRow = append(sqlRow, id)
-		}
-		// append the extra cols to row
-		sqlRow = append(sqlRow, nil)
-		rowsToAdd = append(rowsToAdd, sqlRow)
-		id++
-		return nil
-	})
-	if err != nil {
-		return nil, nil, err
 	}
+	
 	err = db.DropTable(ctx, doltdb.SchemasTableName)
 	if err != nil {
 		return nil, nil, err
@@ -256,20 +317,21 @@ func nextSchemasTableIndex(ctx *sql.Context, root *doltdb.RootValue) (int64, err
 
 // fragFromSchemasTable returns the row with the given schema fragment if it exists.
 func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType string, name string) (sql.Row, bool, error) {
-	indexes, err := tbl.GetIndexes(ctx)
-	if err != nil {
-		return nil, false, err
-	}
+	// indexes, err := tbl.GetIndexes(ctx)
+	// if err != nil {
+	// 	return nil, false, err
+	// }
 	var fragNameIndex sql.Index
-	for _, index := range indexes {
-		if index.ID() == doltdb.SchemasTablesIndexName {
-			fragNameIndex = index
-			break
-		}
-	}
-	if fragNameIndex == nil {
-		return nil, false, noSchemaIndexDefined
-	}
+	// TODO: replace with primary key lookup
+	// for _, index := range indexes {
+	// 	if index.ID() == doltdb.SchemasTablesIndexName {
+	// 		fragNameIndex = index
+	// 		break
+	// 	}
+	// }
+	// if fragNameIndex == nil {
+	// 	return nil, false, noSchemaIndexDefined
+	// }
 
 	exprs := fragNameIndex.Expressions()
 	lookup, err := sql.NewIndexBuilder(fragNameIndex).Equals(ctx, exprs[0], fragType).Equals(ctx, exprs[1], name).Build(ctx)

From 89534b025082c5e69ee8a97c27fa9e426989a036 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 10:17:59 -0800
Subject: [PATCH 05/40] Rip out migration code

---
 go/libraries/doltcore/sqle/database.go     |  22 +-
 go/libraries/doltcore/sqle/schema_table.go | 237 ++-------------------
 2 files changed, 15 insertions(+), 244 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index a3491b5a26..af91d6da35 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -1281,25 +1281,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if exists {
 		return existingErr
 	}
-
-	sess := dsess.DSessFromSess(ctx.Session)
-	dbState, _, err := sess.LookupDbState(ctx, db.Name())
-	if err != nil {
-		return err
-	}
-	ts := dbState.WriteSession
-
-	ws, err := ts.Flush(ctx)
-	if err != nil {
-		return err
-	}
-
-	// If rows exist, then grab the highest id and add 1 to get the new id
-	idx, err := nextSchemasTableIndex(ctx, ws.WorkingRoot())
-	if err != nil {
-		return err
-	}
-
+	
 	// Insert the new row into the db
 	inserter := tbl.Inserter(ctx)
 	defer func() {
@@ -1316,7 +1298,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if err != nil {
 		return err
 	}
-	return inserter.Insert(ctx, sql.Row{fragType, name, definition, idx, extraJSON})
+	return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON})
 }
 
 func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name string, missingErr error) error {
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index a4c9b9ed5c..bbd27a30b3 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -24,13 +24,9 @@ import (
 	"github.com/dolthub/vitess/go/vt/proto/query"
 
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
-	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable"
-	"github.com/dolthub/dolt/go/libraries/doltcore/row"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema/typeinfo"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/index"
-	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil"
-	"github.com/dolthub/dolt/go/store/types"
 )
 
 var errDoltSchemasTableFormat = fmt.Errorf("`%s` schema in unexpected format", doltdb.SchemasTableName)
@@ -60,17 +56,6 @@ func mustCreateStringType(baseType query.Type, length int64, collation sql.Colla
 	return ti
 }
 
-// old dolt_schemas columns, only used for migration
-var schemasTableColsWithId = schema.NewColCollection(
-	mustNewColWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.StringDefaultType, false, "", false, ""),
-	mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.StringDefaultType, false, "", false, ""),
-	mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.StringDefaultType, false, "", false, ""),
-	mustNewColWithTypeInfo(doltdb.SchemasTablesIdCol, schema.DoltSchemasIdTag, typeinfo.Int64Type, true, "", false, "", schema.NotNullConstraint{}),
-	mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), 
-)
-
-var schemaTableWithIdColSchema = schema.MustSchemaFromCols(schemasTableColsWithId)
-
 // dolt_schemas columns
 var schemasTableCols = schema.NewColCollection(
 	mustNewColWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""),
@@ -81,33 +66,10 @@ var schemasTableCols = schema.NewColCollection(
 
 var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols)
 var schemaTableKd = schemaTableSchema.GetKeyDescriptor()
-var schemaTabldVd = schemaTableSchema.GetValueDescriptor()
+var schemaTableVd = schemaTableSchema.GetValueDescriptor()
 
-
-// The fixed dolt schema for the `dolt_schemas` table.
 func SchemasTableSchema() schema.Schema {
-	typeCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesTypeCol, schema.DoltSchemasTypeTag, typeinfo.StringDefaultType, false, "", false, "")
-	if err != nil {
-		panic(err)
-	}
-	nameCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.StringDefaultType, false, "", false, "")
-	if err != nil {
-		panic(err)
-	}
-	fragmentCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.StringDefaultType, false, "", false, "")
-	if err != nil {
-		panic(err)
-	}
-	idCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesIdCol, schema.DoltSchemasIdTag, typeinfo.Int64Type, true, "", false, "", schema.NotNullConstraint{})
-	if err != nil {
-		panic(err)
-	}
-	extraCol, err := schema.NewColumnWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, "")
-	if err != nil {
-		panic(err)
-	}
-	colColl := schema.NewColCollection(typeCol, nameCol, fragmentCol, idCol, extraCol)
-	return schema.MustSchemaFromCols(colColl)
+	return schemaTableSchema
 }
 
 // GetOrCreateDoltSchemasTable returns the `dolt_schemas` table in `db`, creating it if it does not already exist.
@@ -116,203 +78,30 @@ func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 	if err != nil {
 		return nil, err
 	}
-	var rowsToAdd []sql.Row
+
 	if found {
-		schemasTable := tbl.(*WritableDoltTable)
-		// Old schemas table contains the `id` column.
-		if tbl.Schema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) {
-			root, err := db.GetRoot(ctx)
-			if err != nil {
-				return nil, err
-			}
-			root, rowsToAdd, err = migrateOldSchemasTableToNew(ctx, db, root, schemasTable)
-			if err != nil {
-				return nil, err
-			}
-		} else {
-			return schemasTable, nil
-		}
+		return tbl.(*WritableDoltTable), nil
 	}
+
 	root, err := db.GetRoot(ctx)
 	if err != nil {
 		return nil, err
 	}
-	// Create the schemas table as an empty table
-	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, SchemasTableSchema())
-	if err != nil {
-		return nil, err
-	}
-	tbl, found, err = db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
-	if err != nil {
-		return nil, err
-	}
-	if !found {
-		return nil, sql.ErrTableNotFound.New("dolt_schemas")
-	}
-	// Create a unique index on the old primary key columns (type, name)
-	t := (&AlterableDoltTable{*tbl.(*WritableDoltTable)})
-	err = t.CreateIndex(ctx, sql.IndexDef{
-		Name: doltdb.SchemasTablesIndexName,
-		Columns: []sql.IndexColumn{
-			{Name: doltdb.SchemasTablesTypeCol, Length: 0},
-			{Name: doltdb.SchemasTablesNameCol, Length: 0},
-		},
-		Constraint: sql.IndexConstraint_Unique,
-		Storage:    sql.IndexUsing_Default,
-	})
-	if err != nil {
-		return nil, err
-	}
-	// If there was an old schemas table that contained rows, then add that data here
-	tbl, found, err = db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
-	if err != nil {
-		return nil, err
-	}
-	if !found {
-		return nil, sql.ErrTableNotFound.New("dolt_schemas")
-	}
-	if len(rowsToAdd) != 0 {
-		err = func() (retErr error) {
-			inserter := tbl.(*WritableDoltTable).Inserter(ctx)
-			defer func() {
-				err := inserter.Close(ctx)
-				if retErr == nil {
-					retErr = err
-				}
-			}()
-			for _, sqlRow := range rowsToAdd {
-				err = inserter.Insert(ctx, sqlRow)
-				if err != nil {
-					return err
-				}
-			}
-			return nil
-		}()
-		if err != nil {
-			return nil, err
-		}
-	}
-	return tbl.(*WritableDoltTable), nil
-}
-
-func migrateOldSchemasTableToNew(
-	ctx *sql.Context,
-	db Database,
-	root *doltdb.RootValue,
-	schemasTable *WritableDoltTable,
-) (
-	*doltdb.RootValue,
-	[]sql.Row,
-	error,
-) {
-
-	table, err := schemasTable.DoltTable.DoltTable(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	rowData, err := table.GetRowData(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	// Copy all of the old row data over minus the id column
-	var rowsToAdd []sql.Row
-	if types.IsFormat_DOLT(rowData.Format()) {
-		pm := durable.ProllyMapFromIndex(rowData)
-		iter, err := pm.IterAll(ctx)
-		if err != nil {
-			return nil, nil, err
-		}
-		
-		for {
-			_, v, err := iter.Next(ctx)
-			if err != nil {
-				return nil, nil, err
-			}
-			
-			
-		}
-	} else {
-		nomsMap := durable.NomsMapFromIndex(rowData)
-		id := int64(1)
-		err = nomsMap.IterAll(ctx, func(key, val types.Value) error {
-			dRow, err := row.FromNoms(schemasTable.sch, key.(types.Tuple), val.(types.Tuple))
-			if err != nil {
-				return err
-			}
-			sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, schemasTable.sch)
-			if err != nil {
-				return err
-			}
-			// append the new id to row, if missing
-			if !schemasTable.sqlSchema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) {
-				sqlRow = append(sqlRow, id)
-			}
-			// append the extra cols to row
-			sqlRow = append(sqlRow, nil)
-			rowsToAdd = append(rowsToAdd, sqlRow)
-			id++
-			return nil
-		})
-		if err != nil {
-			return nil, nil, err
-		}
-	}
 	
-	err = db.DropTable(ctx, doltdb.SchemasTableName)
+	// Create new empty table
+	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, schemaTableSchema)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-	root, err = db.GetRoot(ctx)
+	tbl, found, err = db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-	return root, rowsToAdd, nil
-}
-
-func nextSchemasTableIndex(ctx *sql.Context, root *doltdb.RootValue) (int64, error) {
-	tbl, _, err := root.GetTable(ctx, doltdb.SchemasTableName)
-	if err != nil {
-		return 0, err
+	if !found {
+		return nil, sql.ErrTableNotFound.New("dolt_schemas")
 	}
 
-	rows, err := tbl.GetRowData(ctx)
-	if err != nil {
-		return 0, err
-	}
-
-	empty, err := rows.Empty()
-	if err != nil {
-		return 0, err
-	}
-	if empty {
-		return 1, nil
-	}
-
-	if types.IsFormat_DOLT(tbl.Format()) {
-		p := durable.ProllyMapFromIndex(rows)
-		key := p.LastKey(ctx)
-		kd, _ := p.Descriptors()
-
-		i, _ := kd.GetInt64(0, key)
-		return i + 1, nil
-	} else {
-		m := durable.NomsMapFromIndex(rows)
-		keyTpl, _, err := m.Last(ctx)
-		if err != nil {
-			return 0, err
-		}
-		if keyTpl == nil {
-			return 1, nil
-		}
-
-		key, err := keyTpl.(types.Tuple).Get(1)
-		if err != nil {
-			return 0, err
-		}
-		return int64(key.(types.Int)) + 1, nil
-	}
+	return tbl.(*WritableDoltTable), nil
 }
 
 // fragFromSchemasTable returns the row with the given schema fragment if it exists.

From 4524b9b7f69b81fc03a22d8e74585ba28c18cf80 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 10:23:55 -0800
Subject: [PATCH 06/40] Fixed listing fragments of type

---
 go/libraries/doltcore/sqle/schema_table.go | 26 +++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index bbd27a30b3..a639e72be7 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -167,6 +167,12 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 		return nil, err
 	}
 
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+	// need to get the column indexes from the current schema
+	nameIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
+	fragmentIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol)
+	extraIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol)
+	
 	var frags []schemaFragment
 	for {
 		sqlRow, err := iter.Next(ctx)
@@ -177,34 +183,34 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 			return nil, err
 		}
 
-		if sqlRow[0] != fragType {
+		if sqlRow[fragmentIdx] != fragType {
 			continue
 		}
 
-		// For tables that haven't been converted yet or are filled with nil, use 1 as the trigger creation time
-		if len(sqlRow) < 5 || sqlRow[4] == nil {
+		// For older tables, use 1 as the trigger creation time
+		if sqlRow[extraIdx] == nil {
 			frags = append(frags, schemaFragment{
-				name:     sqlRow[1].(string),
-				fragment: sqlRow[2].(string),
+				name:     sqlRow[nameIdx].(string),
+				fragment: sqlRow[fragmentIdx].(string),
 				created:  time.Unix(1, 0).UTC(), // TablePlus editor thinks 0 is out of range
 			})
 			continue
 		}
 
 		// Extract Created Time from JSON column
-		createdTime, err := getCreatedTime(ctx, sqlRow)
+		createdTime, err := getCreatedTime(ctx, sqlRow[extraIdx].(gmstypes.JSONValue))
 
 		frags = append(frags, schemaFragment{
-			name:     sqlRow[1].(string),
-			fragment: sqlRow[2].(string),
+			name:     sqlRow[nameIdx].(string),
+			fragment: sqlRow[fragmentIdx].(string),
 			created:  time.Unix(createdTime, 0).UTC(),
 		})
 	}
 	return frags, nil
 }
 
-func getCreatedTime(ctx *sql.Context, row sql.Row) (int64, error) {
-	doc, err := row[4].(gmstypes.JSONValue).Unmarshall(ctx)
+func getCreatedTime(ctx *sql.Context, extraCol gmstypes.JSONValue) (int64, error) {
+	doc, err := extraCol.Unmarshall(ctx)
 	if err != nil {
 		return 0, err
 	}

From 01980e8bdfd47a7b41f84de383a29f77fdcfbca5 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 14:00:47 -0800
Subject: [PATCH 07/40] Simplified schema table lookups

---
 go/libraries/doltcore/sqle/schema_table.go | 76 ++++++++++------------
 go/libraries/doltcore/sqle/tables.go       |  2 +-
 2 files changed, 36 insertions(+), 42 deletions(-)

diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index a639e72be7..3067374dc3 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -17,6 +17,7 @@ package sqle
 import (
 	"fmt"
 	"io"
+	"strings"
 	"time"
 
 	"github.com/dolthub/go-mysql-server/sql"
@@ -26,11 +27,8 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema/typeinfo"
-	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/index"
 )
 
-var errDoltSchemasTableFormat = fmt.Errorf("`%s` schema in unexpected format", doltdb.SchemasTableName)
-
 const (
 	viewFragment    = "view"
 	triggerFragment = "trigger"
@@ -65,8 +63,6 @@ var schemasTableCols = schema.NewColCollection(
 )
 
 var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols)
-var schemaTableKd = schemaTableSchema.GetKeyDescriptor()
-var schemaTableVd = schemaTableSchema.GetValueDescriptor()
 
 func SchemasTableSchema() schema.Schema {
 	return schemaTableSchema
@@ -105,54 +101,43 @@ func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 }
 
 // fragFromSchemasTable returns the row with the given schema fragment if it exists.
-func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType string, name string) (sql.Row, bool, error) {
-	// indexes, err := tbl.GetIndexes(ctx)
-	// if err != nil {
-	// 	return nil, false, err
-	// }
-	var fragNameIndex sql.Index
-	// TODO: replace with primary key lookup
-	// for _, index := range indexes {
-	// 	if index.ID() == doltdb.SchemasTablesIndexName {
-	// 		fragNameIndex = index
-	// 		break
-	// 	}
-	// }
-	// if fragNameIndex == nil {
-	// 	return nil, false, noSchemaIndexDefined
-	// }
+func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType string, name string) (r sql.Row, found bool, rerr error) {
+	fragType, name = strings.ToLower(fragType), strings.ToLower(name)
 
-	exprs := fragNameIndex.Expressions()
-	lookup, err := sql.NewIndexBuilder(fragNameIndex).Equals(ctx, exprs[0], fragType).Equals(ctx, exprs[1], name).Build(ctx)
+	// This performs a full table scan in the worst case, but it's only used when adding or dropping a trigger or view
+	iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil)
 	if err != nil {
 		return nil, false, err
 	}
 
-	iter, err := index.RowIterForIndexLookup(ctx, tbl.DoltTable, lookup, tbl.sqlSch, nil)
-	if err != nil {
-		return nil, false, err
-	}
-
-	defer func() {
-		if cerr := iter.Close(ctx); cerr != nil {
-			err = cerr
+	defer func(iter sql.RowIter, ctx *sql.Context) {
+		err := iter.Close(ctx)
+		if err != nil && rerr == nil {
+			rerr = err
 		}
-	}()
-
-	// todo(andy): use filtered reader?
+	}(iter, ctx)
+	
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+	// need to get the column indexes from the current schema
+	nameIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
+	typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
+	
 	for {
 		sqlRow, err := iter.Next(ctx)
 		if err == io.EOF {
-			return nil, false, nil
+			break
 		}
 		if err != nil {
 			return nil, false, err
 		}
-		if sqlRow[0] != fragType || sqlRow[1] != name {
-			continue
+
+		// These columns are case insensitive, make sure to do a case-insensitive comparison
+		if strings.ToLower(sqlRow[typeIdx].(string)) == fragType && strings.ToLower(sqlRow[nameIdx].(string)) == name {
+			return sqlRow, true, nil
 		}
-		return sqlRow, true, nil
 	}
+	
+	return nil, false, nil
 }
 
 type schemaFragment struct {
@@ -161,7 +146,7 @@ type schemaFragment struct {
 	created  time.Time
 }
 
-func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType string) ([]schemaFragment, error) {
+func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType string) (sf []schemaFragment, rerr error) {
 	iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil)
 	if err != nil {
 		return nil, err
@@ -170,8 +155,16 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
 	// need to get the column indexes from the current schema
 	nameIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
+	typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
 	fragmentIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol)
 	extraIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol)
+
+	defer func(iter sql.RowIter, ctx *sql.Context) {
+		err := iter.Close(ctx)
+		if err != nil && rerr == nil {
+			rerr = err
+		}
+	}(iter, ctx)
 	
 	var frags []schemaFragment
 	for {
@@ -183,12 +176,12 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 			return nil, err
 		}
 
-		if sqlRow[fragmentIdx] != fragType {
+		if sqlRow[typeIdx] != fragType {
 			continue
 		}
 
 		// For older tables, use 1 as the trigger creation time
-		if sqlRow[extraIdx] == nil {
+		if extraIdx < 0 || sqlRow[extraIdx] == nil {
 			frags = append(frags, schemaFragment{
 				name:     sqlRow[nameIdx].(string),
 				fragment: sqlRow[fragmentIdx].(string),
@@ -206,6 +199,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 			created:  time.Unix(createdTime, 0).UTC(),
 		})
 	}
+	
 	return frags, nil
 }
 
diff --git a/go/libraries/doltcore/sqle/tables.go b/go/libraries/doltcore/sqle/tables.go
index 53c6f9f52c..4eafd6386b 100644
--- a/go/libraries/doltcore/sqle/tables.go
+++ b/go/libraries/doltcore/sqle/tables.go
@@ -80,7 +80,7 @@ type projected interface {
 // DoltTable implements the sql.Table interface and gives access to dolt table rows and schema.
 type DoltTable struct {
 	tableName    string
-	sqlSch       sql.PrimaryKeySchema
+	 sqlSch       sql.PrimaryKeySchema
 	db           SqlDatabase
 	lockedToRoot *doltdb.RootValue
 	nbf          *types.NomsBinFormat

From 55c65093c7459560702e0ea77fb673d42380343d Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 14:10:57 -0800
Subject: [PATCH 08/40] Inline schema method, fix a test

---
 go/libraries/doltcore/sqle/schema_table.go    | 4 ----
 go/libraries/doltcore/sqle/sqlddl_test.go     | 6 +++---
 go/libraries/doltcore/sqle/sqldelete_test.go  | 4 ++--
 go/libraries/doltcore/sqle/sqlinsert_test.go  | 6 +++---
 go/libraries/doltcore/sqle/sqlreplace_test.go | 4 ++--
 go/libraries/doltcore/sqle/sqlselect_test.go  | 4 ++--
 go/libraries/doltcore/sqle/sqlupdate_test.go  | 4 ++--
 7 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index 3067374dc3..de61b25d5c 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -64,10 +64,6 @@ var schemasTableCols = schema.NewColCollection(
 
 var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols)
 
-func SchemasTableSchema() schema.Schema {
-	return schemaTableSchema
-}
-
 // GetOrCreateDoltSchemasTable returns the `dolt_schemas` table in `db`, creating it if it does not already exist.
 func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *WritableDoltTable, retErr error) {
 	tbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
diff --git a/go/libraries/doltcore/sqle/sqlddl_test.go b/go/libraries/doltcore/sqle/sqlddl_test.go
index dfea5337c0..d5987c3712 100644
--- a/go/libraries/doltcore/sqle/sqlddl_test.go
+++ b/go/libraries/doltcore/sqle/sqlddl_test.go
@@ -807,15 +807,15 @@ func TestAlterSystemTables(t *testing.T) {
 		err := CreateEmptyTestTable(dEnv, "dolt_docs", doltdb.DocsSchema)
 		require.NoError(t, err)
 
-		err = CreateEmptyTestTable(dEnv, doltdb.SchemasTableName, SchemasTableSchema())
+		err = CreateEmptyTestTable(dEnv, doltdb.SchemasTableName, schemaTableSchema)
 		require.NoError(t, err)
 
 		CreateTestTable(t, dEnv, "dolt_docs", doltdb.DocsSchema,
 			"INSERT INTO dolt_docs VALUES ('LICENSE.md','A license')")
 		CreateTestTable(t, dEnv, doltdb.DoltQueryCatalogTableName, dtables.DoltQueryCatalogSchema,
 			"INSERT INTO dolt_query_catalog VALUES ('abc123', 1, 'example', 'select 2+2 from dual', 'description')")
-		CreateTestTable(t, dEnv, doltdb.SchemasTableName, SchemasTableSchema(),
-			"INSERT INTO dolt_schemas (type, name, fragment, id) VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1)")
+		CreateTestTable(t, dEnv, doltdb.SchemasTableName, schemaTableSchema,
+			"INSERT INTO dolt_schemas (type, name, fragment) VALUES ('view', 'name', 'create view name as select 2+2 from dual')")
 	}
 
 	t.Run("Create", func(t *testing.T) {
diff --git a/go/libraries/doltcore/sqle/sqldelete_test.go b/go/libraries/doltcore/sqle/sqldelete_test.go
index 563d878e5e..aa077130fd 100644
--- a/go/libraries/doltcore/sqle/sqldelete_test.go
+++ b/go/libraries/doltcore/sqle/sqldelete_test.go
@@ -207,12 +207,12 @@ var systemTableDeleteTests = []DeleteTest{
 	},
 	{
 		Name: "delete dolt_schemas",
-		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, SchemasTableSchema(),
+		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
 			"INSERT INTO dolt_schemas (type, name, fragment, id) VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1)"),
 		DeleteQuery:    "delete from dolt_schemas",
 		SelectQuery:    "select * from dolt_schemas",
 		ExpectedRows:   ToSqlRows(dtables.DoltQueryCatalogSchema),
-		ExpectedSchema: SchemasTableSchema(),
+		ExpectedSchema: schemaTableSchema,
 	},
 }
 
diff --git a/go/libraries/doltcore/sqle/sqlinsert_test.go b/go/libraries/doltcore/sqle/sqlinsert_test.go
index 92a704b8c2..4e3c3d412d 100644
--- a/go/libraries/doltcore/sqle/sqlinsert_test.go
+++ b/go/libraries/doltcore/sqle/sqlinsert_test.go
@@ -397,13 +397,13 @@ var systemTableInsertTests = []InsertTest{
 	},
 	{
 		Name:            "insert into dolt_schemas",
-		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, SchemasTableSchema(), ""),
+		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema, ""),
 		InsertQuery:     "insert into dolt_schemas (id, type, name, fragment) values (1, 'view', 'name', 'create view name as select 2+2 from dual')",
 		SelectQuery:     "select * from dolt_schemas ORDER BY id",
-		ExpectedRows: ToSqlRows(CompressSchema(SchemasTableSchema()),
+		ExpectedRows: ToSqlRows(CompressSchema(schemaTableSchema),
 			NewRow(types.String("view"), types.String("name"), types.String("create view name as select 2+2 from dual"), types.Int(1)),
 		),
-		ExpectedSchema: CompressSchema(SchemasTableSchema()),
+		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
 
diff --git a/go/libraries/doltcore/sqle/sqlreplace_test.go b/go/libraries/doltcore/sqle/sqlreplace_test.go
index 9b7ef8abe2..fdb7d4b616 100644
--- a/go/libraries/doltcore/sqle/sqlreplace_test.go
+++ b/go/libraries/doltcore/sqle/sqlreplace_test.go
@@ -272,12 +272,12 @@ var systemTableReplaceTests = []ReplaceTest{
 	},
 	{
 		Name: "replace into dolt_schemas",
-		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, SchemasTableSchema(),
+		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
 			"INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)"),
 		ReplaceQuery:   "replace into dolt_schemas (id, type, name, fragment) values ('1', 'view', 'name', 'create view name as select 1+1 from dual')",
 		SelectQuery:    "select type, name, fragment, id, extra from dolt_schemas",
 		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 1+1 from dual", int64(1), nil}},
-		ExpectedSchema: CompressSchema(SchemasTableSchema()),
+		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
 
diff --git a/go/libraries/doltcore/sqle/sqlselect_test.go b/go/libraries/doltcore/sqle/sqlselect_test.go
index 26788a91aa..7408790738 100644
--- a/go/libraries/doltcore/sqle/sqlselect_test.go
+++ b/go/libraries/doltcore/sqle/sqlselect_test.go
@@ -1298,11 +1298,11 @@ var systemTableSelectTests = []SelectTest{
 	},
 	{
 		Name: "select from dolt_schemas",
-		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, SchemasTableSchema(),
+		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
 			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)`),
 		Query:          "select * from dolt_schemas",
 		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 2+2 from dual", int64(1), nil}},
-		ExpectedSchema: CompressSchema(SchemasTableSchema()),
+		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
 
diff --git a/go/libraries/doltcore/sqle/sqlupdate_test.go b/go/libraries/doltcore/sqle/sqlupdate_test.go
index c8ea97902f..25ef8597fc 100644
--- a/go/libraries/doltcore/sqle/sqlupdate_test.go
+++ b/go/libraries/doltcore/sqle/sqlupdate_test.go
@@ -377,12 +377,12 @@ var systemTableUpdateTests = []UpdateTest{
 	},
 	{
 		Name: "update dolt_schemas",
-		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, SchemasTableSchema(),
+		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
 			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)`),
 		UpdateQuery:    "update dolt_schemas set type = 'not a view'",
 		SelectQuery:    "select * from dolt_schemas",
 		ExpectedRows:   []sql.Row{{"not a view", "name", "create view name as select 2+2 from dual", int64(1), nil}},
-		ExpectedSchema: CompressSchema(SchemasTableSchema()),
+		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
 

From 830203722468fe83c55a793a6638a25bfb867a42 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 14:25:16 -0800
Subject: [PATCH 09/40] Test fixes

---
 .../doltcore/sqle/enginetest/dolt_queries.go  | 31 +++++++++++++++++--
 .../sqle/enginetest/dolt_queries_merge.go     |  1 -
 go/libraries/doltcore/sqle/sqldelete_test.go  |  2 +-
 go/libraries/doltcore/sqle/sqlinsert_test.go  |  6 ++--
 go/libraries/doltcore/sqle/sqlreplace_test.go |  8 ++---
 go/libraries/doltcore/sqle/sqlselect_test.go  | 11 ++-----
 go/libraries/doltcore/sqle/sqlupdate_test.go  |  4 +--
 7 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
index 25969862e3..48184203e7 100644
--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
@@ -744,10 +744,35 @@ var DoltScripts = []queries.ScriptTest{
 		},
 		Assertions: []queries.ScriptTestAssertion{
 			{
-				Query: "SELECT type, name, fragment, id FROM dolt_schemas ORDER BY 1, 2",
+				Query: "SELECT type, name, fragment FROM dolt_schemas ORDER BY 1, 2",
 				Expected: []sql.Row{
-					{"view", "view1", "CREATE VIEW view1 AS SELECT v1 FROM viewtest", int64(1)},
-					{"view", "view2", "CREATE VIEW view2 AS SELECT v2 FROM viewtest", int64(2)},
+					{"view", "view1", "CREATE VIEW view1 AS SELECT v1 FROM viewtest"},
+					{"view", "view2", "CREATE VIEW view2 AS SELECT v2 FROM viewtest"},
+				},
+			},
+			{
+				Query: "CREATE VIEW VIEW1 AS SELECT v2 FROM viewtest",
+				ExpectedErr: sql.ErrExistingView,
+			},
+			{
+				Query: "drop view view1",
+				SkipResultsCheck: true,
+			},
+			{
+				Query: "SELECT type, name, fragment FROM dolt_schemas ORDER BY 1, 2",
+				Expected: []sql.Row{
+					{"view", "view2", "CREATE VIEW view2 AS SELECT v2 FROM viewtest"},
+				},
+			},
+			{
+				Query: "CREATE VIEW VIEW1 AS SELECT v1 FROM viewtest",
+				SkipResultsCheck: true,
+			},
+			{
+				Query: "SELECT type, name, fragment FROM dolt_schemas ORDER BY 1, 2",
+				Expected: []sql.Row{
+					{"view", "view1", "CREATE VIEW VIEW1 AS SELECT v1 FROM viewtest"},
+					{"view", "view2", "CREATE VIEW view2 AS SELECT v2 FROM viewtest"},
 				},
 			},
 		},
diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
index c68d57e548..43d6b759b4 100644
--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
@@ -995,7 +995,6 @@ var MergeScripts = []queries.ScriptTest{
 			"CALL dolt_checkout('other')",
 			"CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100",
 			"CREATE TRIGGER trigger4 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 1000",
-			"UPDATE dolt_schemas SET id = id + 1 WHERE name = 'trigger4'",
 			"CALL dolt_commit('-am', 'created triggers 3 & 4 on other');",
 			"CALL dolt_checkout('main');",
 		},
diff --git a/go/libraries/doltcore/sqle/sqldelete_test.go b/go/libraries/doltcore/sqle/sqldelete_test.go
index aa077130fd..1876305f3b 100644
--- a/go/libraries/doltcore/sqle/sqldelete_test.go
+++ b/go/libraries/doltcore/sqle/sqldelete_test.go
@@ -208,7 +208,7 @@ var systemTableDeleteTests = []DeleteTest{
 	{
 		Name: "delete dolt_schemas",
 		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
-			"INSERT INTO dolt_schemas (type, name, fragment, id) VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1)"),
+			"INSERT INTO dolt_schemas (type, name, fragment) VALUES ('view', 'name', 'create view name as select 2+2 from dual')"),
 		DeleteQuery:    "delete from dolt_schemas",
 		SelectQuery:    "select * from dolt_schemas",
 		ExpectedRows:   ToSqlRows(dtables.DoltQueryCatalogSchema),
diff --git a/go/libraries/doltcore/sqle/sqlinsert_test.go b/go/libraries/doltcore/sqle/sqlinsert_test.go
index 4e3c3d412d..1302ae5d8f 100644
--- a/go/libraries/doltcore/sqle/sqlinsert_test.go
+++ b/go/libraries/doltcore/sqle/sqlinsert_test.go
@@ -398,10 +398,10 @@ var systemTableInsertTests = []InsertTest{
 	{
 		Name:            "insert into dolt_schemas",
 		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema, ""),
-		InsertQuery:     "insert into dolt_schemas (id, type, name, fragment) values (1, 'view', 'name', 'create view name as select 2+2 from dual')",
-		SelectQuery:     "select * from dolt_schemas ORDER BY id",
+		InsertQuery:     "insert into dolt_schemas (type, name, fragment) values ('view', 'name', 'create view name as select 2+2 from dual')",
+		SelectQuery:     "select * from dolt_schemas ORDER BY name",
 		ExpectedRows: ToSqlRows(CompressSchema(schemaTableSchema),
-			NewRow(types.String("view"), types.String("name"), types.String("create view name as select 2+2 from dual"), types.Int(1)),
+			NewRow(types.String("view"), types.String("name"), types.String("create view name as select 2+2 from dual")),
 		),
 		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
diff --git a/go/libraries/doltcore/sqle/sqlreplace_test.go b/go/libraries/doltcore/sqle/sqlreplace_test.go
index fdb7d4b616..608a23ad7e 100644
--- a/go/libraries/doltcore/sqle/sqlreplace_test.go
+++ b/go/libraries/doltcore/sqle/sqlreplace_test.go
@@ -273,10 +273,10 @@ var systemTableReplaceTests = []ReplaceTest{
 	{
 		Name: "replace into dolt_schemas",
 		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
-			"INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)"),
-		ReplaceQuery:   "replace into dolt_schemas (id, type, name, fragment) values ('1', 'view', 'name', 'create view name as select 1+1 from dual')",
-		SelectQuery:    "select type, name, fragment, id, extra from dolt_schemas",
-		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 1+1 from dual", int64(1), nil}},
+			"INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)"),
+		ReplaceQuery:   "replace into dolt_schemas (type, name, fragment) values ('view', 'name', 'create view name as select 1+1 from dual')",
+		SelectQuery:    "select type, name, fragment, extra from dolt_schemas",
+		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 1+1 from dual", nil}},
 		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
diff --git a/go/libraries/doltcore/sqle/sqlselect_test.go b/go/libraries/doltcore/sqle/sqlselect_test.go
index 7408790738..b1470b49d4 100644
--- a/go/libraries/doltcore/sqle/sqlselect_test.go
+++ b/go/libraries/doltcore/sqle/sqlselect_test.go
@@ -1299,20 +1299,13 @@ var systemTableSelectTests = []SelectTest{
 	{
 		Name: "select from dolt_schemas",
 		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
-			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)`),
+			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)`),
 		Query:          "select * from dolt_schemas",
-		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 2+2 from dual", int64(1), nil}},
+		ExpectedRows:   []sql.Row{{"view", "name", "create view name as select 2+2 from dual", nil}},
 		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }
 
-func CreateTestJSON() types.JSON {
-	vrw := types.NewMemoryValueStore()
-	extraJSON, _ := types.NewMap(nil, vrw, types.String("CreatedAt"), types.Float(1))
-	res, _ := types.NewJSONDoc(types.Format_Default, vrw, extraJSON)
-	return res
-}
-
 func TestSelectSystemTables(t *testing.T) {
 	for _, test := range systemTableSelectTests {
 		t.Run(test.Name, func(t *testing.T) {
diff --git a/go/libraries/doltcore/sqle/sqlupdate_test.go b/go/libraries/doltcore/sqle/sqlupdate_test.go
index 25ef8597fc..c3aa89bfc8 100644
--- a/go/libraries/doltcore/sqle/sqlupdate_test.go
+++ b/go/libraries/doltcore/sqle/sqlupdate_test.go
@@ -378,10 +378,10 @@ var systemTableUpdateTests = []UpdateTest{
 	{
 		Name: "update dolt_schemas",
 		AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema,
-			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', 1, NULL)`),
+			`INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)`),
 		UpdateQuery:    "update dolt_schemas set type = 'not a view'",
 		SelectQuery:    "select * from dolt_schemas",
-		ExpectedRows:   []sql.Row{{"not a view", "name", "create view name as select 2+2 from dual", int64(1), nil}},
+		ExpectedRows:   []sql.Row{{"not a view", "name", "create view name as select 2+2 from dual", nil}},
 		ExpectedSchema: CompressSchema(schemaTableSchema),
 	},
 }

From d4a9ea0ce83d2f9f5f9c39b7a2c15b5a1e07519e Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 14:52:49 -0800
Subject: [PATCH 10/40] Fix remaining test

---
 .../doltcore/sqle/enginetest/dolt_queries_merge.go    | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
index 43d6b759b4..09e28b75e2 100644
--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
@@ -1001,13 +1001,12 @@ var MergeScripts = []queries.ScriptTest{
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query:    "CALL DOLT_MERGE('other');",
-				Expected: []sql.Row{{0, 1}},
+				Expected: []sql.Row{{0, 0}},
+			},
+			{
+				Query:    "select count(*) from dolt_schemas where type = 'trigger';",
+				Expected: []sql.Row{{4}},
 			},
-			// todo: merge triggers correctly
-			//{
-			//	Query:    "select count(*) from dolt_schemas where type = 'trigger';",
-			//	Expected: []sql.Row{{4}},
-			//},
 		},
 	},
 	{

From 962c6f440037de8b57d45ef0e3a08f3215d8067d Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 15:32:08 -0800
Subject: [PATCH 11/40] Bats tests

---
 go/libraries/doltcore/doltdb/system_table.go |  5 +++
 go/libraries/doltcore/sqle/database.go       |  3 +-
 integration-tests/bats/create-views.bats     | 38 +++++++++++++++++++-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go
index 525e696467..9347bcc2f4 100644
--- a/go/libraries/doltcore/doltdb/system_table.go
+++ b/go/libraries/doltcore/doltdb/system_table.go
@@ -66,6 +66,11 @@ func IsReadOnlySystemTable(name string) bool {
 	return HasDoltPrefix(name) && !set.NewStrSet(writeableSystemTables).Contains(name)
 }
 
+// IsNonDroppableSystemTable returns whether the table name given is a system table that cannot be dropped.
+func IsNonDroppableSystemTable(name string) bool {
+	return IsReadOnlySystemTable(name) || strings.ToLower(name) == SchemasTableName
+}
+
 // GetNonSystemTableNames gets non-system table names
 func GetNonSystemTableNames(ctx context.Context, root *RootValue) ([]string, error) {
 	tn, err := root.GetTableNames(ctx)
diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index af91d6da35..1e4856677e 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -467,6 +467,7 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds
 			}
 		}
 	}
+	
 	if found {
 		return dt, found, nil
 	}
@@ -732,7 +733,7 @@ func (db Database) DropTable(ctx *sql.Context, tableName string) error {
 	if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil {
 		return err
 	}
-	if doltdb.IsReadOnlySystemTable(tableName) {
+	if doltdb.IsNonDroppableSystemTable(tableName) {
 		return ErrSystemTableAlter.New(tableName)
 	}
 
diff --git a/integration-tests/bats/create-views.bats b/integration-tests/bats/create-views.bats
index 42580205ef..59069a9d4c 100644
--- a/integration-tests/bats/create-views.bats
+++ b/integration-tests/bats/create-views.bats
@@ -35,14 +35,50 @@ SQL
     run dolt sql -q "select name from dolt_schemas" -r csv
     [ "$status" -eq 0 ]
     [[ "${lines[1]}" =~ 'four' ]] || false
+
     run dolt sql -q "drop table dolt_schemas"
-    skip "dropping dolt_schemas is currently unprotected"
     [ "$status" -ne 0 ]
     run dolt sql -q "select name from dolt_schemas" -r csv
     [ "$status" -eq 0 ]
     [[ "${lines[1]}" =~ 'four' ]] || false
 }
 
+@test "create-views: can't alter dolt_schemas" {
+    run dolt sql -q "create view four as select 2+2 as res from dual;"
+    [ "$status" -eq 0 ]
+    run dolt sql -q "select name from dolt_schemas" -r csv
+    [ "$status" -eq 0 ]
+    [[ "${lines[1]}" =~ 'four' ]] || false
+
+    run dolt sql -q "alter table dolt_schemas add column newcol int"
+    [ "$status" -ne 0 ]
+    [[ "$output" =~ "cannot be altered" ]] || false
+    
+    run dolt sql -q "select name from dolt_schemas" -r csv
+    [ "$status" -eq 0 ]
+    [[ "${lines[1]}" =~ 'four' ]] || false
+}
+
+@test "create-views: drop and create same view" {
+    dolt sql -q "create view four as select 2+2 as res from dual;"
+    dolt sql -q "create view six as select 3+3 as res from dual;"
+    
+    run dolt sql -q "select name from dolt_schemas order by name" -r csv
+    [ "$status" -eq 0 ]
+    [[ "${lines[1]}" =~ 'four' ]] || false
+    [[ "${lines[2]}" =~ 'six' ]] || false
+
+    dolt commit -Am "new views"
+    dolt sql -q "drop view four"
+
+    run dolt sql -q "create view four as select 2+2 as res from dual;"
+    [ "$status" -eq 0 ]
+
+    run dolt diff
+    [ "$status" -eq 0 ]
+    [ "${#lines[@]}" -eq 0 ]
+}
+
 @test "create-views: join two views" {
     run dolt sql <<SQL
 create view four as select 2+2 as res from dual;

From c144b8953824b452cf3fae3c178fdc43201384dc Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 16:37:20 -0800
Subject: [PATCH 12/40] Added data migration back

---
 go/libraries/doltcore/sqle/database.go        |  3 +-
 go/libraries/doltcore/sqle/schema_table.go    | 98 ++++++++++++++++++-
 .../doltcore/sqle/schema_table_test.go        | 10 +-
 3 files changed, 99 insertions(+), 12 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 1e4856677e..0cdac6bfa2 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -1270,7 +1270,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil {
 		return err
 	}
-	tbl, err := GetOrCreateDoltSchemasTable(ctx, db)
+	tbl, err := getOrCreateDoltSchemasTable(ctx, db)
 	if err != nil {
 		return err
 	}
@@ -1299,6 +1299,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if err != nil {
 		return err
 	}
+	
 	return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON})
 }
 
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index de61b25d5c..e98a2719e6 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -64,15 +64,26 @@ var schemasTableCols = schema.NewColCollection(
 
 var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols)
 
-// GetOrCreateDoltSchemasTable returns the `dolt_schemas` table in `db`, creating it if it does not already exist.
-func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *WritableDoltTable, retErr error) {
+// getOrCreateDoltSchemasTable returns the `dolt_schemas` table in `db`, creating it if it does not already exist.
+// Also migrates data to the correct format if necessary.
+func getOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *WritableDoltTable, retErr error) {
 	tbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	if err != nil {
 		return nil, err
 	}
 
 	if found {
-		return tbl.(*WritableDoltTable), nil
+		schemasTable := tbl.(*WritableDoltTable)
+		// Old schemas table contains the `id` column or is missing an `extra` column.
+		if tbl.Schema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) || !tbl.Schema().Contains(doltdb.SchemasTablesExtraCol, doltdb.SchemasTableName) {
+			root, err := db.GetRoot(ctx)
+			if err != nil {
+				return nil, err
+			}
+			return migrateOldSchemasTableToNew(ctx, db, root, schemasTable)
+		} else {
+			return schemasTable, nil
+		}
 	}
 
 	root, err := db.GetRoot(ctx)
@@ -96,6 +107,87 @@ func GetOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 	return tbl.(*WritableDoltTable), nil
 }
 
+func migrateOldSchemasTableToNew(
+		ctx *sql.Context,
+		db Database,
+		root *doltdb.RootValue,
+		schemasTable *WritableDoltTable,
+) (newTable *WritableDoltTable, rerr error) {
+	// Copy all of the old data over and add an index column and an extra column
+	iter, err := SqlTableToRowIter(ctx, schemasTable.DoltTable, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+	// need to get the column indexes from the current schema
+	nameIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
+	typeIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
+	fragmentIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol)
+	extraIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol)
+
+	defer func(iter sql.RowIter, ctx *sql.Context) {
+		err := iter.Close(ctx)
+		if err != nil && rerr == nil {
+			rerr = err
+		}
+	}(iter, ctx)
+
+	var newRows []sql.Row
+	for {
+		sqlRow, err := iter.Next(ctx)
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			return nil, err
+		}
+		
+		newRow := make(sql.Row, schemasTableCols.Size())
+		newRow[0] = sqlRow[typeIdx]
+		newRow[1] = sqlRow[nameIdx]
+		newRow[2] = sqlRow[fragmentIdx]
+		if extraIdx >= 0 {
+			newRow[3] = sqlRow[extraIdx]
+		}
+
+		newRows = append(newRows, newRow)
+	}
+
+	err = db.DropTable(ctx, doltdb.SchemasTableName)
+	if err != nil {
+		return nil, err
+	}
+
+	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, schemaTableSchema)
+	if err != nil {
+		return nil, err
+	}
+	
+	tbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
+	if err != nil {
+		return nil, err
+	}
+	if !found {
+		return nil, sql.ErrTableNotFound.New("dolt_schemas")
+	}
+
+	inserter := tbl.(*WritableDoltTable).Inserter(ctx)
+	for _, row := range newRows {
+		err = inserter.Insert(ctx, row)
+		if err != nil {
+			return nil, err
+		}
+	}
+	
+	err = inserter.Close(ctx)
+	if err != nil {
+		return nil, err
+	}
+	
+	return tbl.(*WritableDoltTable), nil
+}
+
 // fragFromSchemasTable returns the row with the given schema fragment if it exists.
 func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType string, name string) (r sql.Row, found bool, rerr error) {
 	fragType, name = strings.ToLower(fragType), strings.ToLower(name)
diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index 5a59a6e982..13c3e27675 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -33,9 +33,6 @@ import (
 )
 
 func TestSchemaTableRecreationOlder(t *testing.T) {
-	if types.Format_Default != types.Format_LD_1 {
-		t.Skip() // schema table migrations predate NBF __DOLT__
-	}
 	ctx := NewTestSQLCtx(context.Background())
 	dEnv := dtestutils.CreateTestEnv()
 	tmpDir, err := dEnv.TempTableFilesDir()
@@ -88,7 +85,7 @@ func TestSchemaTableRecreationOlder(t *testing.T) {
 		return nil
 	})
 
-	tbl, err := GetOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
+	tbl, err := getOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
 	require.NoError(t, err)
 
 	table, err = tbl.DoltTable.DoltTable(ctx)
@@ -118,9 +115,6 @@ func TestSchemaTableRecreationOlder(t *testing.T) {
 }
 
 func TestSchemaTableRecreation(t *testing.T) {
-	if types.Format_Default != types.Format_LD_1 {
-		t.Skip() // schema table migrations predate NBF __DOLT__
-	}
 	ctx := NewTestSQLCtx(context.Background())
 	dEnv := dtestutils.CreateTestEnv()
 	tmpDir, err := dEnv.TempTableFilesDir()
@@ -175,7 +169,7 @@ func TestSchemaTableRecreation(t *testing.T) {
 		return nil
 	})
 
-	tbl, err := GetOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
+	tbl, err := getOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
 	require.NoError(t, err)
 
 	table, err = tbl.DoltTable.DoltTable(ctx)

From cb8ffdf265d3a06feabdb0a9e5060626b44f0a97 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 16:59:04 -0800
Subject: [PATCH 13/40] Re-enabled migration tests and fixed a bug

---
 go/libraries/doltcore/sqle/database.go        |   5 +
 go/libraries/doltcore/sqle/schema_table.go    |  14 +-
 .../doltcore/sqle/schema_table_test.go        | 152 +++++++-----------
 3 files changed, 69 insertions(+), 102 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 0cdac6bfa2..1683e0f71f 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -737,6 +737,11 @@ func (db Database) DropTable(ctx *sql.Context, tableName string) error {
 		return ErrSystemTableAlter.New(tableName)
 	}
 
+	return db.dropTable(ctx, tableName)
+}
+
+// dropTable drops the table with the name given, without any business logic checks
+func (db Database) dropTable(ctx *sql.Context, tableName string) error {
 	ds := dsess.DSessFromSess(ctx.Session)
 	if _, ok := ds.GetTemporaryTable(ctx, db.Name(), tableName); ok {
 		ds.DropTemporaryTable(ctx, db.Name(), tableName)
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index e98a2719e6..4e96a0323e 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -76,11 +76,7 @@ func getOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 		schemasTable := tbl.(*WritableDoltTable)
 		// Old schemas table contains the `id` column or is missing an `extra` column.
 		if tbl.Schema().Contains(doltdb.SchemasTablesIdCol, doltdb.SchemasTableName) || !tbl.Schema().Contains(doltdb.SchemasTablesExtraCol, doltdb.SchemasTableName) {
-			root, err := db.GetRoot(ctx)
-			if err != nil {
-				return nil, err
-			}
-			return migrateOldSchemasTableToNew(ctx, db, root, schemasTable)
+			return migrateOldSchemasTableToNew(ctx, db, schemasTable)
 		} else {
 			return schemasTable, nil
 		}
@@ -110,7 +106,6 @@ func getOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 func migrateOldSchemasTableToNew(
 		ctx *sql.Context,
 		db Database,
-		root *doltdb.RootValue,
 		schemasTable *WritableDoltTable,
 ) (newTable *WritableDoltTable, rerr error) {
 	// Copy all of the old data over and add an index column and an extra column
@@ -154,11 +149,16 @@ func migrateOldSchemasTableToNew(
 		newRows = append(newRows, newRow)
 	}
 
-	err = db.DropTable(ctx, doltdb.SchemasTableName)
+	err = db.dropTable(ctx, doltdb.SchemasTableName)
 	if err != nil {
 		return nil, err
 	}
 
+	root, err := db.GetRoot(ctx)
+	if err != nil {
+		return nil, err
+	}
+	
 	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, schemaTableSchema)
 	if err != nil {
 		return nil, err
diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index 13c3e27675..b126813b09 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -16,6 +16,8 @@ package sqle
 
 import (
 	"context"
+	"encoding/json"
+	"io"
 	"testing"
 
 	"github.com/dolthub/go-mysql-server/sql"
@@ -25,14 +27,11 @@ import (
 
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/dtestutils"
-	"github.com/dolthub/dolt/go/libraries/doltcore/row"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
-	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil"
 	"github.com/dolthub/dolt/go/libraries/doltcore/table/editor"
-	"github.com/dolthub/dolt/go/store/types"
 )
 
-func TestSchemaTableRecreationOlder(t *testing.T) {
+func TestSchemaTableMigrationOriginal(t *testing.T) {
 	ctx := NewTestSQLCtx(context.Background())
 	dEnv := dtestutils.CreateTestEnv()
 	tmpDir, err := dEnv.TempTableFilesDir()
@@ -48,15 +47,17 @@ func TestSchemaTableRecreationOlder(t *testing.T) {
 	require.NoError(t, err)
 	ctx.SetCurrentDatabase(db.Name())
 
-	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{ // schema of dolt_schemas table before the change
+	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{ // original schema of dolt_schemas table
 		{Name: doltdb.SchemasTablesTypeCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
 		{Name: doltdb.SchemasTablesNameCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
 		{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
 	}), sql.Collation_Default)
 	require.NoError(t, err)
+	
 	sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	require.NoError(t, err)
 	require.True(t, found)
+
 	inserter := sqlTbl.(*WritableDoltTable).Inserter(ctx)
 	err = inserter.Insert(ctx, sql.Row{"view", "view1", "SELECT v1 FROM test;"})
 	require.NoError(t, err)
@@ -65,56 +66,33 @@ func TestSchemaTableRecreationOlder(t *testing.T) {
 	err = inserter.Close(ctx)
 	require.NoError(t, err)
 
-	table, err := sqlTbl.(*WritableDoltTable).DoltTable.DoltTable(ctx)
-	require.NoError(t, err)
-
-	rowData, err := table.GetNomsRowData(ctx)
-	require.NoError(t, err)
-	expectedVals := []sql.Row{
-		{"view", "view1", "SELECT v1 FROM test;"},
-		{"view", "view2", "SELECT v2 FROM test;"},
-	}
-	index := 0
-	_ = rowData.IterAll(ctx, func(keyTpl, valTpl types.Value) error {
-		dRow, err := row.FromNoms(sqlTbl.(*WritableDoltTable).sch, keyTpl.(types.Tuple), valTpl.(types.Tuple))
-		require.NoError(t, err)
-		sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, sqlTbl.(*WritableDoltTable).sch)
-		require.NoError(t, err)
-		assert.Equal(t, expectedVals[index], sqlRow)
-		index++
-		return nil
-	})
-
 	tbl, err := getOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
 	require.NoError(t, err)
 
-	table, err = tbl.DoltTable.DoltTable(ctx)
+	iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil)
 	require.NoError(t, err)
-
-	rowData, err = table.GetNomsRowData(ctx)
-	require.NoError(t, err)
-	expectedVals = []sql.Row{
-		{"view", "view1", "SELECT v1 FROM test;", int64(1), nil},
-		{"view", "view2", "SELECT v2 FROM test;", int64(2), nil},
+	
+	var rows []sql.Row
+	for {
+		row, err := iter.Next(ctx)
+		if err == io.EOF {
+			break
+		}
+		
+		require.NoError(t, err)
+		rows = append(rows, row)
 	}
-	index = 0
-	_ = rowData.IterAll(ctx, func(keyTpl, valTpl types.Value) error {
-		dRow, err := row.FromNoms(tbl.sch, keyTpl.(types.Tuple), valTpl.(types.Tuple))
-		require.NoError(t, err)
-		sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, tbl.sch)
-		require.NoError(t, err)
-		assert.Equal(t, expectedVals[index], sqlRow)
-		index++
-		return nil
-	})
-
-	indexes := tbl.sch.Indexes().AllIndexes()
-	require.Len(t, indexes, 1)
-	assert.Equal(t, true, indexes[0].IsUnique())
-	assert.Equal(t, doltdb.SchemasTablesIndexName, indexes[0].Name())
+	
+	require.NoError(t, iter.Close(ctx))
+	expectedRows := []sql.Row{
+		{"view", "view1", "SELECT v1 FROM test;", nil},
+		{"view", "view2", "SELECT v2 FROM test;", nil},
+	}
+	
+	assert.Equal(t, expectedRows, rows)
 }
 
-func TestSchemaTableRecreation(t *testing.T) {
+func TestSchemaTableMigrationV1(t *testing.T) {
 	ctx := NewTestSQLCtx(context.Background())
 	dEnv := dtestutils.CreateTestEnv()
 	tmpDir, err := dEnv.TempTableFilesDir()
@@ -130,70 +108,54 @@ func TestSchemaTableRecreation(t *testing.T) {
 	require.NoError(t, err)
 	ctx.SetCurrentDatabase(db.Name())
 
-	// This is the schema of dolt_schemas table after the change adding the ID column, but before adding the extra column
-	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{ //
-		{Name: doltdb.SchemasTablesTypeCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
-		{Name: doltdb.SchemasTablesNameCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
+	// original schema of dolt_schemas table with the ID column
+	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{ 
+		{Name: doltdb.SchemasTablesTypeCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
+		{Name: doltdb.SchemasTablesNameCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
 		{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
-		{Name: doltdb.SchemasTablesIdCol, Type: gmstypes.Int64, Source: doltdb.SchemasTableName, PrimaryKey: false},
+		{Name: doltdb.SchemasTablesIdCol, Type: gmstypes.Int64, Source: doltdb.SchemasTableName, PrimaryKey: true},
+		{Name: doltdb.SchemasTablesExtraCol, Type: gmstypes.JsonType{}, Source: doltdb.SchemasTableName, PrimaryKey: false, Nullable: true},
 	}), sql.Collation_Default)
 	require.NoError(t, err)
+
 	sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	require.NoError(t, err)
 	require.True(t, found)
+
 	inserter := sqlTbl.(*WritableDoltTable).Inserter(ctx)
-	err = inserter.Insert(ctx, sql.Row{"view", "view1", "SELECT v1 FROM test;", int64(1)})
+	err = inserter.Insert(ctx, sql.Row{"view", "view1", "SELECT v1 FROM test;", 1, `{"extra": "data"}`})
 	require.NoError(t, err)
-	err = inserter.Insert(ctx, sql.Row{"view", "view2", "SELECT v2 FROM test;", int64(2)})
+	err = inserter.Insert(ctx, sql.Row{"view", "view2", "SELECT v2 FROM test;", 2, nil})
 	require.NoError(t, err)
 	err = inserter.Close(ctx)
 	require.NoError(t, err)
 
-	table, err := sqlTbl.(*WritableDoltTable).DoltTable.DoltTable(ctx)
-	require.NoError(t, err)
-
-	rowData, err := table.GetNomsRowData(ctx)
-	require.NoError(t, err)
-	expectedVals := []sql.Row{
-		{"view", "view1", "SELECT v1 FROM test;", int64(1)},
-		{"view", "view2", "SELECT v2 FROM test;", int64(2)},
-	}
-	index := 0
-	_ = rowData.IterAll(ctx, func(keyTpl, valTpl types.Value) error {
-		dRow, err := row.FromNoms(sqlTbl.(*WritableDoltTable).sch, keyTpl.(types.Tuple), valTpl.(types.Tuple))
-		require.NoError(t, err)
-		sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, sqlTbl.(*WritableDoltTable).sch)
-		require.NoError(t, err)
-		assert.Equal(t, expectedVals[index], sqlRow)
-		index++
-		return nil
-	})
-
 	tbl, err := getOrCreateDoltSchemasTable(ctx, db) // removes the old table and recreates it with the new schema
 	require.NoError(t, err)
 
-	table, err = tbl.DoltTable.DoltTable(ctx)
+	iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil)
 	require.NoError(t, err)
 
-	rowData, err = table.GetNomsRowData(ctx)
-	require.NoError(t, err)
-	expectedVals = []sql.Row{
-		{"view", "view1", "SELECT v1 FROM test;", int64(1), nil},
-		{"view", "view2", "SELECT v2 FROM test;", int64(2), nil},
+	var rows []sql.Row
+	for {
+		row, err := iter.Next(ctx)
+		if err == io.EOF {
+			break
+		}
+
+		require.NoError(t, err)
+		rows = append(rows, row)
 	}
-	index = 0
-	_ = rowData.IterAll(ctx, func(keyTpl, valTpl types.Value) error {
-		dRow, err := row.FromNoms(tbl.sch, keyTpl.(types.Tuple), valTpl.(types.Tuple))
-		require.NoError(t, err)
-		sqlRow, err := sqlutil.DoltRowToSqlRow(dRow, tbl.sch)
-		require.NoError(t, err)
-		assert.Equal(t, expectedVals[index], sqlRow)
-		index++
-		return nil
-	})
 
-	indexes := tbl.sch.Indexes().AllIndexes()
-	require.Len(t, indexes, 1)
-	assert.Equal(t, true, indexes[0].IsUnique())
-	assert.Equal(t, doltdb.SchemasTablesIndexName, indexes[0].Name())
+	require.NoError(t, iter.Close(ctx))
+	
+	var expectedJsonDoc any
+	require.NoError(t, json.Unmarshal([]byte(`{"extra": "data"}`), &expectedJsonDoc))
+	
+	expectedRows := []sql.Row{
+		{"view", "view1", "SELECT v1 FROM test;", gmstypes.JSONDocument{Val: expectedJsonDoc}},
+		{"view", "view2", "SELECT v2 FROM test;", nil},
+	}
+
+	assert.Equal(t, expectedRows, rows)
 }

From b75f9a912984c6fb809e9e0222447347495eae74 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 17:23:00 -0800
Subject: [PATCH 14/40] Drop the dolt_schemas table after deleting all views
 and triggers

---
 go/libraries/doltcore/sqle/database.go     | 44 ++++++++++++++++++++-
 go/libraries/doltcore/sqle/schema_table.go |  6 +--
 integration-tests/bats/create-views.bats   | 45 ++++++++++++++++++++++
 3 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 1683e0f71f..99110c2af5 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -1312,6 +1312,7 @@ func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name str
 	if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil {
 		return err
 	}
+
 	stbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	if err != nil {
 		return err
@@ -1319,7 +1320,7 @@ func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name str
 	if !found {
 		return missingErr
 	}
-
+	
 	tbl := stbl.(*WritableDoltTable)
 	row, exists, err := fragFromSchemasTable(ctx, tbl, fragType, name)
 	if err != nil {
@@ -1334,7 +1335,46 @@ func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name str
 		return err
 	}
 
-	return deleter.Close(ctx)
+	err = deleter.Close(ctx)
+	if err != nil {
+		return err
+	}
+	
+	// If the dolt schemas table is now empty, drop it entirely. This is necessary to prevent the creation and 
+	// immediate dropping of views or triggers, when none previously existed, from changing the database state.
+	return db.dropTableIfEmpty(ctx, doltdb.SchemasTableName)
+}
+
+// dropTableIfEmpty drops the table named if it exists and has at least one row.
+func (db Database) dropTableIfEmpty(ctx *sql.Context, tableName string) error {
+	stbl, found, err := db.GetTableInsensitive(ctx, tableName)
+	if err != nil {
+		return err
+	}
+	if !found {
+		return nil
+	}
+
+	table, err := stbl.(*WritableDoltTable).DoltTable.DoltTable(ctx)
+	if err != nil {
+		return err
+	}
+
+	rows, err := table.GetRowData(ctx)
+	if err != nil {
+		return err
+	}
+
+	numRows, err := rows.Count()
+	if err != nil {
+		return err
+	}
+
+	if numRows == 0 {
+		return db.dropTable(ctx, tableName)
+	}
+
+	return nil
 }
 
 // GetAllTemporaryTables returns all temporary tables
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index 4e96a0323e..52e70d8ffe 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -103,11 +103,7 @@ func getOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 	return tbl.(*WritableDoltTable), nil
 }
 
-func migrateOldSchemasTableToNew(
-		ctx *sql.Context,
-		db Database,
-		schemasTable *WritableDoltTable,
-) (newTable *WritableDoltTable, rerr error) {
+func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *WritableDoltTable) (newTable *WritableDoltTable, rerr error) {
 	// Copy all of the old data over and add an index column and an extra column
 	iter, err := SqlTableToRowIter(ctx, schemasTable.DoltTable, nil)
 	if err != nil {
diff --git a/integration-tests/bats/create-views.bats b/integration-tests/bats/create-views.bats
index 59069a9d4c..5f590ceb4c 100644
--- a/integration-tests/bats/create-views.bats
+++ b/integration-tests/bats/create-views.bats
@@ -79,6 +79,51 @@ SQL
     [ "${#lines[@]}" -eq 0 ]
 }
 
+@test "create-views: drop all views" {
+    dolt sql -q "create view four as select 2+2 as res from dual;"
+    dolt sql -q "create view six as select 3+3 as res from dual;"
+    
+    run dolt sql -q "select name from dolt_schemas order by name" -r csv
+    [ "$status" -eq 0 ]
+    [[ "${lines[1]}" =~ 'four' ]] || false
+    [[ "${lines[2]}" =~ 'six' ]] || false
+
+    dolt commit -Am "new views"
+    dolt sql -q "drop view four"
+
+    run dolt ls --all
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "dolt_schemas" ]]
+
+    run dolt diff
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "dolt_schemas" ]]
+
+    dolt commit -Am "dropped a view"
+    dolt sql -q "drop view six"
+
+    # Dropping all views should result in the dolt_schemas table deleting itself
+    run dolt ls --all
+    [ "$status" -eq 0 ]
+    [[ ! "$output" =~ "dolt_schemas" ]]
+
+    run dolt diff
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "deleted table" ]]
+
+    dolt commit -Am "no views left"
+
+    # Creating and then dropping a bunch of views should produce no diff
+    dolt sql -q "create view four as select 2+2 as res from dual;"
+    dolt sql -q "create view six as select 3+3 as res from dual;"
+    dolt sql -q "drop view four"
+    dolt sql -q "drop view six"
+
+    run dolt diff
+    [ "$status" -eq 0 ]
+    [ "${#lines[@]}" -eq 0 ]    
+}
+
 @test "create-views: join two views" {
     run dolt sql <<SQL
 create view four as select 2+2 as res from dual;

From 3c529bf51ab520e1c814a4cf95379fa5592abc48 Mon Sep 17 00:00:00 2001
From: zachmu <zachmu@users.noreply.github.com>
Date: Thu, 16 Feb 2023 01:29:12 +0000
Subject: [PATCH 15/40] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and
 go/Godeps/update.sh

---
 go/libraries/doltcore/doltdb/system_table.go  |  4 +--
 go/libraries/doltcore/sqle/database.go        | 12 ++++----
 .../doltcore/sqle/enginetest/dolt_queries.go  |  6 ++--
 go/libraries/doltcore/sqle/schema_table.go    | 30 +++++++++----------
 .../doltcore/sqle/schema_table_test.go        | 16 +++++-----
 go/libraries/doltcore/sqle/tables.go          |  2 +-
 6 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go
index 9347bcc2f4..954240b727 100644
--- a/go/libraries/doltcore/doltdb/system_table.go
+++ b/go/libraries/doltcore/doltdb/system_table.go
@@ -235,13 +235,13 @@ const (
 	SchemasTablesTypeCol = "type"
 	// SchemasTablesNameCol The name of the column that stores the name of a schema fragment in the dolt_schemas table
 	SchemasTablesNameCol = "name"
-	// SchemasTablesFragmentCol The name of the column that stores the SQL fragment of a schema element in the 
+	// SchemasTablesFragmentCol The name of the column that stores the SQL fragment of a schema element in the
 	// dolt_schemas table
 	SchemasTablesFragmentCol = "fragment"
 	// SchemasTablesExtraCol The name of the column that stores extra information about a schema element in the
 	// dolt_schemas table
 	SchemasTablesExtraCol = "extra"
-	// 
+	//
 	SchemasTablesIndexName = "fragment_name"
 )
 
diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 99110c2af5..6cf0732e02 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -467,7 +467,7 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds
 			}
 		}
 	}
-	
+
 	if found {
 		return dt, found, nil
 	}
@@ -1287,7 +1287,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if exists {
 		return existingErr
 	}
-	
+
 	// Insert the new row into the db
 	inserter := tbl.Inserter(ctx)
 	defer func() {
@@ -1304,7 +1304,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin
 	if err != nil {
 		return err
 	}
-	
+
 	return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON})
 }
 
@@ -1320,7 +1320,7 @@ func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name str
 	if !found {
 		return missingErr
 	}
-	
+
 	tbl := stbl.(*WritableDoltTable)
 	row, exists, err := fragFromSchemasTable(ctx, tbl, fragType, name)
 	if err != nil {
@@ -1339,8 +1339,8 @@ func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name str
 	if err != nil {
 		return err
 	}
-	
-	// If the dolt schemas table is now empty, drop it entirely. This is necessary to prevent the creation and 
+
+	// If the dolt schemas table is now empty, drop it entirely. This is necessary to prevent the creation and
 	// immediate dropping of views or triggers, when none previously existed, from changing the database state.
 	return db.dropTableIfEmpty(ctx, doltdb.SchemasTableName)
 }
diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
index 48184203e7..8c5bd812b2 100644
--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
@@ -751,11 +751,11 @@ var DoltScripts = []queries.ScriptTest{
 				},
 			},
 			{
-				Query: "CREATE VIEW VIEW1 AS SELECT v2 FROM viewtest",
+				Query:       "CREATE VIEW VIEW1 AS SELECT v2 FROM viewtest",
 				ExpectedErr: sql.ErrExistingView,
 			},
 			{
-				Query: "drop view view1",
+				Query:            "drop view view1",
 				SkipResultsCheck: true,
 			},
 			{
@@ -765,7 +765,7 @@ var DoltScripts = []queries.ScriptTest{
 				},
 			},
 			{
-				Query: "CREATE VIEW VIEW1 AS SELECT v1 FROM viewtest",
+				Query:            "CREATE VIEW VIEW1 AS SELECT v1 FROM viewtest",
 				SkipResultsCheck: true,
 			},
 			{
diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go
index 52e70d8ffe..c908b85998 100644
--- a/go/libraries/doltcore/sqle/schema_table.go
+++ b/go/libraries/doltcore/sqle/schema_table.go
@@ -39,7 +39,7 @@ type Extra struct {
 }
 
 func mustNewColWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...schema.ColConstraint) schema.Column {
-	col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo,partOfPK, defaultVal, autoIncrement, comment, constraints...)
+	col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, autoIncrement, comment, constraints...)
 	if err != nil {
 		panic(err)
 	}
@@ -86,7 +86,7 @@ func getOrCreateDoltSchemasTable(ctx *sql.Context, db Database) (retTbl *Writabl
 	if err != nil {
 		return nil, err
 	}
-	
+
 	// Create new empty table
 	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, schemaTableSchema)
 	if err != nil {
@@ -110,7 +110,7 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr
 		return nil, err
 	}
 
-	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we
 	// need to get the column indexes from the current schema
 	nameIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
 	typeIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
@@ -133,7 +133,7 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr
 		if err != nil {
 			return nil, err
 		}
-		
+
 		newRow := make(sql.Row, schemasTableCols.Size())
 		newRow[0] = sqlRow[typeIdx]
 		newRow[1] = sqlRow[nameIdx]
@@ -154,12 +154,12 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr
 	if err != nil {
 		return nil, err
 	}
-	
+
 	err = db.createDoltTable(ctx, doltdb.SchemasTableName, root, schemaTableSchema)
 	if err != nil {
 		return nil, err
 	}
-	
+
 	tbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	if err != nil {
 		return nil, err
@@ -175,12 +175,12 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr
 			return nil, err
 		}
 	}
-	
+
 	err = inserter.Close(ctx)
 	if err != nil {
 		return nil, err
 	}
-	
+
 	return tbl.(*WritableDoltTable), nil
 }
 
@@ -200,12 +200,12 @@ func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType str
 			rerr = err
 		}
 	}(iter, ctx)
-	
-	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we
 	// need to get the column indexes from the current schema
 	nameIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
 	typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
-	
+
 	for {
 		sqlRow, err := iter.Next(ctx)
 		if err == io.EOF {
@@ -220,7 +220,7 @@ func fragFromSchemasTable(ctx *sql.Context, tbl *WritableDoltTable, fragType str
 			return sqlRow, true, nil
 		}
 	}
-	
+
 	return nil, false, nil
 }
 
@@ -236,7 +236,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 		return nil, err
 	}
 
-	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we 
+	// The dolt_schemas table has undergone various changes over time and multiple possible schemas for it exist, so we
 	// need to get the column indexes from the current schema
 	nameIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesNameCol)
 	typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol)
@@ -249,7 +249,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 			rerr = err
 		}
 	}(iter, ctx)
-	
+
 	var frags []schemaFragment
 	for {
 		sqlRow, err := iter.Next(ctx)
@@ -283,7 +283,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType
 			created:  time.Unix(createdTime, 0).UTC(),
 		})
 	}
-	
+
 	return frags, nil
 }
 
diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index b126813b09..2d833ede6c 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -53,7 +53,7 @@ func TestSchemaTableMigrationOriginal(t *testing.T) {
 		{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
 	}), sql.Collation_Default)
 	require.NoError(t, err)
-	
+
 	sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
 	require.NoError(t, err)
 	require.True(t, found)
@@ -71,24 +71,24 @@ func TestSchemaTableMigrationOriginal(t *testing.T) {
 
 	iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil)
 	require.NoError(t, err)
-	
+
 	var rows []sql.Row
 	for {
 		row, err := iter.Next(ctx)
 		if err == io.EOF {
 			break
 		}
-		
+
 		require.NoError(t, err)
 		rows = append(rows, row)
 	}
-	
+
 	require.NoError(t, iter.Close(ctx))
 	expectedRows := []sql.Row{
 		{"view", "view1", "SELECT v1 FROM test;", nil},
 		{"view", "view2", "SELECT v2 FROM test;", nil},
 	}
-	
+
 	assert.Equal(t, expectedRows, rows)
 }
 
@@ -109,7 +109,7 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 	ctx.SetCurrentDatabase(db.Name())
 
 	// original schema of dolt_schemas table with the ID column
-	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{ 
+	err = db.createSqlTable(ctx, doltdb.SchemasTableName, sql.NewPrimaryKeySchema(sql.Schema{
 		{Name: doltdb.SchemasTablesTypeCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
 		{Name: doltdb.SchemasTablesNameCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
 		{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
@@ -148,10 +148,10 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 	}
 
 	require.NoError(t, iter.Close(ctx))
-	
+
 	var expectedJsonDoc any
 	require.NoError(t, json.Unmarshal([]byte(`{"extra": "data"}`), &expectedJsonDoc))
-	
+
 	expectedRows := []sql.Row{
 		{"view", "view1", "SELECT v1 FROM test;", gmstypes.JSONDocument{Val: expectedJsonDoc}},
 		{"view", "view2", "SELECT v2 FROM test;", nil},
diff --git a/go/libraries/doltcore/sqle/tables.go b/go/libraries/doltcore/sqle/tables.go
index 4eafd6386b..53c6f9f52c 100644
--- a/go/libraries/doltcore/sqle/tables.go
+++ b/go/libraries/doltcore/sqle/tables.go
@@ -80,7 +80,7 @@ type projected interface {
 // DoltTable implements the sql.Table interface and gives access to dolt table rows and schema.
 type DoltTable struct {
 	tableName    string
-	 sqlSch       sql.PrimaryKeySchema
+	sqlSch       sql.PrimaryKeySchema
 	db           SqlDatabase
 	lockedToRoot *doltdb.RootValue
 	nbf          *types.NomsBinFormat

From b228edc046392314477c7a5285c5ca8b1eb8177d Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 17:39:36 -0800
Subject: [PATCH 16/40] Fixed tests, made rename illegal for dolt_schemas

---
 go/libraries/doltcore/doltdb/system_table.go |  5 +++--
 go/libraries/doltcore/sqle/database.go       |  4 ++--
 go/libraries/doltcore/sqle/sqlddl_test.go    | 13 ++++---------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go
index 9347bcc2f4..dc616593f2 100644
--- a/go/libraries/doltcore/doltdb/system_table.go
+++ b/go/libraries/doltcore/doltdb/system_table.go
@@ -66,8 +66,9 @@ func IsReadOnlySystemTable(name string) bool {
 	return HasDoltPrefix(name) && !set.NewStrSet(writeableSystemTables).Contains(name)
 }
 
-// IsNonDroppableSystemTable returns whether the table name given is a system table that cannot be dropped.
-func IsNonDroppableSystemTable(name string) bool {
+// IsNonAlterableSystemTable returns whether the table name given is a system table that cannot be dropped or altered 
+// by the user.
+func IsNonAlterableSystemTable(name string) bool {
 	return IsReadOnlySystemTable(name) || strings.ToLower(name) == SchemasTableName
 }
 
diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 99110c2af5..4ccb50811c 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -733,7 +733,7 @@ func (db Database) DropTable(ctx *sql.Context, tableName string) error {
 	if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil {
 		return err
 	}
-	if doltdb.IsNonDroppableSystemTable(tableName) {
+	if doltdb.IsNonAlterableSystemTable(tableName) {
 		return ErrSystemTableAlter.New(tableName)
 	}
 
@@ -1031,7 +1031,7 @@ func (db Database) RenameTable(ctx *sql.Context, oldName, newName string) error
 		return err
 	}
 
-	if doltdb.IsReadOnlySystemTable(oldName) {
+	if doltdb.IsNonAlterableSystemTable(oldName) {
 		return ErrSystemTableAlter.New(oldName)
 	}
 
diff --git a/go/libraries/doltcore/sqle/sqlddl_test.go b/go/libraries/doltcore/sqle/sqlddl_test.go
index d5987c3712..f815f39822 100644
--- a/go/libraries/doltcore/sqle/sqlddl_test.go
+++ b/go/libraries/doltcore/sqle/sqlddl_test.go
@@ -795,8 +795,8 @@ func TestRenameTableStatements(t *testing.T) {
 }
 
 func TestAlterSystemTables(t *testing.T) {
-	systemTableNames := []string{"dolt_log", "dolt_history_people", "dolt_diff_people", "dolt_commit_diff_people"} // "dolt_docs",
-	reservedTableNames := []string{"dolt_schemas", "dolt_query_catalog"}
+	systemTableNames := []string{"dolt_log", "dolt_history_people", "dolt_diff_people", "dolt_commit_diff_people", "dolt_schemas"} // "dolt_docs",
+	reservedTableNames := []string{"dolt_query_catalog"}
 
 	var dEnv *env.DoltEnv
 	var err error
@@ -825,15 +825,10 @@ func TestAlterSystemTables(t *testing.T) {
 		}
 	})
 
-	// The _history and _diff tables give not found errors right now because of https://github.com/dolthub/dolt/issues/373.
-	// We can remove the divergent failure logic when the issue is fixed.
 	t.Run("Drop", func(t *testing.T) {
 		setup()
-		for _, tableName := range systemTableNames {
-			expectedErr := "system table"
-			if strings.HasPrefix(tableName, "dolt_diff") || strings.HasPrefix(tableName, "dolt_history") {
-				expectedErr = "system tables cannot be dropped or altered"
-			}
+		for _, tableName := range append(systemTableNames, "dolt_schemas") {
+			expectedErr := "system tables cannot be dropped or altered"
 			assertFails(t, dEnv, fmt.Sprintf("drop table %s", tableName), expectedErr)
 		}
 		for _, tableName := range reservedTableNames {

From 695666b60a225d866531c195c066d46cb384e002 Mon Sep 17 00:00:00 2001
From: zachmu <zachmu@users.noreply.github.com>
Date: Thu, 16 Feb 2023 01:41:11 +0000
Subject: [PATCH 17/40] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and
 go/Godeps/update.sh

---
 go/libraries/doltcore/doltdb/system_table.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go
index abc4a42aa3..9ef6b10f33 100644
--- a/go/libraries/doltcore/doltdb/system_table.go
+++ b/go/libraries/doltcore/doltdb/system_table.go
@@ -66,7 +66,7 @@ func IsReadOnlySystemTable(name string) bool {
 	return HasDoltPrefix(name) && !set.NewStrSet(writeableSystemTables).Contains(name)
 }
 
-// IsNonAlterableSystemTable returns whether the table name given is a system table that cannot be dropped or altered 
+// IsNonAlterableSystemTable returns whether the table name given is a system table that cannot be dropped or altered
 // by the user.
 func IsNonAlterableSystemTable(name string) bool {
 	return IsReadOnlySystemTable(name) || strings.ToLower(name) == SchemasTableName

From 69286e50ace60a30e7409fd3606d3dfdcd739dca Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Wed, 15 Feb 2023 18:02:55 -0800
Subject: [PATCH 18/40] Rewrote test to not compare json docs, which is failing
 in CI for some reason

---
 go/libraries/doltcore/sqle/schema_table_test.go | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index 2d833ede6c..4635d9c918 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -16,7 +16,6 @@ package sqle
 
 import (
 	"context"
-	"encoding/json"
 	"io"
 	"testing"
 
@@ -144,16 +143,19 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 		}
 
 		require.NoError(t, err)
+		// convert the JSONDocument to a string for comparison
+		if row[3] != nil {
+			row[3], err = row[3].(gmstypes.JSONDocument).ToString(nil)
+			require.NoError(t, err)
+		}
+		
 		rows = append(rows, row)
 	}
 
 	require.NoError(t, iter.Close(ctx))
 
-	var expectedJsonDoc any
-	require.NoError(t, json.Unmarshal([]byte(`{"extra": "data"}`), &expectedJsonDoc))
-
 	expectedRows := []sql.Row{
-		{"view", "view1", "SELECT v1 FROM test;", gmstypes.JSONDocument{Val: expectedJsonDoc}},
+		{"view", "view1", "SELECT v1 FROM test;", `{"extra":"data"}`},
 		{"view", "view2", "SELECT v2 FROM test;", nil},
 	}
 

From de049c87f3c5d7fdd9a5efbb65f2c14f84fbd74e Mon Sep 17 00:00:00 2001
From: zachmu <zachmu@users.noreply.github.com>
Date: Thu, 16 Feb 2023 02:04:19 +0000
Subject: [PATCH 19/40] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and
 go/Godeps/update.sh

---
 go/libraries/doltcore/sqle/schema_table_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index 4635d9c918..d452951836 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -148,7 +148,7 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 			row[3], err = row[3].(gmstypes.JSONDocument).ToString(nil)
 			require.NoError(t, err)
 		}
-		
+
 		rows = append(rows, row)
 	}
 

From 7e1e6715cd492a41f3349433fbb8ea722de31666 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 10:53:30 -0800
Subject: [PATCH 20/40] Fixed test

---
 integration-tests/bats/cherry-pick.bats | 40 ++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/integration-tests/bats/cherry-pick.bats b/integration-tests/bats/cherry-pick.bats
index 1f597f9ccb..541b5bf02b 100644
--- a/integration-tests/bats/cherry-pick.bats
+++ b/integration-tests/bats/cherry-pick.bats
@@ -276,6 +276,7 @@ teardown() {
 
 @test "cherry-pick: add triggers" {
     dolt sql -q "CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v = concat(new.v, ' inserted')"
+    dolt sql -q "CREATE view two as select 1+1"
     dolt sql -q "INSERT INTO test VALUES (4,'z')"
     run dolt sql -q "SELECT * FROM test"
     [[ "$output" =~ "z inserted" ]] || false
@@ -287,8 +288,7 @@ teardown() {
     run dolt sql -q "SHOW TRIGGERS"
     [[ ! "$output" =~ "trigger1" ]] || false
 
-    run dolt cherry-pick branch1
-    [ "$status" -eq "0" ]
+    dolt cherry-pick branch1
 
     run dolt sql -q "SELECT * FROM test"
     [[ "$output" =~ "z inserted" ]] || false
@@ -300,8 +300,40 @@ teardown() {
     dolt sql -q "DROP TRIGGER trigger1"
     dolt commit -am "drop trigger"
     dolt checkout main
-    run dolt cherry-pick branch1
-    [ "$status" -eq "0" ]
+    dolt cherry-pick branch1
+
+    run dolt sql -q "SHOW TRIGGERS"
+    [[ ! "$output" =~ "trigger1" ]] || false
+}
+
+@test "cherry-pick: drop triggers" {
+    dolt sql -q "CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v = concat(new.v, ' inserted')"
+    dolt sql -q "INSERT INTO test VALUES (4,'z')"
+    run dolt sql -q "SELECT * FROM test"
+    [[ "$output" =~ "z inserted" ]] || false
+
+    dolt add .
+    dolt commit -am "add trigger"
+
+    dolt checkout main
+    run dolt sql -q "SHOW TRIGGERS"
+    [[ ! "$output" =~ "trigger1" ]] || false
+
+    dolt cherry-pick branch1
+
+    run dolt sql -q "SELECT * FROM test"
+    [[ "$output" =~ "z inserted" ]] || false
+
+    run dolt sql -q "SHOW TRIGGERS"
+    [[ "$output" =~ "trigger1" ]] || false
+
+    dolt checkout branch1
+    dolt sql -q "DROP TRIGGER trigger1"
+    dolt commit -am "drop trigger"
+    dolt checkout main
+
+    skip "merge cannot handle dropped dolt_schemas" table
+    dolt cherry-pick branch1
 
     run dolt sql -q "SHOW TRIGGERS"
     [[ ! "$output" =~ "trigger1" ]] || false

From 145ddd6d7b680495ff56a3b15c6c7cf85d92601d Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 10:58:26 -0800
Subject: [PATCH 21/40] Fixed tests

---
 integration-tests/bats/merge.bats | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/integration-tests/bats/merge.bats b/integration-tests/bats/merge.bats
index 6aa0fc5a91..e2546bfd9d 100644
--- a/integration-tests/bats/merge.bats
+++ b/integration-tests/bats/merge.bats
@@ -378,12 +378,12 @@ SQL
     dolt checkout main
     run dolt merge other --no-commit
     log_status_eq 0
-    [[ "$output" =~ "CONFLICT" ]] || false
-    run dolt conflicts resolve --theirs dolt_schemas
-    log_status_eq 0
+    [[ ! "$output" =~ "CONFLICT" ]] || false
+
     run dolt sql -q "select name from dolt_schemas" -r csv
     log_status_eq 0
     [[ "$output" =~ "c1c1" ]] || false
+    [[ "$output" =~ "pkpk" ]] || false
 }
 
 @test "merge: Add views on two branches, merge with stored procedure" {
@@ -398,12 +398,12 @@ SQL
     dolt checkout main
     run dolt merge other --no-commit
     log_status_eq 0
-    [[ "$output" =~ "CONFLICT" ]] || false
-    run dolt sql -q "call dolt_conflicts_resolve('--theirs', 'dolt_schemas')"
-    log_status_eq 0
+    [[ ! "$output" =~ "CONFLICT" ]] || false
+    
     run dolt sql -q "select name from dolt_schemas" -r csv
     log_status_eq 0
     [[ "$output" =~ "c1c1" ]] || false
+    [[ "$output" =~ "pkpk" ]] || false
 }
 
 @test "merge: Add views on two branches, merge without conflicts" {

From a6fbdd66e56646f13aed7828ce48faeb443f0be2 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 10:59:31 -0800
Subject: [PATCH 22/40] Fixed test

---
 integration-tests/bats/sql-cherry-pick.bats | 1 +
 1 file changed, 1 insertion(+)

diff --git a/integration-tests/bats/sql-cherry-pick.bats b/integration-tests/bats/sql-cherry-pick.bats
index b9d7a50e78..243ba435a5 100644
--- a/integration-tests/bats/sql-cherry-pick.bats
+++ b/integration-tests/bats/sql-cherry-pick.bats
@@ -289,6 +289,7 @@ SQL
 
 @test "sql-cherry-pick: add triggers" {
     dolt sql -q "CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v = concat(new.v, ' inserted')"
+    dolt sql -q "CREATE view two as select 1+1"
     dolt sql -q "INSERT INTO test VALUES (4,'z')"
     run dolt sql -q "SELECT * FROM test"
     [[ "$output" =~ "z inserted" ]] || false

From 3de1d5ead1feee4cc2993fbd597919787d8fe82b Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 11:32:53 -0800
Subject: [PATCH 23/40] More broken tests

---
 integration-tests/bats/triggers.bats | 53 +++++++++-------------------
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/integration-tests/bats/triggers.bats b/integration-tests/bats/triggers.bats
index 45a7fc6489..fe91d16d25 100644
--- a/integration-tests/bats/triggers.bats
+++ b/integration-tests/bats/triggers.bats
@@ -63,18 +63,18 @@ SQL
     [[ "$output" =~ "y" ]] || false
     [[ "$output" =~ "4" ]] || false
     [[ "${#lines[@]}" = "2" ]] || false
-    run dolt sql -q "SELECT * FROM dolt_schemas ORDER BY id" -r=csv
+    run dolt sql -q "SELECT * FROM dolt_schemas ORDER BY name" -r=csv
     [ "$status" -eq "0" ]
-    [[ "$output" =~ "type,name,fragment,id" ]] || false
-    [[ "$output" =~ "trigger,trigger1,CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1,1" ]] || false
-    [[ "$output" =~ "view,view1,CREATE VIEW view1 AS SELECT v1 FROM test,2" ]] || false
-    [[ "$output" =~ "view,view2,CREATE VIEW view2 AS SELECT y FROM b,3" ]] || false
-    [[ "$output" =~ "trigger,trigger2,CREATE TRIGGER trigger2 AFTER INSERT ON a FOR EACH ROW INSERT INTO b VALUES (new.x * 2),4" ]] || false
+    [[ "$output" =~ "type,name,fragment" ]] || false
+    [[ "$output" =~ "trigger,trigger1,CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1" ]] || false
+    [[ "$output" =~ "view,view1,CREATE VIEW view1 AS SELECT v1 FROM test" ]] || false
+    [[ "$output" =~ "view,view2,CREATE VIEW view2 AS SELECT y FROM b" ]] || false
+    [[ "$output" =~ "trigger,trigger2,CREATE TRIGGER trigger2 AFTER INSERT ON a FOR EACH ROW INSERT INTO b VALUES (new.x * 2)" ]] || false
     [[ "${#lines[@]}" = "5" ]] || false
 }
 
 @test "triggers: Order preservation" {
-    skip "DROP TRIGGER not yet implemented and broken otherwise"
+    skip "DROP TRIGGER with dependencies not implemented"
     dolt sql <<SQL
 CREATE TABLE x (a BIGINT PRIMARY KEY);
 CREATE TRIGGER trigger1 BEFORE INSERT ON x FOR EACH ROW SET new.a = new.a + 1;
@@ -122,7 +122,7 @@ SQL
 @test "triggers: Writing directly into dolt_schemas" {
     dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v1 BIGINT);"
     dolt sql -q "CREATE VIEW view1 AS SELECT v1 FROM test;"
-    dolt sql -q "INSERT INTO dolt_schemas VALUES ('trigger', 'trigger1', 'CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1;', 2, json_object('CreatedAt', 1));"
+    dolt sql -q "INSERT INTO dolt_schemas VALUES ('trigger', 'trigger1', 'CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1;', json_object('CreatedAt', 1));"
     dolt sql -q "INSERT INTO test VALUES (1, 1);"
     run dolt sql -q "SELECT * FROM test" -r=csv
     [ "$status" -eq "0" ]
@@ -131,7 +131,7 @@ SQL
     [[ "${#lines[@]}" = "2" ]] || false
 }
 
-@test "triggers: Merge with manual resolution" {
+@test "triggers: Merge triggers on different branches, no conflict" {
     dolt sql <<SQL
 CREATE TABLE x(a BIGINT PRIMARY KEY);
 CREATE TRIGGER trigger1 BEFORE INSERT ON x FOR EACH ROW SET new.a = new.a + 1;
@@ -147,7 +147,6 @@ SQL
     dolt sql <<SQL
 CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100;
 CREATE TRIGGER trigger4 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 1000;
-UPDATE dolt_schemas SET id = id + 1 WHERE name = 'trigger4';
 SQL
     dolt add -A
     dolt commit -m "On other"
@@ -156,36 +155,18 @@ SQL
     [ "$status" -eq "0" ]
     [[ "$output" =~ "CREATE TRIGGER trigger2 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 10" ]] || false
     [[ "$output" =~ "CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100" ]] || false
+
     dolt merge other --no-commit
     dolt add dolt_schemas
-    run dolt commit -am "can't commit conflicts"
-    [ "$status" -eq "1" ]
-    [[ "$output" =~ "dolt_schemas" ]] || false
-    run dolt conflicts cat dolt_schemas
-    [ "$status" -eq "0" ]
-    [[ "$output" =~ "CREATE TRIGGER trigger2 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 10" ]] || false
-    [[ "$output" =~ "CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100" ]] || false
-
-    skip "SQL conflict resolution not working below"
-    
-    dolt sql  <<SQL
-    INSERT INTO dolt_schemas VALUES ('trigger', 'trigger3', 'CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100', 3);
-    DELETE FROM dolt_conflicts_dolt_schemas;
-    commit;
-SQL
-
-    run dolt conflicts cat dolt_schemas
-    [ "$status" -eq "0" ]
-    ! [[ "$output" =~ "CREATE TRIGGER" ]] || false
-    dolt add dolt_schemas
     dolt commit -m "Merged other table"
+
     run dolt sql -q "SELECT * FROM dolt_schemas" -r=csv
     [ "$status" -eq "0" ]
-    [[ "$output" =~ "type,name,fragment,id" ]] || false
-    [[ "$output" =~ "trigger,trigger1,CREATE TRIGGER trigger1 BEFORE INSERT ON x FOR EACH ROW SET new.a = new.a + 1,1" ]] || false
-    [[ "$output" =~ "trigger,trigger2,CREATE TRIGGER trigger2 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 10,2" ]] || false
-    [[ "$output" =~ "trigger,trigger3,CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100,3" ]] || false
-    [[ "$output" =~ "trigger,trigger4,CREATE TRIGGER trigger4 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 1000,4" ]] || false
+    [[ "$output" =~ "type,name,fragment" ]] || false
+    [[ "$output" =~ "trigger,trigger1,CREATE TRIGGER trigger1 BEFORE INSERT ON x FOR EACH ROW SET new.a = new.a + 1" ]] || false
+    [[ "$output" =~ "trigger,trigger2,CREATE TRIGGER trigger2 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 10" ]] || false
+    [[ "$output" =~ "trigger,trigger3,CREATE TRIGGER trigger3 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 100" ]] || false
+    [[ "$output" =~ "trigger,trigger4,CREATE TRIGGER trigger4 BEFORE INSERT ON x FOR EACH ROW SET new.a = (new.a * 2) + 1000" ]] || false
     [[ "${#lines[@]}" = "5" ]] || false
 }
 
@@ -209,7 +190,7 @@ SQL
     # creating a new view/trigger will recreate the dolt_schemas table
     dolt sql -q "CREATE VIEW view2 AS SELECT 3+3 FROM dual;"
 
-    skip "broken with new diff"
+    skip "diff is broken on schema change"
     run dolt diff
     [ "$status" -eq "0" ]
     [[ "$output" =~ "deleted table" ]] || false

From b7353c7b68005c4ec13d115b795d126e43d778df Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 11:35:59 -0800
Subject: [PATCH 24/40] PR feedback

---
 go/libraries/doltcore/sqle/sqlddl_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go/libraries/doltcore/sqle/sqlddl_test.go b/go/libraries/doltcore/sqle/sqlddl_test.go
index f815f39822..2fe7746b1c 100644
--- a/go/libraries/doltcore/sqle/sqlddl_test.go
+++ b/go/libraries/doltcore/sqle/sqlddl_test.go
@@ -827,7 +827,7 @@ func TestAlterSystemTables(t *testing.T) {
 
 	t.Run("Drop", func(t *testing.T) {
 		setup()
-		for _, tableName := range append(systemTableNames, "dolt_schemas") {
+		for _, tableName := range append(systemTableNames) {
 			expectedErr := "system tables cannot be dropped or altered"
 			assertFails(t, dEnv, fmt.Sprintf("drop table %s", tableName), expectedErr)
 		}

From d9acefa0ee9603a9fe9f00fd6dbc484442a168fb Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 11:50:06 -0800
Subject: [PATCH 25/40] Fix test for older dolt format

---
 go/libraries/doltcore/sqle/schema_table_test.go | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index d452951836..b324580327 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -19,6 +19,7 @@ import (
 	"io"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/json"
 	"github.com/dolthub/go-mysql-server/sql"
 	gmstypes "github.com/dolthub/go-mysql-server/sql/types"
 	"github.com/stretchr/testify/assert"
@@ -145,7 +146,17 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 		require.NoError(t, err)
 		// convert the JSONDocument to a string for comparison
 		if row[3] != nil {
-			row[3], err = row[3].(gmstypes.JSONDocument).ToString(nil)
+			// Annoying difference in representation between storage versions here
+			jsonDoc, ok := row[3].(gmstypes.JSONDocument)
+			if ok {
+				row[3], err = jsonDoc.ToString(nil)
+			}
+
+			nomsJson, ok := row[3].(json.NomsJSON)
+			if ok {
+				row[3], err = nomsJson.ToString(ctx)
+			}
+			
 			require.NoError(t, err)
 		}
 

From ca469929a72d3f64625d2c8e30bde3de15a71e24 Mon Sep 17 00:00:00 2001
From: zachmu <zachmu@users.noreply.github.com>
Date: Thu, 16 Feb 2023 19:51:30 +0000
Subject: [PATCH 26/40] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and
 go/Godeps/update.sh

---
 go/libraries/doltcore/sqle/schema_table_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index b324580327..2966a89a92 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -19,7 +19,6 @@ import (
 	"io"
 	"testing"
 
-	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/json"
 	"github.com/dolthub/go-mysql-server/sql"
 	gmstypes "github.com/dolthub/go-mysql-server/sql/types"
 	"github.com/stretchr/testify/assert"
@@ -28,6 +27,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/dtestutils"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
+	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/json"
 	"github.com/dolthub/dolt/go/libraries/doltcore/table/editor"
 )
 
@@ -156,7 +156,7 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 			if ok {
 				row[3], err = nomsJson.ToString(ctx)
 			}
-			
+
 			require.NoError(t, err)
 		}
 

From 493c674e6c336677341fbc499c30c2fe31e87767 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 12:21:09 -0800
Subject: [PATCH 27/40] Another test fix

---
 go/libraries/doltcore/sqle/schema_table_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index b324580327..5283be7e35 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -123,7 +123,8 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 	require.True(t, found)
 
 	inserter := sqlTbl.(*WritableDoltTable).Inserter(ctx)
-	err = inserter.Insert(ctx, sql.Row{"view", "view1", "SELECT v1 FROM test;", 1, `{"extra": "data"}`})
+	// JSON string has no spaces because our various JSON libraries don't agree on how to marshall it
+	err = inserter.Insert(ctx, sql.Row{"view", "view1", "SELECT v1 FROM test;", 1, `{"extra":"data"}`})
 	require.NoError(t, err)
 	err = inserter.Insert(ctx, sql.Row{"view", "view2", "SELECT v2 FROM test;", 2, nil})
 	require.NoError(t, err)

From cdc3baadfac0260bd2f20bc44822f78b3ccc10e8 Mon Sep 17 00:00:00 2001
From: Zach Musgrave <zach@dolthub.com>
Date: Thu, 16 Feb 2023 12:33:56 -0800
Subject: [PATCH 28/40] More annoying json formatting fixes

---
 go/libraries/doltcore/sqle/schema_table_test.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go
index 15949fb939..d0e602aa51 100644
--- a/go/libraries/doltcore/sqle/schema_table_test.go
+++ b/go/libraries/doltcore/sqle/schema_table_test.go
@@ -17,6 +17,7 @@ package sqle
 import (
 	"context"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/dolthub/go-mysql-server/sql"
@@ -151,11 +152,13 @@ func TestSchemaTableMigrationV1(t *testing.T) {
 			jsonDoc, ok := row[3].(gmstypes.JSONDocument)
 			if ok {
 				row[3], err = jsonDoc.ToString(nil)
+				row[3] = strings.ReplaceAll(row[3].(string), " ", "") // remove spaces
 			}
 
 			nomsJson, ok := row[3].(json.NomsJSON)
 			if ok {
 				row[3], err = nomsJson.ToString(ctx)
+				row[3] = strings.ReplaceAll(row[3].(string), " ", "") // remove spaces
 			}
 
 			require.NoError(t, err)

From 5a662bd8b1a8aa0a0f8cb88007fa10e8454a4a0a Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Thu, 16 Feb 2023 12:49:40 -0800
Subject: [PATCH 29/40] go/store/nbs: rename chunks journal constants in
 preparation for journal index

---
 go/store/nbs/journal_record.go      | 132 ++++++++++++++--------------
 go/store/nbs/journal_record_test.go |  64 +++++++-------
 go/store/nbs/journal_writer.go      |   4 +-
 3 files changed, 100 insertions(+), 100 deletions(-)

diff --git a/go/store/nbs/journal_record.go b/go/store/nbs/journal_record.go
index 35acd18751..1c64ff5f20 100644
--- a/go/store/nbs/journal_record.go
+++ b/go/store/nbs/journal_record.go
@@ -24,7 +24,7 @@ import (
 	"github.com/dolthub/dolt/go/store/d"
 )
 
-// journalRec is a record in a chunk journal. It's serialization format uses
+// journalRec is a record in a chunk journal. Its serialization format uses
 // uint8 tag prefixes to identify fields and allow for format evolution.
 //
 // There are two kinds of journalRecs: chunk records and root hash records.
@@ -43,7 +43,7 @@ import (
 // offset. See recLookup for more detail.
 type journalRec struct {
 	length   uint32
-	kind     recKind
+	kind     journalRecKind
 	address  addr
 	payload  []byte
 	checksum uint32
@@ -52,7 +52,7 @@ type journalRec struct {
 // payloadOffset returns the journalOffset of the payload within the record
 // assuming only the checksum field follows the payload.
 func (r journalRec) payloadOffset() uint32 {
-	return r.length - uint32(len(r.payload)+recChecksumSz)
+	return r.length - uint32(len(r.payload)+journalRecChecksumSz)
 }
 
 // uncompressedPayloadSize returns the uncompressed size of the payload.
@@ -63,76 +63,76 @@ func (r journalRec) uncompressedPayloadSize() (sz uint64) {
 	return
 }
 
-type recKind uint8
+type journalRecKind uint8
 
 const (
-	unknownKind     recKind = 0
-	rootHashRecKind recKind = 1
-	chunkRecKind    recKind = 2
+	unknownJournalRecKind  journalRecKind = 0
+	rootHashJournalRecKind journalRecKind = 1
+	chunkJournalRecKind    journalRecKind = 2
 )
 
-type recTag uint8
+type journalRecTag uint8
 
 const (
-	unknownTag recTag = 0
-	kindTag    recTag = 1
-	addrTag    recTag = 2
-	payloadTag recTag = 3
+	unknownJournalRecTag journalRecTag = 0
+	kindJournalRecTag    journalRecTag = 1
+	addrJournalRecTag    journalRecTag = 2
+	payloadJournalRecTag journalRecTag = 3
 )
 
 const (
-	recTagSz      = 1
-	recLenSz      = 4
-	recKindSz     = 1
-	recAddrSz     = 20
-	recChecksumSz = 4
+	journalRecTagSz      = 1
+	journalRecLenSz      = 4
+	journalRecKindSz     = 1
+	journalRecAddrSz     = 20
+	journalRecChecksumSz = 4
 
 	// todo(andy): less arbitrary
-	recMaxSz = 128 * 1024
+	journalRecMaxSz = 128 * 1024
 )
 
 func chunkRecordSize(c CompressedChunk) (recordSz, payloadOff uint32) {
-	recordSz += recLenSz
-	recordSz += recTagSz + recKindSz
-	recordSz += recTagSz + recAddrSz
-	recordSz += recTagSz // payload tag
+	recordSz += journalRecLenSz
+	recordSz += journalRecTagSz + journalRecKindSz
+	recordSz += journalRecTagSz + journalRecAddrSz
+	recordSz += journalRecTagSz // payload tag
 	payloadOff = recordSz
 	recordSz += uint32(len(c.FullCompressedChunk))
-	recordSz += recChecksumSz
+	recordSz += journalRecChecksumSz
 	return
 }
 
 func rootHashRecordSize() (recordSz int) {
-	recordSz += recLenSz
-	recordSz += recTagSz + recKindSz
-	recordSz += recTagSz + recAddrSz
-	recordSz += recChecksumSz
+	recordSz += journalRecLenSz
+	recordSz += journalRecTagSz + journalRecKindSz
+	recordSz += journalRecTagSz + journalRecAddrSz
+	recordSz += journalRecChecksumSz
 	return
 }
 
 func writeChunkRecord(buf []byte, c CompressedChunk) (n uint32) {
 	// length
 	l, _ := chunkRecordSize(c)
-	writeUint(buf[:recLenSz], l)
-	n += recLenSz
+	writeUint(buf[:journalRecLenSz], l)
+	n += journalRecLenSz
 	// kind
-	buf[n] = byte(kindTag)
-	n += recTagSz
-	buf[n] = byte(chunkRecKind)
-	n += recKindSz
+	buf[n] = byte(kindJournalRecTag)
+	n += journalRecTagSz
+	buf[n] = byte(chunkJournalRecKind)
+	n += journalRecKindSz
 	// address
-	buf[n] = byte(addrTag)
-	n += recTagSz
+	buf[n] = byte(addrJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], c.H[:])
-	n += recAddrSz
+	n += journalRecAddrSz
 	// payload
-	buf[n] = byte(payloadTag)
-	n += recTagSz
+	buf[n] = byte(payloadJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], c.FullCompressedChunk)
 	n += uint32(len(c.FullCompressedChunk))
 	// checksum
 	writeUint(buf[n:], crc(buf[:n]))
-	n += recChecksumSz
+	n += journalRecChecksumSz
 	d.PanicIfFalse(l == n)
 	return
 }
@@ -140,62 +140,62 @@ func writeChunkRecord(buf []byte, c CompressedChunk) (n uint32) {
 func writeRootHashRecord(buf []byte, root addr) (n uint32) {
 	// length
 	l := rootHashRecordSize()
-	writeUint(buf[:recLenSz], uint32(l))
-	n += recLenSz
+	writeUint(buf[:journalRecLenSz], uint32(l))
+	n += journalRecLenSz
 	// kind
-	buf[n] = byte(kindTag)
-	n += recTagSz
-	buf[n] = byte(rootHashRecKind)
-	n += recKindSz
+	buf[n] = byte(kindJournalRecTag)
+	n += journalRecTagSz
+	buf[n] = byte(rootHashJournalRecKind)
+	n += journalRecKindSz
 	// address
-	buf[n] = byte(addrTag)
-	n += recTagSz
+	buf[n] = byte(addrJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], root[:])
-	n += recAddrSz
+	n += journalRecAddrSz
 	// empty payload
 	// checksum
 	writeUint(buf[n:], crc(buf[:n]))
-	n += recChecksumSz
+	n += journalRecChecksumSz
 	return
 }
 
 func readJournalRecord(buf []byte) (rec journalRec, err error) {
 	rec.length = readUint(buf)
-	buf = buf[recLenSz:]
-	for len(buf) > recChecksumSz {
-		tag := recTag(buf[0])
-		buf = buf[recTagSz:]
+	buf = buf[journalRecLenSz:]
+	for len(buf) > journalRecChecksumSz {
+		tag := journalRecTag(buf[0])
+		buf = buf[journalRecTagSz:]
 		switch tag {
-		case kindTag:
-			rec.kind = recKind(buf[0])
-			buf = buf[recKindSz:]
-		case addrTag:
+		case kindJournalRecTag:
+			rec.kind = journalRecKind(buf[0])
+			buf = buf[journalRecKindSz:]
+		case addrJournalRecTag:
 			copy(rec.address[:], buf)
-			buf = buf[recAddrSz:]
-		case payloadTag:
-			sz := len(buf) - recChecksumSz
+			buf = buf[journalRecAddrSz:]
+		case payloadJournalRecTag:
+			sz := len(buf) - journalRecChecksumSz
 			rec.payload = buf[:sz]
 			buf = buf[sz:]
-		case unknownTag:
+		case unknownJournalRecTag:
 			fallthrough
 		default:
 			err = fmt.Errorf("unknown record field tag: %d", tag)
 			return
 		}
 	}
-	rec.checksum = readUint(buf[:recChecksumSz])
+	rec.checksum = readUint(buf[:journalRecChecksumSz])
 	return
 }
 
 func validateJournalRecord(buf []byte) (ok bool) {
-	if len(buf) > (recLenSz + recChecksumSz) {
-		off := len(buf) - recChecksumSz
+	if len(buf) > (journalRecLenSz + journalRecChecksumSz) {
+		off := len(buf) - journalRecChecksumSz
 		ok = crc(buf[:off]) == readUint(buf[off:])
 	}
 	return
 }
 
-func processRecords(ctx context.Context, r io.ReadSeeker, cb func(o int64, r journalRec) error) (int64, error) {
+func processJournalRecords(ctx context.Context, r io.ReadSeeker, cb func(o int64, r journalRec) error) (int64, error) {
 	var (
 		buf []byte
 		off int64
@@ -210,7 +210,7 @@ func processRecords(ctx context.Context, r io.ReadSeeker, cb func(o int64, r jou
 		}
 
 		l := readUint(buf)
-		if l > recMaxSz {
+		if l > journalRecMaxSz {
 			break
 		} else if buf, err = rdr.Peek(int(l)); err != nil {
 			break
diff --git a/go/store/nbs/journal_record_test.go b/go/store/nbs/journal_record_test.go
index 08e0ef679c..a1bb86e3ef 100644
--- a/go/store/nbs/journal_record_test.go
+++ b/go/store/nbs/journal_record_test.go
@@ -97,7 +97,7 @@ func TestProcessRecords(t *testing.T) {
 		return
 	}
 
-	n, err := processRecords(ctx, bytes.NewReader(journal), check)
+	n, err := processJournalRecords(ctx, bytes.NewReader(journal), check)
 	assert.Equal(t, cnt, i)
 	assert.Equal(t, int(off), int(n))
 	require.NoError(t, err)
@@ -105,7 +105,7 @@ func TestProcessRecords(t *testing.T) {
 	i, sum = 0, 0
 	// write a bogus record to the end and process again
 	writeCorruptRecord(journal[off:])
-	n, err = processRecords(ctx, bytes.NewReader(journal), check)
+	n, err = processJournalRecords(ctx, bytes.NewReader(journal), check)
 	assert.Equal(t, cnt, i)
 	assert.Equal(t, int(off), int(n))
 	require.NoError(t, err)
@@ -134,29 +134,29 @@ func makeChunkRecord() (journalRec, []byte) {
 	buf := make([]byte, sz)
 	// length
 	writeUint(buf[n:], uint32(len(buf)))
-	n += recLenSz
+	n += journalRecLenSz
 	// kind
-	buf[n] = byte(kindTag)
-	n += recTagSz
-	buf[n] = byte(chunkRecKind)
-	n += recKindSz
+	buf[n] = byte(kindJournalRecTag)
+	n += journalRecTagSz
+	buf[n] = byte(chunkJournalRecKind)
+	n += journalRecKindSz
 	// address
-	buf[n] = byte(addrTag)
-	n += recTagSz
+	buf[n] = byte(addrJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], cc.H[:])
-	n += recAddrSz
+	n += journalRecAddrSz
 	// payload
-	buf[n] = byte(payloadTag)
-	n += recTagSz
+	buf[n] = byte(payloadJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], payload)
 	n += len(payload)
 	// checksum
-	c := crc(buf[:len(buf)-recChecksumSz])
-	writeUint(buf[len(buf)-recChecksumSz:], c)
+	c := crc(buf[:len(buf)-journalRecChecksumSz])
+	writeUint(buf[len(buf)-journalRecChecksumSz:], c)
 
 	r := journalRec{
 		length:   uint32(len(buf)),
-		kind:     chunkRecKind,
+		kind:     chunkJournalRecKind,
 		address:  addr(cc.H),
 		payload:  payload,
 		checksum: c,
@@ -170,23 +170,23 @@ func makeRootHashRecord() (journalRec, []byte) {
 	buf := make([]byte, rootHashRecordSize())
 	// length
 	writeUint(buf[n:], uint32(len(buf)))
-	n += recLenSz
+	n += journalRecLenSz
 	// kind
-	buf[n] = byte(kindTag)
-	n += recTagSz
-	buf[n] = byte(rootHashRecKind)
-	n += recKindSz
+	buf[n] = byte(kindJournalRecTag)
+	n += journalRecTagSz
+	buf[n] = byte(rootHashJournalRecKind)
+	n += journalRecKindSz
 	// address
-	buf[n] = byte(addrTag)
-	n += recTagSz
+	buf[n] = byte(addrJournalRecTag)
+	n += journalRecTagSz
 	copy(buf[n:], a[:])
-	n += recAddrSz
+	n += journalRecAddrSz
 	// checksum
-	c := crc(buf[:len(buf)-recChecksumSz])
-	writeUint(buf[len(buf)-recChecksumSz:], c)
+	c := crc(buf[:len(buf)-journalRecChecksumSz])
+	writeUint(buf[len(buf)-journalRecChecksumSz:], c)
 	r := journalRec{
 		length:   uint32(len(buf)),
-		kind:     rootHashRecKind,
+		kind:     rootHashJournalRecKind,
 		address:  a,
 		checksum: c,
 	}
@@ -194,13 +194,13 @@ func makeRootHashRecord() (journalRec, []byte) {
 }
 
 func makeUnknownTagRecord() (buf []byte) {
-	const fakeTag recTag = 111
+	const fakeTag journalRecTag = 111
 	_, buf = makeRootHashRecord()
 	// overwrite recKind
-	buf[recLenSz] = byte(fakeTag)
+	buf[journalRecLenSz] = byte(fakeTag)
 	// redo checksum
-	c := crc(buf[:len(buf)-recChecksumSz])
-	writeUint(buf[len(buf)-recChecksumSz:], c)
+	c := crc(buf[:len(buf)-journalRecChecksumSz])
+	writeUint(buf[len(buf)-journalRecChecksumSz:], c)
 	return
 }
 
@@ -210,12 +210,12 @@ func writeCorruptRecord(buf []byte) (n uint32) {
 	rand.Read(buf[:n])
 	// write a valid size, kind
 	writeUint(buf, n)
-	buf[recLenSz] = byte(rootHashRecKind)
+	buf[journalRecLenSz] = byte(rootHashJournalRecKind)
 	return
 }
 
 func mustCompressedChunk(rec journalRec) CompressedChunk {
-	d.PanicIfFalse(rec.kind == chunkRecKind)
+	d.PanicIfFalse(rec.kind == chunkJournalRecKind)
 	cc, err := NewCompressedChunk(hash.Hash(rec.address), rec.payload)
 	d.PanicIfError(err)
 	return cc
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index 9e1dc2a38e..1b68dfbdf2 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -220,14 +220,14 @@ func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, cs
 	}
 	wr.off, err = processRecords(ctx, wr.file, func(o int64, r journalRec) error {
 		switch r.kind {
-		case chunkRecKind:
+		case chunkJournalRecKind:
 			src.lookups.put(r.address, recLookup{
 				journalOff: o,
 				recordLen:  r.length,
 				payloadOff: r.payloadOffset(),
 			})
 			src.uncompressedSz += r.uncompressedPayloadSize()
-		case rootHashRecKind:
+		case rootHashJournalRecKind:
 			last = hash.Hash(r.address)
 		default:
 			return fmt.Errorf("unknown journal record kind (%d)", r.kind)

From 2513f49e2c3027cf9e448ebfbca7f3d4df3ca61d Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Thu, 16 Feb 2023 13:01:28 -0800
Subject: [PATCH 30/40] go/store/nbs: write code that compiles

---
 go/store/nbs/journal_chunk_source.go | 2 +-
 go/store/nbs/journal_writer.go       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/go/store/nbs/journal_chunk_source.go b/go/store/nbs/journal_chunk_source.go
index eb599139dd..bc7b88090d 100644
--- a/go/store/nbs/journal_chunk_source.go
+++ b/go/store/nbs/journal_chunk_source.go
@@ -45,7 +45,7 @@ func rangeFromLookup(l recLookup) Range {
 	return Range{
 		// see journalRec for serialization format
 		Offset: uint64(l.journalOff) + uint64(l.payloadOff),
-		Length: l.recordLen - (l.payloadOff + recChecksumSz),
+		Length: l.recordLen - (l.payloadOff + journalRecChecksumSz),
 	}
 }
 
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index 1b68dfbdf2..7a78eb7439 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -218,7 +218,7 @@ func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, cs
 		address: journalAddr,
 		lookups: newLookupMap(),
 	}
-	wr.off, err = processRecords(ctx, wr.file, func(o int64, r journalRec) error {
+	wr.off, err = processJournalRecords(ctx, wr.file, func(o int64, r journalRec) error {
 		switch r.kind {
 		case chunkJournalRecKind:
 			src.lookups.put(r.address, recLookup{

From 7406c4658a12abdfe4660aa2ee3d06cc7a235ba5 Mon Sep 17 00:00:00 2001
From: Aaron Son <aaron@dolthub.com>
Date: Thu, 16 Feb 2023 16:01:18 -0800
Subject: [PATCH 31/40] go/store/nbs: Fix some quota leaks in conjoin, GC.

Adds a paranoid mode where we noisely detect unclosed table files. The mode can
be enabled by setting an environment variable.

Fixes some unit tests, including all of go/store/... to run cleanly under the
paranoid mode.

Changes the quota interface to:
* Release |sz int| bytes instead of requiring a []byte with the correct length
  to show up.
* Work with |int| instead of |uint64|, since MaxUint64 is never allocatable and
  MaxInt32+z is only allocatable on 64-bit platforms.
* Not return an error on Release(). Implementations should not fail to release
  quota.
---
 go/cmd/dolt/commands/sqlserver/server_test.go | 44 ++++++++++---
 .../dtestutils/testcommands/multienv.go       |  9 +++
 go/store/cmd/noms/noms_root.go                |  1 +
 go/store/datas/pull/puller_test.go            |  3 +-
 go/store/nbs/aws_chunk_source.go              |  6 +-
 go/store/nbs/aws_chunk_source_test.go         |  2 +
 go/store/nbs/aws_table_persister_test.go      | 36 ++++++++++
 go/store/nbs/block_store_test.go              |  3 +
 go/store/nbs/cmp_chunk_table_writer_test.go   |  2 +
 go/store/nbs/conjoiner.go                     |  8 +++
 go/store/nbs/conjoiner_test.go                | 10 +++
 go/store/nbs/file_table_persister_test.go     | 28 +++++++-
 go/store/nbs/file_table_reader.go             | 15 +++--
 go/store/nbs/file_table_reader_test.go        |  1 +
 go/store/nbs/mem_table_test.go                |  3 +
 go/store/nbs/quota.go                         | 19 +++---
 go/store/nbs/s3_fake_test.go                  |  2 +-
 go/store/nbs/store.go                         |  4 +-
 go/store/nbs/store_test.go                    |  3 +
 go/store/nbs/table_index.go                   | 65 +++++++++++++------
 go/store/nbs/table_index_test.go              |  3 +
 go/store/nbs/table_persister_test.go          | 12 +++-
 go/store/nbs/table_set_test.go                | 12 ++++
 go/store/nbs/table_test.go                    |  9 +++
 go/store/spec/spec_test.go                    | 10 ++-
 25 files changed, 255 insertions(+), 55 deletions(-)

diff --git a/go/cmd/dolt/commands/sqlserver/server_test.go b/go/cmd/dolt/commands/sqlserver/server_test.go
index 4af85cacc9..f78243af8b 100644
--- a/go/cmd/dolt/commands/sqlserver/server_test.go
+++ b/go/cmd/dolt/commands/sqlserver/server_test.go
@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 	"testing"
 
 	_ "github.com/go-sql-driver/mysql"
@@ -61,10 +62,12 @@ var (
 
 func TestServerArgs(t *testing.T) {
 	serverController := NewServerController()
+	dEnv, err := sqle.CreateEnvWithSeedData()
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, dEnv.DoltDB.Close())
+	}()
 	go func() {
-		dEnv, err := sqle.CreateEnvWithSeedData()
-		require.NoError(t, err)
-
 		startServer(context.Background(), "0.0.0", "dolt sql-server", []string{
 			"-H", "localhost",
 			"-P", "15200",
@@ -75,7 +78,7 @@ func TestServerArgs(t *testing.T) {
 			"-r",
 		}, dEnv, serverController)
 	}()
-	err := serverController.WaitForStart()
+	err = serverController.WaitForStart()
 	require.NoError(t, err)
 	conn, err := dbr.Open("mysql", "username:password@tcp(localhost:15200)/", nil)
 	require.NoError(t, err)
@@ -103,17 +106,20 @@ listener:
     read_timeout_millis: 5000
     write_timeout_millis: 5000
 `
+	dEnv, err := sqle.CreateEnvWithSeedData()
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, dEnv.DoltDB.Close())
+	}()
 	serverController := NewServerController()
 	go func() {
-		dEnv, err := sqle.CreateEnvWithSeedData()
-		require.NoError(t, err)
 
 		dEnv.FS.WriteFile("config.yaml", []byte(yamlConfig))
 		startServer(context.Background(), "0.0.0", "dolt sql-server", []string{
 			"--config", "config.yaml",
 		}, dEnv, serverController)
 	}()
-	err := serverController.WaitForStart()
+	err = serverController.WaitForStart()
 	require.NoError(t, err)
 	conn, err := dbr.Open("mysql", "username:password@tcp(localhost:15200)/", nil)
 	require.NoError(t, err)
@@ -127,6 +133,9 @@ listener:
 func TestServerBadArgs(t *testing.T) {
 	env, err := sqle.CreateEnvWithSeedData()
 	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, env.DoltDB.Close())
+	}()
 
 	tests := [][]string{
 		{"-H", "127.0.0.0.1"},
@@ -156,6 +165,9 @@ func TestServerBadArgs(t *testing.T) {
 func TestServerGoodParams(t *testing.T) {
 	env, err := sqle.CreateEnvWithSeedData()
 	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, env.DoltDB.Close())
+	}()
 
 	tests := []ServerConfig{
 		DefaultServerConfig(),
@@ -195,6 +207,9 @@ func TestServerGoodParams(t *testing.T) {
 func TestServerSelect(t *testing.T) {
 	env, err := sqle.CreateEnvWithSeedData()
 	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, env.DoltDB.Close())
+	}()
 
 	serverConfig := DefaultServerConfig().withLogLevel(LogLevel_Fatal).WithPort(15300)
 
@@ -254,8 +269,16 @@ func TestServerFailsIfPortInUse(t *testing.T) {
 	}
 	dEnv, err := sqle.CreateEnvWithSeedData()
 	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, dEnv.DoltDB.Close())
+	}()
 
-	go server.ListenAndServe()
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		server.ListenAndServe()
+	}()
 	go func() {
 		startServer(context.Background(), "test", "dolt sql-server", []string{
 			"-H", "localhost",
@@ -271,11 +294,15 @@ func TestServerFailsIfPortInUse(t *testing.T) {
 	err = serverController.WaitForStart()
 	require.Error(t, err)
 	server.Close()
+	wg.Wait()
 }
 
 func TestServerSetDefaultBranch(t *testing.T) {
 	dEnv, err := sqle.CreateEnvWithSeedData()
 	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, dEnv.DoltDB.Close())
+	}()
 
 	serverConfig := DefaultServerConfig().withLogLevel(LogLevel_Fatal).WithPort(15302)
 
@@ -408,6 +435,7 @@ func TestReadReplica(t *testing.T) {
 	defer os.Chdir(cwd)
 
 	multiSetup := testcommands.NewMultiRepoTestSetup(t.Fatal)
+	defer multiSetup.Close()
 	defer os.RemoveAll(multiSetup.Root)
 
 	multiSetup.NewDB("read_replica")
diff --git a/go/libraries/doltcore/dtestutils/testcommands/multienv.go b/go/libraries/doltcore/dtestutils/testcommands/multienv.go
index 96fa0b9111..be2b0a945a 100644
--- a/go/libraries/doltcore/dtestutils/testcommands/multienv.go
+++ b/go/libraries/doltcore/dtestutils/testcommands/multienv.go
@@ -84,6 +84,15 @@ func (mr *MultiRepoTestSetup) homeProv() (string, error) {
 	return mr.Home, nil
 }
 
+func (mr *MultiRepoTestSetup) Close() {
+	for _, db := range mr.DoltDBs {
+		err := db.Close()
+		if err != nil {
+			mr.Errhand(err)
+		}
+	}
+}
+
 func (mr *MultiRepoTestSetup) Cleanup(dbName string) {
 	os.RemoveAll(mr.Root)
 }
diff --git a/go/store/cmd/noms/noms_root.go b/go/store/cmd/noms/noms_root.go
index 3377d00ed2..8086b7e231 100644
--- a/go/store/cmd/noms/noms_root.go
+++ b/go/store/cmd/noms/noms_root.go
@@ -62,6 +62,7 @@ func runRoot(ctx context.Context, args []string) int {
 	cfg := config.NewResolver()
 	cs, err := cfg.GetChunkStore(ctx, args[0])
 	util.CheckErrorNoUsage(err)
+	defer cs.Close()
 
 	currRoot, err := cs.Root(ctx)
 
diff --git a/go/store/datas/pull/puller_test.go b/go/store/datas/pull/puller_test.go
index a88adbc38d..bcbbfb9027 100644
--- a/go/store/datas/pull/puller_test.go
+++ b/go/store/datas/pull/puller_test.go
@@ -164,6 +164,7 @@ type datasFactory func(context.Context) (types.ValueReadWriter, datas.Database)
 func testPuller(t *testing.T, makeDB datasFactory) {
 	ctx := context.Background()
 	vs, db := makeDB(ctx)
+	defer db.Close()
 
 	deltas := []struct {
 		name       string
@@ -325,6 +326,7 @@ func testPuller(t *testing.T, makeDB datasFactory) {
 			}()
 
 			sinkvs, sinkdb := makeDB(ctx)
+			defer sinkdb.Close()
 
 			tmpDir := filepath.Join(os.TempDir(), uuid.New().String())
 			err = os.MkdirAll(tmpDir, os.ModePerm)
@@ -351,7 +353,6 @@ func testPuller(t *testing.T, makeDB datasFactory) {
 			eq, err := pullerAddrEquality(ctx, rootAddr, sinkRootAddr, vs, sinkvs)
 			require.NoError(t, err)
 			assert.True(t, eq)
-
 		})
 	}
 }
diff --git a/go/store/nbs/aws_chunk_source.go b/go/store/nbs/aws_chunk_source.go
index 4a8df217a0..b08da41a9e 100644
--- a/go/store/nbs/aws_chunk_source.go
+++ b/go/store/nbs/aws_chunk_source.go
@@ -98,14 +98,14 @@ func newAWSChunkSource(ctx context.Context, ddb *ddbTableStore, s3 *s3ObjectRead
 func loadTableIndex(ctx context.Context, stats *Stats, cnt uint32, q MemoryQuotaProvider, loadIndexBytes func(p []byte) error) (tableIndex, error) {
 	idxSz := int(indexSize(cnt) + footerSize)
 	offsetSz := int((cnt - (cnt / 2)) * offsetSize)
-	buf, err := q.AcquireQuotaBytes(ctx, uint64(idxSz+offsetSz))
+	buf, err := q.AcquireQuotaBytes(ctx, idxSz+offsetSz)
 	if err != nil {
 		return nil, err
 	}
 
 	t1 := time.Now()
 	if err := loadIndexBytes(buf[:idxSz]); err != nil {
-		q.ReleaseQuotaBytes(buf)
+		q.ReleaseQuotaBytes(len(buf))
 		return nil, err
 	}
 	stats.IndexReadLatency.SampleTimeSince(t1)
@@ -113,7 +113,7 @@ func loadTableIndex(ctx context.Context, stats *Stats, cnt uint32, q MemoryQuota
 
 	idx, err := parseTableIndexWithOffsetBuff(buf[:idxSz], buf[idxSz:], q)
 	if err != nil {
-		q.ReleaseQuotaBytes(buf)
+		q.ReleaseQuotaBytes(len(buf))
 	}
 	return idx, err
 }
diff --git a/go/store/nbs/aws_chunk_source_test.go b/go/store/nbs/aws_chunk_source_test.go
index ad51cb5bdc..697962d806 100644
--- a/go/store/nbs/aws_chunk_source_test.go
+++ b/go/store/nbs/aws_chunk_source_test.go
@@ -67,6 +67,7 @@ func TestAWSChunkSource(t *testing.T) {
 		t.Run("Has Chunks", func(t *testing.T) {
 			src := makeSrc(len(chunks) + 1)
 			assertChunksInReader(chunks, src, assert.New(t))
+			src.close()
 		})
 	})
 
@@ -76,6 +77,7 @@ func TestAWSChunkSource(t *testing.T) {
 		t.Run("Has Chunks", func(t *testing.T) {
 			src := makeSrc(len(chunks) - 1)
 			assertChunksInReader(chunks, src, assert.New(t))
+			src.close()
 		})
 	})
 }
diff --git a/go/store/nbs/aws_table_persister_test.go b/go/store/nbs/aws_table_persister_test.go
index 4151157455..4eec8f821b 100644
--- a/go/store/nbs/aws_table_persister_test.go
+++ b/go/store/nbs/aws_table_persister_test.go
@@ -58,10 +58,12 @@ func TestAWSTablePersisterPersist(t *testing.T) {
 
 				src, err := s3p.Persist(context.Background(), mt, nil, &Stats{})
 				require.NoError(t, err)
+				defer src.close()
 
 				if assert.True(mustUint32(src.count()) > 0) {
 					if r, err := s3svc.readerForTableWithNamespace(ctx, ns, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 						assertChunksInReader(testChunks, r, assert)
+						r.close()
 					}
 				}
 			})
@@ -75,9 +77,11 @@ func TestAWSTablePersisterPersist(t *testing.T) {
 
 				src, err := s3p.Persist(context.Background(), mt, nil, &Stats{})
 				require.NoError(t, err)
+				defer src.close()
 				if assert.True(mustUint32(src.count()) > 0) {
 					if r, err := s3svc.readerForTableWithNamespace(ctx, ns, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 						assertChunksInReader(testChunks, r, assert)
+						r.close()
 					}
 				}
 			})
@@ -99,6 +103,7 @@ func TestAWSTablePersisterPersist(t *testing.T) {
 
 				src, err := s3p.Persist(context.Background(), mt, existingTable, &Stats{})
 				require.NoError(t, err)
+				defer src.close()
 				assert.True(mustUint32(src.count()) == 0)
 
 				_, present := s3svc.data[src.hash().String()]
@@ -268,6 +273,11 @@ func TestAWSTablePersisterDividePlan(t *testing.T) {
 	tooBig := bytesToChunkSource(t, bigUns...)
 
 	sources := chunkSources{justRight, tooBig, tooSmall}
+	defer func() {
+		for _, s := range sources {
+			s.close()
+		}
+	}()
 	plan, err := planRangeCopyConjoin(sources, &Stats{})
 	require.NoError(t, err)
 	copies, manuals, _, err := dividePlan(context.Background(), plan, minPartSize, maxPartSize)
@@ -349,6 +359,7 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 		ti, err := src.index()
 		require.NoError(t, err)
 		smallChunkTotal += calcChunkRangeSize(ti)
+		ti.Close()
 	}
 
 	t.Run("Small", func(t *testing.T) {
@@ -372,10 +383,15 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 			sources := makeSources(s3p, chunks)
 			src, err := s3p.ConjoinAll(context.Background(), sources, &Stats{})
 			require.NoError(t, err)
+			defer src.close()
+			for _, s := range sources {
+				s.close()
+			}
 
 			if assert.True(mustUint32(src.count()) > 0) {
 				if r, err := s3svc.readerForTable(ctx, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 					assertChunksInReader(chunks, r, assert)
+					r.close()
 				}
 			}
 		})
@@ -388,10 +404,15 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 			sources := makeSources(s3p, smallChunks)
 			src, err := s3p.ConjoinAll(context.Background(), sources, &Stats{})
 			require.NoError(t, err)
+			defer src.close()
+			for _, s := range sources {
+				s.close()
+			}
 
 			if assert.True(mustUint32(src.count()) > 0) {
 				if r, err := s3svc.readerForTable(ctx, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 					assertChunksInReader(smallChunks, r, assert)
+					r.close()
 				}
 			}
 		})
@@ -424,11 +445,16 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 		}
 		src, err := s3p.ConjoinAll(context.Background(), sources, &Stats{})
 		require.NoError(t, err)
+		defer src.close()
+		for _, s := range sources {
+			s.close()
+		}
 
 		if assert.True(mustUint32(src.count()) > 0) {
 			if r, err := s3svc.readerForTable(ctx, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 				assertChunksInReader(bigUns1, r, assert)
 				assertChunksInReader(bigUns2, r, assert)
+				r.close()
 			}
 		}
 	})
@@ -460,11 +486,16 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 
 		src, err := s3p.ConjoinAll(context.Background(), sources, &Stats{})
 		require.NoError(t, err)
+		defer src.close()
+		for _, s := range sources {
+			s.close()
+		}
 
 		if assert.True(mustUint32(src.count()) > 0) {
 			if r, err := s3svc.readerForTable(ctx, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 				assertChunksInReader(bigUns1, r, assert)
 				assertChunksInReader(medChunks, r, assert)
+				r.close()
 			}
 		}
 	})
@@ -510,12 +541,17 @@ func TestAWSTablePersisterConjoinAll(t *testing.T) {
 
 		src, err := s3p.ConjoinAll(context.Background(), sources, &Stats{})
 		require.NoError(t, err)
+		defer src.close()
+		for _, s := range sources {
+			s.close()
+		}
 
 		if assert.True(mustUint32(src.count()) > 0) {
 			if r, err := s3svc.readerForTable(ctx, src.hash()); assert.NotNil(r) && assert.NoError(err) {
 				assertChunksInReader(smallChunks, r, assert)
 				assertChunksInReader(bigUns1, r, assert)
 				assertChunksInReader(medChunks, r, assert)
+				r.close()
 			}
 		}
 	})
diff --git a/go/store/nbs/block_store_test.go b/go/store/nbs/block_store_test.go
index 693ad2cf38..703fe18def 100644
--- a/go/store/nbs/block_store_test.go
+++ b/go/store/nbs/block_store_test.go
@@ -320,6 +320,7 @@ func (suite *BlockStoreSuite) TestChunkStoreFlushOptimisticLockFail() {
 
 	interloper, err := suite.factory(context.Background(), suite.dir)
 	suite.NoError(err)
+	defer interloper.Close()
 	err = interloper.Put(context.Background(), c1, noopGetAddrs)
 	suite.NoError(err)
 	h, err := interloper.Root(context.Background())
@@ -369,6 +370,7 @@ func (suite *BlockStoreSuite) TestChunkStoreRebaseOnNoOpFlush() {
 
 	interloper, err := suite.factory(context.Background(), suite.dir)
 	suite.NoError(err)
+	defer interloper.Close()
 	err = interloper.Put(context.Background(), c1, noopGetAddrs)
 	suite.NoError(err)
 	root, err := interloper.Root(context.Background())
@@ -408,6 +410,7 @@ func (suite *BlockStoreSuite) TestChunkStorePutWithRebase() {
 
 	interloper, err := suite.factory(context.Background(), suite.dir)
 	suite.NoError(err)
+	defer interloper.Close()
 	err = interloper.Put(context.Background(), c1, noopGetAddrs)
 	suite.NoError(err)
 	h, err := interloper.Root(context.Background())
diff --git a/go/store/nbs/cmp_chunk_table_writer_test.go b/go/store/nbs/cmp_chunk_table_writer_test.go
index 0cc9199452..cd1c7450d4 100644
--- a/go/store/nbs/cmp_chunk_table_writer_test.go
+++ b/go/store/nbs/cmp_chunk_table_writer_test.go
@@ -40,6 +40,7 @@ func TestCmpChunkTableWriter(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	hashes := make(hash.HashSet)
 	for _, chnk := range testMDChunks {
@@ -89,6 +90,7 @@ func TestCmpChunkTableWriter(t *testing.T) {
 	require.NoError(t, err)
 	outputTR, err := newTableReader(outputTI, tableReaderAtFromBytes(buff), fileBlockSize)
 	require.NoError(t, err)
+	defer outputTR.close()
 
 	compareContentsOfTables(t, ctx, hashes, tr, outputTR)
 }
diff --git a/go/store/nbs/conjoiner.go b/go/store/nbs/conjoiner.go
index 722d1ab674..68cf6bc489 100644
--- a/go/store/nbs/conjoiner.go
+++ b/go/store/nbs/conjoiner.go
@@ -197,6 +197,13 @@ func conjoinTables(ctx context.Context, conjoinees []tableSpec, p tablePersister
 			return
 		})
 	}
+	defer func() {
+		for _, cs := range toConjoin {
+			if cs != nil {
+				cs.close()
+			}
+		}
+	}()
 	if err = eg.Wait(); err != nil {
 		return tableSpec{}, err
 	}
@@ -207,6 +214,7 @@ func conjoinTables(ctx context.Context, conjoinees []tableSpec, p tablePersister
 	if err != nil {
 		return tableSpec{}, err
 	}
+	defer conjoinedSrc.close()
 
 	stats.ConjoinLatency.SampleTimeSince(t1)
 	stats.TablesPerConjoin.SampleLen(len(toConjoin))
diff --git a/go/store/nbs/conjoiner_test.go b/go/store/nbs/conjoiner_test.go
index 59e3d5cbf7..19cd246e21 100644
--- a/go/store/nbs/conjoiner_test.go
+++ b/go/store/nbs/conjoiner_test.go
@@ -68,6 +68,7 @@ func makeTestSrcs(t *testing.T, tableSizes []uint32, p tablePersister) (srcs chu
 		c, err := cs.clone()
 		require.NoError(t, err)
 		srcs = append(srcs, c)
+		cs.close()
 	}
 	return
 }
@@ -143,6 +144,14 @@ func testConjoin(t *testing.T, factory func(t *testing.T) tablePersister) {
 		}
 
 		expectSrcs, actualSrcs := open(expect), open(actual)
+		defer func() {
+			for _, s := range expectSrcs {
+				s.close()
+			}
+			for _, s := range actualSrcs {
+				s.close()
+			}
+		}()
 
 		ctx := context.Background()
 		for _, src := range expectSrcs {
@@ -173,6 +182,7 @@ func testConjoin(t *testing.T, factory func(t *testing.T) tablePersister) {
 		mt.addChunk(computeAddr(data), data)
 		src, err := p.Persist(context.Background(), mt, nil, &Stats{})
 		require.NoError(t, err)
+		defer src.close()
 		return tableSpec{src.hash(), mustUint32(src.count())}
 	}
 
diff --git a/go/store/nbs/file_table_persister_test.go b/go/store/nbs/file_table_persister_test.go
index a9a489387d..151a632d01 100644
--- a/go/store/nbs/file_table_persister_test.go
+++ b/go/store/nbs/file_table_persister_test.go
@@ -54,8 +54,9 @@ func TestFSTableCacheOnOpen(t *testing.T) {
 			names = append(names, name)
 		}
 		for _, name := range names {
-			_, err := fts.Open(context.Background(), name, 1, nil)
+			tr, err := fts.Open(context.Background(), name, 1, nil)
 			require.NoError(t, err)
+			defer tr.close()
 		}
 	}()
 
@@ -63,6 +64,7 @@ func TestFSTableCacheOnOpen(t *testing.T) {
 	for i, name := range names {
 		src, err := fts.Open(context.Background(), name, 1, nil)
 		require.NoError(t, err)
+		defer src.close()
 		h := computeAddr([]byte{byte(i)})
 		assert.True(src.has(h))
 	}
@@ -70,8 +72,9 @@ func TestFSTableCacheOnOpen(t *testing.T) {
 	// Kick a table out of the cache
 	name, err := writeTableData(dir, []byte{0xff})
 	require.NoError(t, err)
-	_, err = fts.Open(context.Background(), name, 1, nil)
+	tr, err := fts.Open(context.Background(), name, 1, nil)
 	require.NoError(t, err)
+	defer tr.close()
 
 	present := fc.reportEntries()
 	// Since 0 refcount entries are evicted randomly, the only thing we can validate is that fc remains at its target size
@@ -125,6 +128,7 @@ func TestFSTablePersisterPersist(t *testing.T) {
 
 	src, err := persistTableData(fts, testChunks...)
 	require.NoError(t, err)
+	defer src.close()
 	if assert.True(mustUint32(src.count()) > 0) {
 		buff, err := os.ReadFile(filepath.Join(dir, src.hash().String()))
 		require.NoError(t, err)
@@ -132,6 +136,7 @@ func TestFSTablePersisterPersist(t *testing.T) {
 		require.NoError(t, err)
 		tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), fileBlockSize)
 		require.NoError(t, err)
+		defer tr.close()
 		assertChunksInReader(testChunks, tr, assert)
 	}
 }
@@ -182,17 +187,20 @@ func TestFSTablePersisterCacheOnPersist(t *testing.T) {
 	func() {
 		src, err := persistTableData(fts, testChunks...)
 		require.NoError(t, err)
+		defer src.close()
 		name = src.hash()
 	}()
 
 	// Table should still be cached
 	src, err := fts.Open(context.Background(), name, uint32(len(testChunks)), nil)
 	require.NoError(t, err)
+	defer src.close()
 	assertChunksInReader(testChunks, src, assert)
 
 	// Evict |name| from cache
-	_, err = persistTableData(fts, []byte{0xff})
+	tr, err := persistTableData(fts, []byte{0xff})
 	require.NoError(t, err)
+	defer tr.close()
 
 	present := fc.reportEntries()
 	// Since 0 refcount entries are evicted randomly, the only thing we can validate is that fc remains at its target size
@@ -223,9 +231,15 @@ func TestFSTablePersisterConjoinAll(t *testing.T) {
 		sources[i], err = fts.Open(ctx, name, 2, nil)
 		require.NoError(t, err)
 	}
+	defer func() {
+		for _, s := range sources {
+			s.close()
+		}
+	}()
 
 	src, err := fts.ConjoinAll(ctx, sources, &Stats{})
 	require.NoError(t, err)
+	defer src.close()
 
 	if assert.True(mustUint32(src.count()) > 0) {
 		buff, err := os.ReadFile(filepath.Join(dir, src.hash().String()))
@@ -234,6 +248,7 @@ func TestFSTablePersisterConjoinAll(t *testing.T) {
 		require.NoError(t, err)
 		tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), fileBlockSize)
 		require.NoError(t, err)
+		defer tr.close()
 		assertChunksInReader(testChunks, tr, assert)
 	}
 
@@ -263,9 +278,15 @@ func TestFSTablePersisterConjoinAllDups(t *testing.T) {
 		sources[i], err = fts.Persist(ctx, mt, nil, &Stats{})
 		require.NoError(t, err)
 	}
+	defer func() {
+		for _, s := range sources {
+			s.close()
+		}
+	}()
 
 	src, err := fts.ConjoinAll(ctx, sources, &Stats{})
 	require.NoError(t, err)
+	defer src.close()
 
 	if assert.True(mustUint32(src.count()) > 0) {
 		buff, err := os.ReadFile(filepath.Join(dir, src.hash().String()))
@@ -274,6 +295,7 @@ func TestFSTablePersisterConjoinAllDups(t *testing.T) {
 		require.NoError(t, err)
 		tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), fileBlockSize)
 		require.NoError(t, err)
+		defer tr.close()
 		assertChunksInReader(testChunks, tr, assert)
 		assert.EqualValues(reps*len(testChunks), mustUint32(tr.count()))
 	}
diff --git a/go/store/nbs/file_table_reader.go b/go/store/nbs/file_table_reader.go
index 2aa858e5d2..e6e14f0c4e 100644
--- a/go/store/nbs/file_table_reader.go
+++ b/go/store/nbs/file_table_reader.go
@@ -86,29 +86,34 @@ func newFileTableReader(ctx context.Context, dir string, h addr, chunkCount uint
 		indexOffset := sz - idxSz
 		r := io.NewSectionReader(f, indexOffset, idxSz)
 
+		if int64(int(idxSz)) != idxSz {
+			err = fmt.Errorf("table file %s/%s is too large to read on this platform. index size %d > max int.", dir, h.String(), idxSz)
+			return
+		}
+
 		var b []byte
-		b, err = q.AcquireQuotaBytes(ctx, uint64(idxSz))
+		b, err = q.AcquireQuotaBytes(ctx, int(idxSz))
 		if err != nil {
 			return
 		}
 
 		_, err = io.ReadFull(r, b)
 		if err != nil {
-			q.ReleaseQuotaBytes(b)
+			q.ReleaseQuotaBytes(len(b))
 			return
 		}
 
 		defer func() {
 			unrefErr := fc.UnrefFile(path)
-
-			if unrefErr != nil {
+			if unrefErr != nil && err == nil {
+				q.ReleaseQuotaBytes(len(b))
 				err = unrefErr
 			}
 		}()
 
 		ti, err = parseTableIndex(ctx, b, q)
 		if err != nil {
-			q.ReleaseQuotaBytes(b)
+			q.ReleaseQuotaBytes(len(b))
 			return
 		}
 
diff --git a/go/store/nbs/file_table_reader_test.go b/go/store/nbs/file_table_reader_test.go
index fbee9ae774..8df22ebbce 100644
--- a/go/store/nbs/file_table_reader_test.go
+++ b/go/store/nbs/file_table_reader_test.go
@@ -56,5 +56,6 @@ func TestMmapTableReader(t *testing.T) {
 
 	trc, err := newFileTableReader(ctx, dir, h, uint32(len(chunks)), &UnlimitedQuotaProvider{}, fc)
 	require.NoError(t, err)
+	defer trc.close()
 	assertChunksInReader(chunks, trc, assert)
 }
diff --git a/go/store/nbs/mem_table_test.go b/go/store/nbs/mem_table_test.go
index acdfff9e8e..77f4e0575a 100644
--- a/go/store/nbs/mem_table_test.go
+++ b/go/store/nbs/mem_table_test.go
@@ -156,6 +156,7 @@ func TestMemTableWrite(t *testing.T) {
 	require.NoError(t, err)
 	tr1, err := newTableReader(ti1, tableReaderAtFromBytes(td1), fileBlockSize)
 	require.NoError(t, err)
+	defer tr1.close()
 	assert.True(tr1.has(computeAddr(chunks[1])))
 
 	td2, _, err := buildTable(chunks[2:])
@@ -164,6 +165,7 @@ func TestMemTableWrite(t *testing.T) {
 	require.NoError(t, err)
 	tr2, err := newTableReader(ti2, tableReaderAtFromBytes(td2), fileBlockSize)
 	require.NoError(t, err)
+	defer tr2.close()
 	assert.True(tr2.has(computeAddr(chunks[2])))
 
 	_, data, count, err := mt.write(chunkReaderGroup{tr1, tr2}, &Stats{})
@@ -174,6 +176,7 @@ func TestMemTableWrite(t *testing.T) {
 	require.NoError(t, err)
 	outReader, err := newTableReader(ti, tableReaderAtFromBytes(data), fileBlockSize)
 	require.NoError(t, err)
+	defer outReader.close()
 	assert.True(outReader.has(computeAddr(chunks[0])))
 	assert.False(outReader.has(computeAddr(chunks[1])))
 	assert.False(outReader.has(computeAddr(chunks[2])))
diff --git a/go/store/nbs/quota.go b/go/store/nbs/quota.go
index c720df3d92..1116679a81 100644
--- a/go/store/nbs/quota.go
+++ b/go/store/nbs/quota.go
@@ -20,8 +20,8 @@ import (
 )
 
 type MemoryQuotaProvider interface {
-	AcquireQuotaBytes(ctx context.Context, sz uint64) ([]byte, error)
-	ReleaseQuotaBytes(buf []byte) error
+	AcquireQuotaBytes(ctx context.Context, sz int) ([]byte, error)
+	ReleaseQuotaBytes(sz int)
 	Usage() uint64
 }
 
@@ -34,23 +34,24 @@ func NewUnlimitedMemQuotaProvider() *UnlimitedQuotaProvider {
 	return &UnlimitedQuotaProvider{}
 }
 
-func (q *UnlimitedQuotaProvider) AcquireQuotaBytes(ctx context.Context, sz uint64) ([]byte, error) {
+func (q *UnlimitedQuotaProvider) AcquireQuotaBytes(ctx context.Context, sz int) ([]byte, error) {
 	buf := make([]byte, sz)
 	q.mu.Lock()
 	defer q.mu.Unlock()
-	q.used += sz
+	q.used += uint64(sz)
 	return buf, nil
 }
 
-func (q *UnlimitedQuotaProvider) ReleaseQuotaBytes(buf []byte) error {
+func (q *UnlimitedQuotaProvider) ReleaseQuotaBytes(sz int) {
+	if sz < 0 {
+		panic("tried to release negative bytes")
+	}
 	q.mu.Lock()
 	defer q.mu.Unlock()
-	memory := uint64(len(buf))
-	if memory > q.used {
+	if uint64(sz) > q.used {
 		panic("tried to release too much quota")
 	}
-	q.used -= memory
-	return nil
+	q.used -= uint64(sz)
 }
 
 func (q *UnlimitedQuotaProvider) Usage() uint64 {
diff --git a/go/store/nbs/s3_fake_test.go b/go/store/nbs/s3_fake_test.go
index dafac14b7a..fe48bcea9c 100644
--- a/go/store/nbs/s3_fake_test.go
+++ b/go/store/nbs/s3_fake_test.go
@@ -78,12 +78,12 @@ func (m *fakeS3) readerForTable(ctx context.Context, name addr) (chunkReader, er
 	defer m.mu.Unlock()
 	if buff, present := m.data[name.String()]; present {
 		ti, err := parseTableIndexByCopy(ctx, buff, &UnlimitedQuotaProvider{})
-
 		if err != nil {
 			return nil, err
 		}
 		tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), s3BlockSize)
 		if err != nil {
+			ti.Close()
 			return nil, err
 		}
 		return tr, nil
diff --git a/go/store/nbs/store.go b/go/store/nbs/store.go
index 798cd6672b..ea514d9951 100644
--- a/go/store/nbs/store.go
+++ b/go/store/nbs/store.go
@@ -1625,9 +1625,9 @@ func (nbs *NomsBlockStore) swapTables(ctx context.Context, specs []tableSpec) (e
 	if err != nil {
 		return err
 	}
+	oldTables := nbs.tables
 	nbs.tables, nbs.upstream = ts, upstream
-
-	return nil
+	return oldTables.close()
 }
 
 // SetRootChunk changes the root chunk hash from the previous value to the new root.
diff --git a/go/store/nbs/store_test.go b/go/store/nbs/store_test.go
index 6c33c64be4..19aaa797b3 100644
--- a/go/store/nbs/store_test.go
+++ b/go/store/nbs/store_test.go
@@ -124,6 +124,7 @@ func TestNBSAsTableFileStore(t *testing.T) {
 
 func TestConcurrentPuts(t *testing.T) {
 	st, _, _ := makeTestLocalStore(t, 100)
+	defer st.Close()
 
 	errgrp, ctx := errgroup.WithContext(context.Background())
 
@@ -190,6 +191,7 @@ func TestNBSPruneTableFiles(t *testing.T) {
 	maxTableFiles := 16
 	st, nomsDir, _ := makeTestLocalStore(t, maxTableFiles)
 	fileToData := populateLocalStore(t, st, numTableFiles)
+	defer st.Close()
 
 	// add a chunk and flush to trigger a conjoin
 	c := chunks.NewChunk([]byte("it's a boy!"))
@@ -272,6 +274,7 @@ func makeChunkSet(N, size int) (s map[hash.Hash]chunks.Chunk) {
 func TestNBSCopyGC(t *testing.T) {
 	ctx := context.Background()
 	st, _, _ := makeTestLocalStore(t, 8)
+	defer st.Close()
 
 	keepers := makeChunkSet(64, 64)
 	tossers := makeChunkSet(64, 64)
diff --git a/go/store/nbs/table_index.go b/go/store/nbs/table_index.go
index 0b29c1b3ae..e1dd83a8fc 100644
--- a/go/store/nbs/table_index.go
+++ b/go/store/nbs/table_index.go
@@ -19,7 +19,11 @@ import (
 	"context"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
+	"os"
+	"runtime"
+	"runtime/debug"
 	"sync/atomic"
 
 	"github.com/dolthub/dolt/go/libraries/utils/iohelp"
@@ -31,6 +35,25 @@ var (
 	ErrWrongCopySize   = errors.New("could not copy enough bytes")
 )
 
+// By setting the environment variable DOLT_ASSERT_TABLE_FILES_CLOSED to any
+// non-empty string, dolt will run some sanity checks on table file lifecycle
+// management. In particular, dolt will install a GC finalizer on the table
+// file index buffer to assert that it has been properly closed at the time
+// that it gets garbage collected.
+//
+// This is mostly intended for developers. It isa recommended mode in tests and
+// can make sense in other contexts as well. At the time of this writing---
+// (2023/02, aaron@)---lifecycle management in tests in particular is not good
+// enough to globally enable this.
+
+var TableIndexAssertClosedWithGCFinalizer bool
+
+func init() {
+	if os.Getenv("DOLT_ASSERT_TABLE_FILES_CLOSED") != "" {
+		TableIndexAssertClosedWithGCFinalizer = true
+	}
+}
+
 type tableIndex interface {
 	// entrySuffixMatches returns true if the entry at index |idx| matches
 	// the suffix of the address |h|. Used by |lookup| after finding
@@ -109,13 +132,13 @@ func parseTableIndex(ctx context.Context, buff []byte, q MemoryQuotaProvider) (o
 
 	chunks2 := chunkCount / 2
 	chunks1 := chunkCount - chunks2
-	offsetsBuff1, err := q.AcquireQuotaBytes(ctx, uint64(chunks1*offsetSize))
+	offsetsBuff1, err := q.AcquireQuotaBytes(ctx, int(chunks1*offsetSize))
 	if err != nil {
 		return onHeapTableIndex{}, err
 	}
 	idx, err := newOnHeapTableIndex(buff, offsetsBuff1, chunkCount, totalUncompressedData, q)
 	if err != nil {
-		q.ReleaseQuotaBytes(offsetsBuff1)
+		q.ReleaseQuotaBytes(len(offsetsBuff1))
 	}
 	return idx, err
 }
@@ -150,28 +173,32 @@ func readTableIndexByCopy(ctx context.Context, rd io.ReadSeeker, q MemoryQuotaPr
 		return onHeapTableIndex{}, err
 	}
 
-	buff, err := q.AcquireQuotaBytes(ctx, uint64(idxSz))
+	if int64(int(idxSz)) != idxSz {
+		return onHeapTableIndex{}, fmt.Errorf("table file index is too large to read on this platform. index size %d > max int.", idxSz)
+	}
+
+	buff, err := q.AcquireQuotaBytes(ctx, int(idxSz))
 	if err != nil {
 		return onHeapTableIndex{}, err
 	}
 
 	_, err = io.ReadFull(rd, buff)
 	if err != nil {
-		q.ReleaseQuotaBytes(buff)
+		q.ReleaseQuotaBytes(len(buff))
 		return onHeapTableIndex{}, err
 	}
 
 	chunks1 := chunkCount - (chunkCount / 2)
-	offsets1Buff, err := q.AcquireQuotaBytes(ctx, uint64(chunks1*offsetSize))
+	offsets1Buff, err := q.AcquireQuotaBytes(ctx, int(chunks1*offsetSize))
 	if err != nil {
-		q.ReleaseQuotaBytes(buff)
+		q.ReleaseQuotaBytes(len(buff))
 		return onHeapTableIndex{}, err
 	}
 
 	idx, err := newOnHeapTableIndex(buff, offsets1Buff, chunkCount, totalUncompressedData, q)
 	if err != nil {
-		q.ReleaseQuotaBytes(buff)
-		q.ReleaseQuotaBytes(offsets1Buff)
+		q.ReleaseQuotaBytes(len(buff))
+		q.ReleaseQuotaBytes(len(offsets1Buff))
 	}
 	return idx, err
 }
@@ -251,6 +278,12 @@ func newOnHeapTableIndex(indexBuff []byte, offsetsBuff1 []byte, count uint32, to
 
 	refCnt := new(int32)
 	*refCnt = 1
+	if TableIndexAssertClosedWithGCFinalizer {
+		stack := string(debug.Stack())
+		runtime.SetFinalizer(refCnt, func(i *int32) {
+			panic(fmt.Sprintf("OnHeapTableIndex not closed:\n%s", stack))
+		})
+	}
 
 	return onHeapTableIndex{
 		refCnt:         refCnt,
@@ -506,19 +539,11 @@ func (ti onHeapTableIndex) Close() error {
 		return nil
 	}
 
-	if err := ti.q.ReleaseQuotaBytes(ti.prefixTuples); err != nil {
-		return err
+	if TableIndexAssertClosedWithGCFinalizer {
+		runtime.SetFinalizer(ti.refCnt, nil)
 	}
-	if err := ti.q.ReleaseQuotaBytes(ti.offsets1); err != nil {
-		return err
-	}
-	if err := ti.q.ReleaseQuotaBytes(ti.offsets2); err != nil {
-		return err
-	}
-	if err := ti.q.ReleaseQuotaBytes(ti.suffixes); err != nil {
-		return err
-	}
-	return ti.q.ReleaseQuotaBytes(ti.footer)
+	ti.q.ReleaseQuotaBytes(len(ti.prefixTuples) + len(ti.offsets1) + len(ti.offsets2) + len(ti.suffixes) + len(ti.footer))
+	return nil
 }
 
 func (ti onHeapTableIndex) clone() (tableIndex, error) {
diff --git a/go/store/nbs/table_index_test.go b/go/store/nbs/table_index_test.go
index 95b9331615..375fa2a110 100644
--- a/go/store/nbs/table_index_test.go
+++ b/go/store/nbs/table_index_test.go
@@ -129,6 +129,7 @@ func TestResolveOneHash(t *testing.T) {
 	td, _, err := buildTable(chunks)
 	tIdx, err := parseTableIndexByCopy(ctx, td, &UnlimitedQuotaProvider{})
 	require.NoError(t, err)
+	defer tIdx.Close()
 
 	// get hashes out
 	hashes := make([]string, len(chunks))
@@ -161,6 +162,7 @@ func TestResolveFewHash(t *testing.T) {
 	td, _, err := buildTable(chunks)
 	tIdx, err := parseTableIndexByCopy(ctx, td, &UnlimitedQuotaProvider{})
 	require.NoError(t, err)
+	defer tIdx.Close()
 
 	// get hashes out
 	hashes := make([]string, len(chunks))
@@ -194,6 +196,7 @@ func TestAmbiguousShortHash(t *testing.T) {
 	td, _, err := buildFakeChunkTable(chunks)
 	idx, err := parseTableIndexByCopy(ctx, td, &UnlimitedQuotaProvider{})
 	require.NoError(t, err)
+	defer idx.Close()
 
 	tests := []struct {
 		pre string
diff --git a/go/store/nbs/table_persister_test.go b/go/store/nbs/table_persister_test.go
index 0d64065efb..1b25cdaea6 100644
--- a/go/store/nbs/table_persister_test.go
+++ b/go/store/nbs/table_persister_test.go
@@ -38,6 +38,8 @@ func TestPlanCompaction(t *testing.T) {
 		{[]byte("solo")},
 	}
 
+	q := &UnlimitedQuotaProvider{}
+
 	var sources chunkSources
 	var dataLens []uint64
 	var totalUnc uint64
@@ -47,7 +49,7 @@ func TestPlanCompaction(t *testing.T) {
 		}
 		data, name, err := buildTable(content)
 		require.NoError(t, err)
-		ti, err := parseTableIndexByCopy(ctx, data, &UnlimitedQuotaProvider{})
+		ti, err := parseTableIndexByCopy(ctx, data, q)
 		require.NoError(t, err)
 		tr, err := newTableReader(ti, tableReaderAtFromBytes(data), fileBlockSize)
 		require.NoError(t, err)
@@ -55,6 +57,11 @@ func TestPlanCompaction(t *testing.T) {
 		dataLens = append(dataLens, uint64(len(data))-indexSize(mustUint32(src.count()))-footerSize)
 		sources = append(sources, src)
 	}
+	defer func() {
+		for _, s := range sources {
+			s.close()
+		}
+	}()
 
 	plan, err := planRangeCopyConjoin(sources, &Stats{})
 	require.NoError(t, err)
@@ -65,7 +72,7 @@ func TestPlanCompaction(t *testing.T) {
 		totalChunks += mustUint32(src.count())
 	}
 
-	idx, err := parseTableIndex(ctx, plan.mergedIndex, &UnlimitedQuotaProvider{})
+	idx, err := parseTableIndexByCopy(ctx, plan.mergedIndex, q)
 	require.NoError(t, err)
 
 	assert.Equal(totalChunks, idx.chunkCount())
@@ -73,6 +80,7 @@ func TestPlanCompaction(t *testing.T) {
 
 	tr, err := newTableReader(idx, tableReaderAtFromBytes(nil), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 	for _, content := range tableContents {
 		assertChunksInReader(content, tr, assert)
 	}
diff --git a/go/store/nbs/table_set_test.go b/go/store/nbs/table_set_test.go
index 6edde5d412..b17f87f26a 100644
--- a/go/store/nbs/table_set_test.go
+++ b/go/store/nbs/table_set_test.go
@@ -49,6 +49,9 @@ func TestTableSetPrepend(t *testing.T) {
 	assert := assert.New(t)
 	ts := newFakeTableSet(&UnlimitedQuotaProvider{})
 	specs, err := ts.toSpecs()
+	defer func() {
+		ts.close()
+	}()
 	require.NoError(t, err)
 	assert.Empty(specs)
 	mt := newMemTable(testMemTableSize)
@@ -75,6 +78,9 @@ func TestTableSetPrepend(t *testing.T) {
 func TestTableSetToSpecsExcludesEmptyTable(t *testing.T) {
 	assert := assert.New(t)
 	ts := newFakeTableSet(&UnlimitedQuotaProvider{})
+	defer func() {
+		ts.close()
+	}()
 	specs, err := ts.toSpecs()
 	require.NoError(t, err)
 	assert.Empty(specs)
@@ -101,6 +107,9 @@ func TestTableSetToSpecsExcludesEmptyTable(t *testing.T) {
 func TestTableSetFlattenExcludesEmptyTable(t *testing.T) {
 	assert := assert.New(t)
 	ts := newFakeTableSet(&UnlimitedQuotaProvider{})
+	defer func() {
+		ts.close()
+	}()
 	specs, err := ts.toSpecs()
 	require.NoError(t, err)
 	assert.Empty(specs)
@@ -183,6 +192,9 @@ func TestTableSetRebase(t *testing.T) {
 func TestTableSetPhysicalLen(t *testing.T) {
 	assert := assert.New(t)
 	ts := newFakeTableSet(&UnlimitedQuotaProvider{})
+	defer func() {
+		ts.close()
+	}()
 	specs, err := ts.toSpecs()
 	require.NoError(t, err)
 	assert.Empty(specs)
diff --git a/go/store/nbs/table_test.go b/go/store/nbs/table_test.go
index 9293c7bb40..88d732ade7 100644
--- a/go/store/nbs/table_test.go
+++ b/go/store/nbs/table_test.go
@@ -83,6 +83,7 @@ func TestSimple(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	assertChunksInReader(chunks, tr, assert)
 
@@ -131,6 +132,7 @@ func TestHasMany(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	addrs := addrSlice{computeAddr(chunks[0]), computeAddr(chunks[1]), computeAddr(chunks[2])}
 	hasAddrs := []hasRecord{
@@ -183,6 +185,7 @@ func TestHasManySequentialPrefix(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(buff), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	hasAddrs := make([]hasRecord, 2)
 	// Leave out the first address
@@ -237,6 +240,7 @@ func BenchmarkHasMany(b *testing.B) {
 	require.NoError(b, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(b, err)
+	defer tr.close()
 
 	b.ResetTimer()
 	b.Run("dense has many", func(b *testing.B) {
@@ -277,6 +281,7 @@ func TestGetMany(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	addrs := addrSlice{computeAddr(data[0]), computeAddr(data[1]), computeAddr(data[2])}
 	getBatch := []getRecord{
@@ -312,6 +317,7 @@ func TestCalcReads(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), 0)
 	require.NoError(t, err)
+	defer tr.close()
 	addrs := addrSlice{computeAddr(chunks[0]), computeAddr(chunks[1]), computeAddr(chunks[2])}
 	getBatch := []getRecord{
 		{&addrs[0], binary.BigEndian.Uint64(addrs[0][:addrPrefixSize]), false},
@@ -350,6 +356,7 @@ func TestExtract(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	addrs := addrSlice{computeAddr(chunks[0]), computeAddr(chunks[1]), computeAddr(chunks[2])}
 
@@ -390,6 +397,7 @@ func Test65k(t *testing.T) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	for i := 0; i < count; i++ {
 		data := dataFn(i)
@@ -444,6 +452,7 @@ func doTestNGetMany(t *testing.T, count int) {
 	require.NoError(t, err)
 	tr, err := newTableReader(ti, tableReaderAtFromBytes(tableData), fileBlockSize)
 	require.NoError(t, err)
+	defer tr.close()
 
 	getBatch := make([]getRecord, len(data))
 	for i := 0; i < count; i++ {
diff --git a/go/store/spec/spec_test.go b/go/store/spec/spec_test.go
index 9dcb7915bc..6df9283f30 100644
--- a/go/store/spec/spec_test.go
+++ b/go/store/spec/spec_test.go
@@ -239,14 +239,18 @@ func TestHref(t *testing.T) {
 
 	sp, _ := ForDatabase("aws://table/foo/bar/baz")
 	assert.Equal("aws://table/foo/bar/baz", sp.Href())
+	sp.Close()
 	sp, _ = ForDataset("aws://[table:bucket]/foo/bar/baz::myds")
 	assert.Equal("aws://[table:bucket]/foo/bar/baz", sp.Href())
+	sp.Close()
 	sp, _ = ForPath("aws://[table:bucket]/foo/bar/baz::myds.my.path")
 	assert.Equal("aws://[table:bucket]/foo/bar/baz", sp.Href())
+	sp.Close()
 
 	sp, err := ForPath("mem::myds.my.path")
 	assert.NoError(err)
 	assert.Equal("", sp.Href())
+	sp.Close()
 }
 
 func TestForDatabase(t *testing.T) {
@@ -323,8 +327,9 @@ func TestForDataset(t *testing.T) {
 
 	validDatasetNames := []string{"a", "Z", "0", "/", "-", "_"}
 	for _, s := range validDatasetNames {
-		_, err := ForDataset("mem::" + s)
+		spec, err := ForDataset("mem::" + s)
 		assert.NoError(t, err)
+		spec.Close()
 	}
 
 	tmpDir, err := os.MkdirTemp("", "spec_test")
@@ -416,6 +421,8 @@ func TestMultipleSpecsSameNBS(t *testing.T) {
 
 	assert.NoError(err1)
 	assert.NoError(err2)
+	defer spec1.Close()
+	defer spec2.Close()
 
 	s := types.String("hello")
 	db := spec1.GetDatabase(context.Background())
@@ -479,6 +486,7 @@ func TestExternalProtocol(t *testing.T) {
 
 	sp, err := ForDataset("test:foo::bar")
 	assert.NoError(err)
+	defer sp.Close()
 	assert.Equal("test", sp.Protocol)
 	assert.Equal("foo", sp.DatabaseName)
 

From 4b3c909bb142e3850f8109ad6b4276a03de0807d Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Thu, 16 Feb 2023 16:40:54 -0800
Subject: [PATCH 32/40] go/store/nbs: refactor chunk journal record lookups
 into journalWriter

---
 go/store/nbs/journal_chunk_source.go | 36 +++++++---------------------
 go/store/nbs/journal_writer.go       | 36 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/go/store/nbs/journal_chunk_source.go b/go/store/nbs/journal_chunk_source.go
index bc7b88090d..26c7748829 100644
--- a/go/store/nbs/journal_chunk_source.go
+++ b/go/store/nbs/journal_chunk_source.go
@@ -88,7 +88,7 @@ func (m lookupMap) count() int {
 // more commits are made to the chunkJournal.
 type journalChunkSource struct {
 	address        addr
-	journal        snapshotReader
+	journal        *journalWriter
 	lookups        lookupMap
 	uncompressedSz uint64
 }
@@ -96,14 +96,13 @@ type journalChunkSource struct {
 var _ chunkSource = journalChunkSource{}
 
 func (s journalChunkSource) has(h addr) (bool, error) {
-	_, ok := s.lookups.get(h)
-	return ok, nil
+	return s.journal.has(h, s.lookups), nil
 }
 
 func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error) {
 	for i := range addrs {
-		a := addrs[i].a
-		if _, ok := s.lookups.get(*a); ok {
+		ok := s.journal.has(*addrs[i].a, s.lookups)
+		if ok {
 			addrs[i].has = true
 		} else {
 			missing = true
@@ -113,28 +112,11 @@ func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error)
 }
 
 func (s journalChunkSource) getCompressed(_ context.Context, h addr, _ *Stats) (CompressedChunk, error) {
-	l, ok := s.lookups.get(h)
-	if !ok {
-		return CompressedChunk{}, nil
-	}
-
-	buf := make([]byte, l.recordLen)
-	if _, err := s.journal.ReadAt(buf, l.journalOff); err != nil {
-		return CompressedChunk{}, nil
-	}
-
-	rec, err := readJournalRecord(buf)
-	if err != nil {
-		return CompressedChunk{}, err
-	} else if h != rec.address {
-		return CompressedChunk{}, fmt.Errorf("chunk record hash does not match lookup hash (%s != %s)",
-			h.String(), rec.address.String())
-	}
-	return NewCompressedChunk(hash.Hash(h), rec.payload)
+	return s.journal.getCompressed(h, s.lookups)
 }
 
-func (s journalChunkSource) get(ctx context.Context, h addr, stats *Stats) ([]byte, error) {
-	cc, err := s.getCompressed(ctx, h, stats)
+func (s journalChunkSource) get(_ context.Context, h addr, _ *Stats) ([]byte, error) {
+	cc, err := s.journal.getCompressed(h, s.lookups)
 	if err != nil {
 		return nil, err
 	} else if cc.IsEmpty() {
@@ -204,12 +186,12 @@ func (s journalChunkSource) getRecordRanges(requests []getRecord) (map[hash.Hash
 		if req.found {
 			continue
 		}
-		l, ok := s.lookups.get(*req.a)
+		rng, ok := s.journal.getRange(*req.a, s.lookups)
 		if !ok {
 			continue
 		}
 		req.found = true // update |requests|
-		ranges[hash.Hash(*req.a)] = rangeFromLookup(l)
+		ranges[hash.Hash(*req.a)] = rng
 	}
 	return ranges, nil
 }
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index 7a78eb7439..c176d6da32 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -316,3 +316,39 @@ func (wr *journalWriter) flush() (err error) {
 	wr.buf = wr.buf[:0]
 	return
 }
+
+func (wr *journalWriter) has(h addr, lookups lookupMap) (ok bool) {
+	_, ok = lookups.get(h)
+	return
+}
+
+func (wr *journalWriter) getRange(h addr, lookups lookupMap) (rng Range, ok bool) {
+	var l recLookup
+	l, ok = lookups.get(h)
+	if ok {
+		rng = rangeFromLookup(l)
+	}
+	return
+}
+
+func (wr *journalWriter) getCompressed(h addr, lookups lookupMap) (CompressedChunk, error) {
+	l, ok := lookups.get(h)
+	if !ok {
+		return CompressedChunk{}, nil
+	}
+
+	buf := make([]byte, l.recordLen)
+	if _, err := wr.ReadAt(buf, l.journalOff); err != nil {
+		return CompressedChunk{}, nil
+	}
+
+	rec, err := readJournalRecord(buf)
+	if err != nil {
+		return CompressedChunk{}, err
+	} else if h != rec.address {
+		err = fmt.Errorf("chunk record hash does not match (%s != %s)",
+			h.String(), rec.address.String())
+		return CompressedChunk{}, err
+	}
+	return NewCompressedChunk(hash.Hash(h), rec.payload)
+}

From 9be1e16de9d0fb64bf0020cd476bb7e493886006 Mon Sep 17 00:00:00 2001
From: Maximilian Hoffman <max@dolthub.com>
Date: Fri, 17 Feb 2023 09:30:35 -0800
Subject: [PATCH 33/40] ArtifactEditor now resets after flushing (#5367)

ArtifactEditor held on to keys after flushing because it was
a value receiver. Before the flush threshold, constraint violations
only flush once, so we don't notice this bug. For >50k constraint
violations, every incremental CV will write the previous duplicate 40k
CV's.
---
 .../doltcore/merge/merge_prolly_indexes.go         |  4 ++--
 go/libraries/doltcore/merge/merge_prolly_rows.go   |  6 +++---
 go/libraries/doltcore/merge/violations_fk.go       |  2 +-
 .../doltcore/merge/violations_unique_prolly.go     |  4 ++--
 .../sqle/dtables/conflicts_tables_prolly.go        |  2 +-
 .../sqle/dtables/constraint_violations_prolly.go   |  2 +-
 go/store/prolly/artifact_map.go                    | 14 +++++++-------
 go/store/prolly/tuple_mutable_map.go               |  4 ----
 8 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/go/libraries/doltcore/merge/merge_prolly_indexes.go b/go/libraries/doltcore/merge/merge_prolly_indexes.go
index 9ded62e6cf..5d76b61759 100644
--- a/go/libraries/doltcore/merge/merge_prolly_indexes.go
+++ b/go/libraries/doltcore/merge/merge_prolly_indexes.go
@@ -107,7 +107,7 @@ func mergeProllySecondaryIndexes(
 	leftSet, rightSet durable.IndexSet,
 	finalSch schema.Schema,
 	finalRows durable.Index,
-	artifacts prolly.ArtifactsEditor,
+	artifacts *prolly.ArtifactsEditor,
 ) (durable.IndexSet, error) {
 
 	ancSet, err := tm.ancTbl.GetIndexSet(ctx)
@@ -197,7 +197,7 @@ func mergeProllySecondaryIndexes(
 	return mergedIndexSet, nil
 }
 
-func buildIndex(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeStore, postMergeSchema schema.Schema, index schema.Index, m prolly.Map, artEditor prolly.ArtifactsEditor, theirRootIsh doltdb.Rootish, tblName string) (durable.Index, error) {
+func buildIndex(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeStore, postMergeSchema schema.Schema, index schema.Index, m prolly.Map, artEditor *prolly.ArtifactsEditor, theirRootIsh doltdb.Rootish, tblName string) (durable.Index, error) {
 	if index.IsUnique() {
 		meta, err := makeUniqViolMeta(postMergeSchema, index)
 		if err != nil {
diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go
index 706a2a1c6d..73022666a3 100644
--- a/go/libraries/doltcore/merge/merge_prolly_rows.go
+++ b/go/libraries/doltcore/merge/merge_prolly_rows.go
@@ -377,7 +377,7 @@ func (m *valueMerger) processColumn(i int, left, right, base val.Tuple) ([]byte,
 }
 
 type conflictProcessor interface {
-	process(ctx context.Context, conflictChan chan confVals, artEditor prolly.ArtifactsEditor) error
+	process(ctx context.Context, conflictChan chan confVals, artEditor *prolly.ArtifactsEditor) error
 }
 
 func makeConflictProcessor(ctx context.Context, tm TableMerger) (conflictProcessor, error) {
@@ -434,7 +434,7 @@ func newInsertingProcessor(theirRootIsh, baseRootIsh doltdb.Rootish) (*inserting
 	return &p, nil
 }
 
-func (p *insertingProcessor) process(ctx context.Context, conflictChan chan confVals, artEditor prolly.ArtifactsEditor) error {
+func (p *insertingProcessor) process(ctx context.Context, conflictChan chan confVals, artEditor *prolly.ArtifactsEditor) error {
 	for {
 		select {
 		case conflict, ok := <-conflictChan:
@@ -453,7 +453,7 @@ func (p *insertingProcessor) process(ctx context.Context, conflictChan chan conf
 
 type abortingProcessor struct{}
 
-func (p abortingProcessor) process(ctx context.Context, conflictChan chan confVals, artEditor prolly.ArtifactsEditor) error {
+func (p abortingProcessor) process(ctx context.Context, conflictChan chan confVals, _ *prolly.ArtifactsEditor) error {
 	select {
 	case _, ok := <-conflictChan:
 		if !ok {
diff --git a/go/libraries/doltcore/merge/violations_fk.go b/go/libraries/doltcore/merge/violations_fk.go
index 4f1193001d..f99d7facef 100644
--- a/go/libraries/doltcore/merge/violations_fk.go
+++ b/go/libraries/doltcore/merge/violations_fk.go
@@ -212,7 +212,7 @@ type foreignKeyViolationWriter struct {
 	currTbl *doltdb.Table
 
 	// prolly
-	artEditor     prolly.ArtifactsEditor
+	artEditor     *prolly.ArtifactsEditor
 	kd            val.TupleDesc
 	cInfoJsonData []byte
 
diff --git a/go/libraries/doltcore/merge/violations_unique_prolly.go b/go/libraries/doltcore/merge/violations_unique_prolly.go
index d2f4b0b576..04b42ccf72 100644
--- a/go/libraries/doltcore/merge/violations_unique_prolly.go
+++ b/go/libraries/doltcore/merge/violations_unique_prolly.go
@@ -39,7 +39,7 @@ func addUniqIdxViols(
 	index schema.Index,
 	left, right, base prolly.Map,
 	m prolly.Map,
-	artEditor prolly.ArtifactsEditor,
+	artEditor *prolly.ArtifactsEditor,
 	theirRootIsh doltdb.Rootish,
 	tblName string) error {
 
@@ -153,7 +153,7 @@ func (m UniqCVMeta) PrettyPrint() string {
 	return jsonStr
 }
 
-func replaceUniqueKeyViolation(ctx context.Context, edt prolly.ArtifactsEditor, m prolly.Map, k val.Tuple, kd val.TupleDesc, theirRootIsh doltdb.Rootish, vInfo []byte, tblName string) error {
+func replaceUniqueKeyViolation(ctx context.Context, edt *prolly.ArtifactsEditor, m prolly.Map, k val.Tuple, kd val.TupleDesc, theirRootIsh doltdb.Rootish, vInfo []byte, tblName string) error {
 	var value val.Tuple
 	err := m.Get(ctx, k, func(_, v val.Tuple) error {
 		value = v
diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go
index 8b287c9219..d31517e924 100644
--- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go
+++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go
@@ -539,7 +539,7 @@ type prollyConflictDeleter struct {
 	kd, vd         val.TupleDesc
 	kB, vB         *val.TupleBuilder
 	pool           pool.BuffPool
-	ed             prolly.ArtifactsEditor
+	ed             *prolly.ArtifactsEditor
 	ct             ProllyConflictsTable
 	rs             RootSetter
 	ourDiffTypeIdx int
diff --git a/go/libraries/doltcore/sqle/dtables/constraint_violations_prolly.go b/go/libraries/doltcore/sqle/dtables/constraint_violations_prolly.go
index 11ca2cb391..3bbe6444a3 100644
--- a/go/libraries/doltcore/sqle/dtables/constraint_violations_prolly.go
+++ b/go/libraries/doltcore/sqle/dtables/constraint_violations_prolly.go
@@ -217,7 +217,7 @@ type prollyCVDeleter struct {
 	kd   val.TupleDesc
 	kb   *val.TupleBuilder
 	pool pool.BuffPool
-	ed   prolly.ArtifactsEditor
+	ed   *prolly.ArtifactsEditor
 	cvt  *prollyConstraintViolationsTable
 }
 
diff --git a/go/store/prolly/artifact_map.go b/go/store/prolly/artifact_map.go
index 7c46b78fb5..e6dc28ed3b 100644
--- a/go/store/prolly/artifact_map.go
+++ b/go/store/prolly/artifact_map.go
@@ -155,9 +155,9 @@ func (m ArtifactMap) Pool() pool.BuffPool {
 	return m.tuples.NodeStore.Pool()
 }
 
-func (m ArtifactMap) Editor() ArtifactsEditor {
+func (m ArtifactMap) Editor() *ArtifactsEditor {
 	artKD, artVD := m.Descriptors()
-	return ArtifactsEditor{
+	return &ArtifactsEditor{
 		srcKeyDesc: m.srcKeyDesc,
 		mut: MutableMap{
 			tuples:  m.tuples.Mutate(),
@@ -316,7 +316,7 @@ type ArtifactsEditor struct {
 	pool         pool.BuffPool
 }
 
-func (wr ArtifactsEditor) Add(ctx context.Context, srcKey val.Tuple, theirRootIsh hash.Hash, artType ArtifactType, meta []byte) error {
+func (wr *ArtifactsEditor) Add(ctx context.Context, srcKey val.Tuple, theirRootIsh hash.Hash, artType ArtifactType, meta []byte) error {
 	for i := 0; i < srcKey.Count(); i++ {
 		wr.artKB.PutRaw(i, srcKey.GetField(i))
 	}
@@ -344,7 +344,7 @@ func (e *ErrMergeArtifactCollision) Error() string {
 // the given will be inserted. Returns true if a violation was replaced. If an
 // existing violation exists but has a different |meta.VInfo| value then
 // ErrMergeArtifactCollision is a returned.
-func (wr ArtifactsEditor) ReplaceConstraintViolation(ctx context.Context, srcKey val.Tuple, theirRootIsh hash.Hash, artType ArtifactType, meta ConstraintViolationMeta) error {
+func (wr *ArtifactsEditor) ReplaceConstraintViolation(ctx context.Context, srcKey val.Tuple, theirRootIsh hash.Hash, artType ArtifactType, meta ConstraintViolationMeta) error {
 	itr, err := wr.mut.IterRange(ctx, PrefixRange(srcKey, wr.srcKeyDesc))
 	if err != nil {
 		return err
@@ -406,11 +406,11 @@ func (wr ArtifactsEditor) ReplaceConstraintViolation(ctx context.Context, srcKey
 	return nil
 }
 
-func (wr ArtifactsEditor) Delete(ctx context.Context, key val.Tuple) error {
+func (wr *ArtifactsEditor) Delete(ctx context.Context, key val.Tuple) error {
 	return wr.mut.Delete(ctx, key)
 }
 
-func (wr ArtifactsEditor) Flush(ctx context.Context) (ArtifactMap, error) {
+func (wr *ArtifactsEditor) Flush(ctx context.Context) (ArtifactMap, error) {
 	s := message.NewMergeArtifactSerializer(wr.artKB.Desc, wr.NodeStore().Pool())
 
 	m, err := wr.mut.flushWithSerializer(ctx, s)
@@ -426,7 +426,7 @@ func (wr ArtifactsEditor) Flush(ctx context.Context) (ArtifactMap, error) {
 	}, nil
 }
 
-func (wr ArtifactsEditor) NodeStore() tree.NodeStore {
+func (wr *ArtifactsEditor) NodeStore() tree.NodeStore {
 	return wr.mut.NodeStore()
 }
 
diff --git a/go/store/prolly/tuple_mutable_map.go b/go/store/prolly/tuple_mutable_map.go
index 0e89304048..f9a4acc05a 100644
--- a/go/store/prolly/tuple_mutable_map.go
+++ b/go/store/prolly/tuple_mutable_map.go
@@ -63,10 +63,6 @@ func (mut *MutableMap) Map(ctx context.Context) (Map, error) {
 }
 
 func (mut *MutableMap) flushWithSerializer(ctx context.Context, s message.Serializer) (Map, error) {
-	if err := mut.Checkpoint(ctx); err != nil {
-		return Map{}, err
-	}
-
 	sm := mut.tuples.StaticMap
 	fn := tree.ApplyMutations[val.Tuple, val.TupleDesc, message.Serializer]
 

From 2ddf1fd9f59e59d58914980ee5b1ab24fd2a5ac0 Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Fri, 17 Feb 2023 09:45:42 -0800
Subject: [PATCH 34/40] go/store/nbs: move chunk index hashmap into journal
 writer

---
 go/store/nbs/journal.go              | 15 ++----
 go/store/nbs/journal_chunk_source.go | 55 ++++---------------
 go/store/nbs/journal_writer.go       | 80 +++++++++++++++++-----------
 go/store/nbs/journal_writer_test.go  | 21 ++++----
 4 files changed, 73 insertions(+), 98 deletions(-)

diff --git a/go/store/nbs/journal.go b/go/store/nbs/journal.go
index ee2fba0832..3cd357944d 100644
--- a/go/store/nbs/journal.go
+++ b/go/store/nbs/journal.go
@@ -56,7 +56,6 @@ const (
 // both memTable persists and manifest updates to a single file.
 type chunkJournal struct {
 	wr   *journalWriter
-	src  journalChunkSource
 	path string
 
 	contents  manifestContents
@@ -105,7 +104,7 @@ func (j *chunkJournal) openJournal(ctx context.Context) (err error) {
 			return err
 		}
 
-		_, j.src, err = j.wr.ProcessJournal(ctx)
+		_, err = j.wr.ProcessJournal(ctx)
 		if err != nil {
 			return err
 		}
@@ -133,8 +132,7 @@ func (j *chunkJournal) openJournal(ctx context.Context) (err error) {
 	}
 
 	// parse existing journal file
-	var root hash.Hash
-	root, j.src, err = j.wr.ProcessJournal(ctx)
+	root, err := j.wr.ProcessJournal(ctx)
 	if err != nil {
 		return err
 	}
@@ -178,15 +176,12 @@ func (j *chunkJournal) Persist(ctx context.Context, mt *memTable, haver chunkRea
 			continue
 		}
 		c := chunks.NewChunkWithHash(hash.Hash(*record.a), mt.chunks[*record.a])
-		cc := ChunkToCompressedChunk(c)
-		lookup, err := j.wr.WriteChunk(cc)
+		err := j.wr.WriteChunk(ChunkToCompressedChunk(c))
 		if err != nil {
 			return nil, err
 		}
-		j.src.lookups.put(*record.a, lookup)
-		j.src.uncompressedSz += uint64(c.Size())
 	}
-	return j.src, nil
+	return journalChunkSource{journal: j.wr}, nil
 }
 
 // ConjoinAll implements tablePersister.
@@ -200,7 +195,7 @@ func (j *chunkJournal) Open(ctx context.Context, name addr, chunkCount uint32, s
 		if err := j.maybeInit(ctx); err != nil {
 			return nil, err
 		}
-		return j.src, nil
+		return journalChunkSource{journal: j.wr}, nil
 	}
 	return j.persister.Open(ctx, name, chunkCount, stats)
 }
diff --git a/go/store/nbs/journal_chunk_source.go b/go/store/nbs/journal_chunk_source.go
index 26c7748829..7f56823d60 100644
--- a/go/store/nbs/journal_chunk_source.go
+++ b/go/store/nbs/journal_chunk_source.go
@@ -18,7 +18,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"sync"
 
 	"golang.org/x/sync/errgroup"
 
@@ -49,59 +48,23 @@ func rangeFromLookup(l recLookup) Range {
 	}
 }
 
-// lookupMap is a thread-safe collection of recLookups.
-type lookupMap struct {
-	data map[addr]recLookup
-	lock *sync.RWMutex
-}
-
-func newLookupMap() lookupMap {
-	return lookupMap{
-		data: make(map[addr]recLookup),
-		lock: new(sync.RWMutex),
-	}
-}
-
-func (m lookupMap) get(a addr) (l recLookup, ok bool) {
-	m.lock.RLock()
-	defer m.lock.RUnlock()
-	l, ok = m.data[a]
-	return
-}
-
-func (m lookupMap) put(a addr, l recLookup) {
-	m.lock.Lock()
-	defer m.lock.Unlock()
-	m.data[a] = l
-	return
-}
-
-func (m lookupMap) count() int {
-	m.lock.RLock()
-	defer m.lock.RUnlock()
-	return len(m.data)
-}
-
 // journalChunkSource is a chunkSource that reads chunks
 // from a chunkJournal. Unlike other NBS chunkSources,
 // it is not immutable and its set of chunks grows as
 // more commits are made to the chunkJournal.
 type journalChunkSource struct {
-	address        addr
-	journal        *journalWriter
-	lookups        lookupMap
-	uncompressedSz uint64
+	journal *journalWriter
 }
 
 var _ chunkSource = journalChunkSource{}
 
 func (s journalChunkSource) has(h addr) (bool, error) {
-	return s.journal.has(h, s.lookups), nil
+	return s.journal.has(h), nil
 }
 
 func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error) {
 	for i := range addrs {
-		ok := s.journal.has(*addrs[i].a, s.lookups)
+		ok := s.journal.has(*addrs[i].a)
 		if ok {
 			addrs[i].has = true
 		} else {
@@ -112,11 +75,11 @@ func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error)
 }
 
 func (s journalChunkSource) getCompressed(_ context.Context, h addr, _ *Stats) (CompressedChunk, error) {
-	return s.journal.getCompressed(h, s.lookups)
+	return s.journal.getCompressed(h)
 }
 
 func (s journalChunkSource) get(_ context.Context, h addr, _ *Stats) ([]byte, error) {
-	cc, err := s.journal.getCompressed(h, s.lookups)
+	cc, err := s.journal.getCompressed(h)
 	if err != nil {
 		return nil, err
 	} else if cc.IsEmpty() {
@@ -163,15 +126,15 @@ func (s journalChunkSource) getManyCompressed(ctx context.Context, _ *errgroup.G
 }
 
 func (s journalChunkSource) count() (uint32, error) {
-	return uint32(s.lookups.count()), nil
+	return s.journal.recordCount(), nil
 }
 
 func (s journalChunkSource) uncompressedLen() (uint64, error) {
-	return s.uncompressedSz, nil
+	return s.journal.uncompressedSize(), nil
 }
 
 func (s journalChunkSource) hash() addr {
-	return s.address
+	return journalAddr
 }
 
 // reader implements chunkSource.
@@ -186,7 +149,7 @@ func (s journalChunkSource) getRecordRanges(requests []getRecord) (map[hash.Hash
 		if req.found {
 			continue
 		}
-		rng, ok := s.journal.getRange(*req.a, s.lookups)
+		rng, ok := s.journal.getRange(*req.a)
 		if !ok {
 			continue
 		}
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index c176d6da32..f21e0550ac 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -78,9 +78,10 @@ func openJournalWriter(ctx context.Context, path string) (wr *journalWriter, exi
 	}
 
 	return &journalWriter{
-		buf:  make([]byte, 0, journalWriterBuffSize),
-		file: f,
-		path: path,
+		buf:     make([]byte, 0, journalWriterBuffSize),
+		lookups: make(map[addr]recLookup),
+		file:    f,
+		path:    path,
 	}, true, nil
 }
 
@@ -117,9 +118,10 @@ func createJournalWriter(ctx context.Context, path string) (wr *journalWriter, e
 	}
 
 	return &journalWriter{
-		buf:  make([]byte, 0, journalWriterBuffSize),
-		file: f,
-		path: path,
+		buf:     make([]byte, 0, journalWriterBuffSize),
+		lookups: make(map[addr]recLookup),
+		file:    f,
+		path:    path,
 	}, nil
 }
 
@@ -134,11 +136,13 @@ type snapshotReader interface {
 }
 
 type journalWriter struct {
-	buf  []byte
-	file *os.File
-	off  int64
-	path string
-	lock sync.RWMutex
+	buf     []byte
+	lookups map[addr]recLookup
+	file    *os.File
+	off     int64
+	uncmpSz uint64
+	path    string
+	lock    sync.RWMutex
 }
 
 var _ io.WriteCloser = &journalWriter{}
@@ -190,6 +194,7 @@ func (wr *journalWriter) CurrentSize() int64 {
 	return wr.off
 }
 
+// todo: remove, this method is only used for testing
 func (wr *journalWriter) Write(p []byte) (n int, err error) {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
@@ -210,23 +215,18 @@ func (wr *journalWriter) Write(p []byte) (n int, err error) {
 	return
 }
 
-func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, cs journalChunkSource, err error) {
+func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, err error) {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
-	src := journalChunkSource{
-		journal: wr,
-		address: journalAddr,
-		lookups: newLookupMap(),
-	}
 	wr.off, err = processJournalRecords(ctx, wr.file, func(o int64, r journalRec) error {
 		switch r.kind {
 		case chunkJournalRecKind:
-			src.lookups.put(r.address, recLookup{
+			wr.lookups[r.address] = recLookup{
 				journalOff: o,
 				recordLen:  r.length,
 				payloadOff: r.payloadOffset(),
-			})
-			src.uncompressedSz += r.uncompressedPayloadSize()
+			}
+			wr.uncmpSz += r.uncompressedPayloadSize()
 		case rootHashJournalRecKind:
 			last = hash.Hash(r.address)
 		default:
@@ -235,13 +235,12 @@ func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, cs
 		return nil
 	})
 	if err != nil {
-		return hash.Hash{}, journalChunkSource{}, err
+		return hash.Hash{}, err
 	}
-	cs = src
 	return
 }
 
-func (wr *journalWriter) WriteChunk(cc CompressedChunk) (recLookup, error) {
+func (wr *journalWriter) WriteChunk(cc CompressedChunk) error {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
 	l, o := chunkRecordSize(cc)
@@ -252,10 +251,11 @@ func (wr *journalWriter) WriteChunk(cc CompressedChunk) (recLookup, error) {
 	}
 	buf, err := wr.getBytes(int(rec.recordLen))
 	if err != nil {
-		return recLookup{}, err
+		return err
 	}
 	_ = writeChunkRecord(buf, cc)
-	return rec, nil
+	wr.lookups[addr(cc.H)] = rec
+	return nil
 }
 
 func (wr *journalWriter) WriteRootHash(root hash.Hash) error {
@@ -317,22 +317,28 @@ func (wr *journalWriter) flush() (err error) {
 	return
 }
 
-func (wr *journalWriter) has(h addr, lookups lookupMap) (ok bool) {
-	_, ok = lookups.get(h)
+func (wr *journalWriter) has(h addr) (ok bool) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	_, ok = wr.lookups[h]
 	return
 }
 
-func (wr *journalWriter) getRange(h addr, lookups lookupMap) (rng Range, ok bool) {
+func (wr *journalWriter) getRange(h addr) (rng Range, ok bool) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
 	var l recLookup
-	l, ok = lookups.get(h)
+	l, ok = wr.lookups[h]
 	if ok {
 		rng = rangeFromLookup(l)
 	}
 	return
 }
 
-func (wr *journalWriter) getCompressed(h addr, lookups lookupMap) (CompressedChunk, error) {
-	l, ok := lookups.get(h)
+func (wr *journalWriter) getCompressed(h addr) (CompressedChunk, error) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	l, ok := wr.lookups[h]
 	if !ok {
 		return CompressedChunk{}, nil
 	}
@@ -352,3 +358,15 @@ func (wr *journalWriter) getCompressed(h addr, lookups lookupMap) (CompressedChu
 	}
 	return NewCompressedChunk(hash.Hash(h), rec.payload)
 }
+
+func (wr *journalWriter) recordCount() uint32 {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	return uint32(len(wr.lookups))
+}
+
+func (wr *journalWriter) uncompressedSize() uint64 {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	return wr.uncmpSz
+}
diff --git a/go/store/nbs/journal_writer_test.go b/go/store/nbs/journal_writer_test.go
index 087ed8eef1..ba67a02a72 100644
--- a/go/store/nbs/journal_writer_test.go
+++ b/go/store/nbs/journal_writer_test.go
@@ -195,15 +195,14 @@ func TestJournalWriterWriteChunk(t *testing.T) {
 	require.NoError(t, err)
 
 	data := randomCompressedChunks()
-	lookups := make(map[addr]recLookup)
 
 	for a, cc := range data {
-		l, err := j.WriteChunk(cc)
+		err = j.WriteChunk(cc)
 		require.NoError(t, err)
-		lookups[a] = l
+		l := j.lookups[a]
 		validateLookup(t, j, l, cc)
 	}
-	for a, l := range lookups {
+	for a, l := range j.lookups {
 		validateLookup(t, j, l, data[a])
 	}
 	require.NoError(t, j.Close())
@@ -217,22 +216,22 @@ func TestJournalWriterBootstrap(t *testing.T) {
 	require.NoError(t, err)
 
 	data := randomCompressedChunks()
-	lookups := make(map[addr]recLookup)
-	for a, cc := range data {
-		l, err := j.WriteChunk(cc)
+	for _, cc := range data {
+		err = j.WriteChunk(cc)
 		require.NoError(t, err)
-		lookups[a] = l
 	}
 	assert.NoError(t, j.Close())
 
 	j, _, err = openJournalWriter(ctx, path)
 	require.NoError(t, err)
-	_, source, err := j.ProcessJournal(ctx)
+	_, err = j.ProcessJournal(ctx)
 	require.NoError(t, err)
 
-	for a, l := range lookups {
+	for a, l := range j.lookups {
 		validateLookup(t, j, l, data[a])
 	}
+
+	source := journalChunkSource{journal: j}
 	for a, cc := range data {
 		buf, err := source.get(ctx, a, nil)
 		require.NoError(t, err)
@@ -259,7 +258,7 @@ func TestJournalWriterSyncClose(t *testing.T) {
 	j, err := createJournalWriter(ctx, newTestFilePath(t))
 	require.NotNil(t, j)
 	require.NoError(t, err)
-	_, _, err = j.ProcessJournal(ctx)
+	_, err = j.ProcessJournal(ctx)
 	require.NoError(t, err)
 
 	// close triggers flush

From 4036810654631046ea5ef8fa5ab72f29ac2ffbf2 Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Fri, 17 Feb 2023 11:32:38 -0800
Subject: [PATCH 35/40] go/store/nbs: flush journal writer before returning
 record ranges

---
 go/store/nbs/journal_chunk_source.go |  6 ++++--
 go/store/nbs/journal_writer.go       | 19 ++++++++++++++++++-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/go/store/nbs/journal_chunk_source.go b/go/store/nbs/journal_chunk_source.go
index 7f56823d60..7fcac3ff8e 100644
--- a/go/store/nbs/journal_chunk_source.go
+++ b/go/store/nbs/journal_chunk_source.go
@@ -149,8 +149,10 @@ func (s journalChunkSource) getRecordRanges(requests []getRecord) (map[hash.Hash
 		if req.found {
 			continue
 		}
-		rng, ok := s.journal.getRange(*req.a)
-		if !ok {
+		rng, ok, err := s.journal.getRange(*req.a)
+		if err != nil {
+			return nil, err
+		} else if !ok {
 			continue
 		}
 		req.found = true // update |requests|
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index f21e0550ac..8a5f073a8e 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -317,6 +317,18 @@ func (wr *journalWriter) flush() (err error) {
 	return
 }
 
+func (wr *journalWriter) maybeFlush() (err error) {
+	wr.lock.RLock()
+	empty := len(wr.buf) == 0
+	wr.lock.RUnlock()
+	if empty {
+		return
+	}
+	wr.lock.Lock()
+	defer wr.lock.Unlock()
+	return wr.flush()
+}
+
 func (wr *journalWriter) has(h addr) (ok bool) {
 	wr.lock.RLock()
 	defer wr.lock.RUnlock()
@@ -324,7 +336,12 @@ func (wr *journalWriter) has(h addr) (ok bool) {
 	return
 }
 
-func (wr *journalWriter) getRange(h addr) (rng Range, ok bool) {
+func (wr *journalWriter) getRange(h addr) (rng Range, ok bool, err error) {
+	// callers will use |rng| to read directly from the
+	// journal file, so we must flush here
+	if err = wr.maybeFlush(); err != nil {
+		return
+	}
 	wr.lock.RLock()
 	defer wr.lock.RUnlock()
 	var l recLookup

From a2b7ce923c412baaae38ea95766ead4f548db07f Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Fri, 17 Feb 2023 13:01:01 -0800
Subject: [PATCH 36/40] go/store/nbs: cleanup, rename methods of journalWriter

---
 go/store/nbs/journal.go              |  10 +-
 go/store/nbs/journal_chunk_source.go |  13 +-
 go/store/nbs/journal_test.go         |   2 +-
 go/store/nbs/journal_writer.go       | 267 +++++++++++++--------------
 go/store/nbs/journal_writer_test.go  |  38 ++--
 5 files changed, 153 insertions(+), 177 deletions(-)

diff --git a/go/store/nbs/journal.go b/go/store/nbs/journal.go
index 3cd357944d..e4d7adea14 100644
--- a/go/store/nbs/journal.go
+++ b/go/store/nbs/journal.go
@@ -104,7 +104,7 @@ func (j *chunkJournal) openJournal(ctx context.Context) (err error) {
 			return err
 		}
 
-		_, err = j.wr.ProcessJournal(ctx)
+		_, err = j.wr.bootstrapJournal(ctx)
 		if err != nil {
 			return err
 		}
@@ -116,7 +116,7 @@ func (j *chunkJournal) openJournal(ctx context.Context) (err error) {
 		}
 		if ok {
 			// write the current root hash to the journal file
-			if err = j.wr.WriteRootHash(contents.root); err != nil {
+			if err = j.wr.writeRootHash(contents.root); err != nil {
 				return
 			}
 			j.contents = contents
@@ -132,7 +132,7 @@ func (j *chunkJournal) openJournal(ctx context.Context) (err error) {
 	}
 
 	// parse existing journal file
-	root, err := j.wr.ProcessJournal(ctx)
+	root, err := j.wr.bootstrapJournal(ctx)
 	if err != nil {
 		return err
 	}
@@ -176,7 +176,7 @@ func (j *chunkJournal) Persist(ctx context.Context, mt *memTable, haver chunkRea
 			continue
 		}
 		c := chunks.NewChunkWithHash(hash.Hash(*record.a), mt.chunks[*record.a])
-		err := j.wr.WriteChunk(ChunkToCompressedChunk(c))
+		err := j.wr.writeCompressedChunk(ChunkToCompressedChunk(c))
 		if err != nil {
 			return nil, err
 		}
@@ -258,7 +258,7 @@ func (j *chunkJournal) Update(ctx context.Context, lastLock addr, next manifestC
 		}
 	}
 
-	if err := j.wr.WriteRootHash(next.root); err != nil {
+	if err := j.wr.writeRootHash(next.root); err != nil {
 		return manifestContents{}, err
 	}
 	j.contents = next
diff --git a/go/store/nbs/journal_chunk_source.go b/go/store/nbs/journal_chunk_source.go
index 7fcac3ff8e..3bf7f1fc8c 100644
--- a/go/store/nbs/journal_chunk_source.go
+++ b/go/store/nbs/journal_chunk_source.go
@@ -59,12 +59,12 @@ type journalChunkSource struct {
 var _ chunkSource = journalChunkSource{}
 
 func (s journalChunkSource) has(h addr) (bool, error) {
-	return s.journal.has(h), nil
+	return s.journal.hasAddr(h), nil
 }
 
 func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error) {
 	for i := range addrs {
-		ok := s.journal.has(*addrs[i].a)
+		ok := s.journal.hasAddr(*addrs[i].a)
 		if ok {
 			addrs[i].has = true
 		} else {
@@ -75,11 +75,11 @@ func (s journalChunkSource) hasMany(addrs []hasRecord) (missing bool, err error)
 }
 
 func (s journalChunkSource) getCompressed(_ context.Context, h addr, _ *Stats) (CompressedChunk, error) {
-	return s.journal.getCompressed(h)
+	return s.journal.getCompressedChunk(h)
 }
 
 func (s journalChunkSource) get(_ context.Context, h addr, _ *Stats) ([]byte, error) {
-	cc, err := s.journal.getCompressed(h)
+	cc, err := s.journal.getCompressedChunk(h)
 	if err != nil {
 		return nil, err
 	} else if cc.IsEmpty() {
@@ -139,7 +139,7 @@ func (s journalChunkSource) hash() addr {
 
 // reader implements chunkSource.
 func (s journalChunkSource) reader(context.Context) (io.ReadCloser, uint64, error) {
-	rdr, sz, err := s.journal.Snapshot()
+	rdr, sz, err := s.journal.snapshot()
 	return io.NopCloser(rdr), uint64(sz), err
 }
 
@@ -164,7 +164,7 @@ func (s journalChunkSource) getRecordRanges(requests []getRecord) (map[hash.Hash
 // size implements chunkSource.
 // size returns the total size of the chunkSource: chunks, index, and footer
 func (s journalChunkSource) currentSize() uint64 {
-	return uint64(s.journal.CurrentSize())
+	return uint64(s.journal.currentSize())
 }
 
 // index implements chunkSource.
@@ -177,6 +177,7 @@ func (s journalChunkSource) clone() (chunkSource, error) {
 }
 
 func (s journalChunkSource) close() error {
+	// |s.journal| closed via chunkJournal
 	return nil
 }
 
diff --git a/go/store/nbs/journal_test.go b/go/store/nbs/journal_test.go
index 3d2ced73b8..ae1a9a67d5 100644
--- a/go/store/nbs/journal_test.go
+++ b/go/store/nbs/journal_test.go
@@ -96,7 +96,7 @@ func TestReadRecordRanges(t *testing.T) {
 	jcs, err := j.Persist(ctx, mt, emptyChunkSource{}, &Stats{})
 	require.NoError(t, err)
 
-	rdr, sz, err := jcs.(journalChunkSource).journal.Snapshot()
+	rdr, sz, err := jcs.(journalChunkSource).journal.snapshot()
 	require.NoError(t, err)
 
 	buf = make([]byte, sz)
diff --git a/go/store/nbs/journal_writer.go b/go/store/nbs/journal_writer.go
index 8a5f073a8e..4841d88e91 100644
--- a/go/store/nbs/journal_writer.go
+++ b/go/store/nbs/journal_writer.go
@@ -125,16 +125,6 @@ func createJournalWriter(ctx context.Context, path string) (wr *journalWriter, e
 	}, nil
 }
 
-type snapshotReader interface {
-	io.ReaderAt
-	// Snapshot returns an io.Reader that provides a consistent view
-	// of the current state of the snapshotReader.
-	Snapshot() (io.Reader, int64, error)
-
-	// currentSize returns the current size.
-	CurrentSize() int64
-}
-
 type journalWriter struct {
 	buf     []byte
 	lookups map[addr]recLookup
@@ -145,77 +135,11 @@ type journalWriter struct {
 	lock    sync.RWMutex
 }
 
-var _ io.WriteCloser = &journalWriter{}
-var _ snapshotReader = &journalWriter{}
+var _ io.Closer = &journalWriter{}
 
-func (wr *journalWriter) ReadAt(p []byte, off int64) (n int, err error) {
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	var bp []byte
-	if off < wr.off {
-		// fill some or all of |p| from |wr.file|
-		fread := int(wr.off - off)
-		if len(p) > fread {
-			// straddled read
-			bp = p[fread:]
-			p = p[:fread]
-		}
-		if n, err = wr.file.ReadAt(p, off); err != nil {
-			return 0, err
-		}
-		off = 0
-	} else {
-		// fill all of |p| from |wr.buf|
-		bp = p
-		off -= wr.off
-	}
-	n += copy(bp, wr.buf[off:])
-	return
-}
-
-func (wr *journalWriter) Snapshot() (io.Reader, int64, error) {
-	wr.lock.Lock()
-	defer wr.lock.Unlock()
-	if err := wr.flush(); err != nil {
-		return nil, 0, err
-	}
-	// open a new file descriptor with an
-	// independent lifecycle from |wr.file|
-	f, err := os.Open(wr.path)
-	if err != nil {
-		return nil, 0, err
-	}
-	return io.LimitReader(f, wr.off), wr.off, nil
-}
-
-func (wr *journalWriter) CurrentSize() int64 {
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	return wr.off
-}
-
-// todo: remove, this method is only used for testing
-func (wr *journalWriter) Write(p []byte) (n int, err error) {
-	wr.lock.Lock()
-	defer wr.lock.Unlock()
-	if len(p) > len(wr.buf) {
-		// write directly to |wr.file|
-		if err = wr.flush(); err != nil {
-			return 0, err
-		}
-		n, err = wr.file.WriteAt(p, wr.off)
-		wr.off += int64(n)
-		return
-	}
-	var buf []byte
-	if buf, err = wr.getBytes(len(p)); err != nil {
-		return 0, err
-	}
-	n = copy(buf, p)
-	return
-}
-
-func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, err error) {
+// bootstrapJournal reads the journal file collecting a recLookup for each record and
+// returning the latest committed root hash.
+func (wr *journalWriter) bootstrapJournal(ctx context.Context) (last hash.Hash, err error) {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
 	wr.off, err = processJournalRecords(ctx, wr.file, func(o int64, r journalRec) error {
@@ -240,7 +164,58 @@ func (wr *journalWriter) ProcessJournal(ctx context.Context) (last hash.Hash, er
 	return
 }
 
-func (wr *journalWriter) WriteChunk(cc CompressedChunk) error {
+// hasAddr returns true if the journal contains a chunk with addr |h|.
+func (wr *journalWriter) hasAddr(h addr) (ok bool) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	_, ok = wr.lookups[h]
+	return
+}
+
+// getCompressedChunk reads the CompressedChunks with addr |h|.
+func (wr *journalWriter) getCompressedChunk(h addr) (CompressedChunk, error) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	l, ok := wr.lookups[h]
+	if !ok {
+		return CompressedChunk{}, nil
+	}
+
+	buf := make([]byte, l.recordLen)
+	if _, err := wr.readAt(buf, l.journalOff); err != nil {
+		return CompressedChunk{}, nil
+	}
+
+	rec, err := readJournalRecord(buf)
+	if err != nil {
+		return CompressedChunk{}, err
+	} else if h != rec.address {
+		err = fmt.Errorf("chunk record hash does not match (%s != %s)",
+			h.String(), rec.address.String())
+		return CompressedChunk{}, err
+	}
+	return NewCompressedChunk(hash.Hash(h), rec.payload)
+}
+
+// getRange returns a Range for the chunk with addr |h|.
+func (wr *journalWriter) getRange(h addr) (rng Range, ok bool, err error) {
+	// callers will use |rng| to read directly from the
+	// journal file, so we must flush here
+	if err = wr.maybeFlush(); err != nil {
+		return
+	}
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	var l recLookup
+	l, ok = wr.lookups[h]
+	if ok {
+		rng = rangeFromLookup(l)
+	}
+	return
+}
+
+// writeCompressedChunk writes |cc| to the journal.
+func (wr *journalWriter) writeCompressedChunk(cc CompressedChunk) error {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
 	l, o := chunkRecordSize(cc)
@@ -258,7 +233,8 @@ func (wr *journalWriter) WriteChunk(cc CompressedChunk) error {
 	return nil
 }
 
-func (wr *journalWriter) WriteRootHash(root hash.Hash) error {
+// writeRootHash commits |root| to the journal and syncs the file to disk.
+func (wr *journalWriter) writeRootHash(root hash.Hash) error {
 	wr.lock.Lock()
 	defer wr.lock.Unlock()
 	buf, err := wr.getBytes(rootHashRecordSize())
@@ -273,25 +249,33 @@ func (wr *journalWriter) WriteRootHash(root hash.Hash) error {
 	return wr.file.Sync()
 }
 
-func (wr *journalWriter) Close() (err error) {
-	wr.lock.Lock()
-	defer wr.lock.Unlock()
-	if err = wr.flush(); err != nil {
-		return err
-	}
-	if cerr := wr.file.Sync(); cerr != nil {
-		err = cerr
-	}
-	if cerr := wr.file.Close(); cerr != nil {
-		err = cerr
+// readAt reads len(p) bytes from the journal at offset |off|.
+func (wr *journalWriter) readAt(p []byte, off int64) (n int, err error) {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	var bp []byte
+	if off < wr.off {
+		// fill some or all of |p| from |wr.file|
+		fread := int(wr.off - off)
+		if len(p) > fread {
+			// straddled read
+			bp = p[fread:]
+			p = p[:fread]
+		}
+		if n, err = wr.file.ReadAt(p, off); err != nil {
+			return 0, err
+		}
+		off = 0
+	} else {
+		// fill all of |p| from |wr.buf|
+		bp = p
+		off -= wr.off
 	}
+	n += copy(bp, wr.buf[off:])
 	return
 }
 
-func (wr *journalWriter) offset() int64 {
-	return wr.off + int64(len(wr.buf))
-}
-
+// getBytes returns a buffer for writers to copy data into.
 func (wr *journalWriter) getBytes(n int) (buf []byte, err error) {
 	c, l := cap(wr.buf), len(wr.buf)
 	if n > c {
@@ -308,6 +292,7 @@ func (wr *journalWriter) getBytes(n int) (buf []byte, err error) {
 	return
 }
 
+// flush writes buffered data into the journal file.
 func (wr *journalWriter) flush() (err error) {
 	if _, err = wr.file.WriteAt(wr.buf, wr.off); err != nil {
 		return err
@@ -317,6 +302,7 @@ func (wr *journalWriter) flush() (err error) {
 	return
 }
 
+// maybeFlush flushes buffered data, if any exists.
 func (wr *journalWriter) maybeFlush() (err error) {
 	wr.lock.RLock()
 	empty := len(wr.buf) == 0
@@ -329,51 +315,37 @@ func (wr *journalWriter) maybeFlush() (err error) {
 	return wr.flush()
 }
 
-func (wr *journalWriter) has(h addr) (ok bool) {
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	_, ok = wr.lookups[h]
-	return
-}
-
-func (wr *journalWriter) getRange(h addr) (rng Range, ok bool, err error) {
-	// callers will use |rng| to read directly from the
-	// journal file, so we must flush here
-	if err = wr.maybeFlush(); err != nil {
-		return
+// snapshot returns an io.Reader with a consistent view of
+// the current state of the journal file.
+func (wr *journalWriter) snapshot() (io.Reader, int64, error) {
+	wr.lock.Lock()
+	defer wr.lock.Unlock()
+	if err := wr.flush(); err != nil {
+		return nil, 0, err
 	}
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	var l recLookup
-	l, ok = wr.lookups[h]
-	if ok {
-		rng = rangeFromLookup(l)
-	}
-	return
-}
-
-func (wr *journalWriter) getCompressed(h addr) (CompressedChunk, error) {
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	l, ok := wr.lookups[h]
-	if !ok {
-		return CompressedChunk{}, nil
-	}
-
-	buf := make([]byte, l.recordLen)
-	if _, err := wr.ReadAt(buf, l.journalOff); err != nil {
-		return CompressedChunk{}, nil
-	}
-
-	rec, err := readJournalRecord(buf)
+	// open a new file descriptor with an
+	// independent lifecycle from |wr.file|
+	f, err := os.Open(wr.path)
 	if err != nil {
-		return CompressedChunk{}, err
-	} else if h != rec.address {
-		err = fmt.Errorf("chunk record hash does not match (%s != %s)",
-			h.String(), rec.address.String())
-		return CompressedChunk{}, err
+		return nil, 0, err
 	}
-	return NewCompressedChunk(hash.Hash(h), rec.payload)
+	return io.LimitReader(f, wr.off), wr.off, nil
+}
+
+func (wr *journalWriter) offset() int64 {
+	return wr.off + int64(len(wr.buf))
+}
+
+func (wr *journalWriter) currentSize() int64 {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	return wr.offset()
+}
+
+func (wr *journalWriter) uncompressedSize() uint64 {
+	wr.lock.RLock()
+	defer wr.lock.RUnlock()
+	return wr.uncmpSz
 }
 
 func (wr *journalWriter) recordCount() uint32 {
@@ -382,8 +354,17 @@ func (wr *journalWriter) recordCount() uint32 {
 	return uint32(len(wr.lookups))
 }
 
-func (wr *journalWriter) uncompressedSize() uint64 {
-	wr.lock.RLock()
-	defer wr.lock.RUnlock()
-	return wr.uncmpSz
+func (wr *journalWriter) Close() (err error) {
+	wr.lock.Lock()
+	defer wr.lock.Unlock()
+	if err = wr.flush(); err != nil {
+		return err
+	}
+	if cerr := wr.file.Sync(); cerr != nil {
+		err = cerr
+	}
+	if cerr := wr.file.Close(); cerr != nil {
+		err = cerr
+	}
+	return
 }
diff --git a/go/store/nbs/journal_writer_test.go b/go/store/nbs/journal_writer_test.go
index ba67a02a72..c8db1f8ae1 100644
--- a/go/store/nbs/journal_writer_test.go
+++ b/go/store/nbs/journal_writer_test.go
@@ -123,17 +123,6 @@ func TestJournalWriter(t *testing.T) {
 				{kind: readOp, buf: []byte("loremipsumdolorsitamet"), readAt: 0},
 			},
 		},
-		{
-			name: "write larger that buffer",
-			size: 8,
-			ops: []operation{
-				{kind: writeOp, buf: []byte("loremipsum")},
-				{kind: flushOp},
-				{kind: writeOp, buf: []byte("dolorsitamet")},
-				{kind: readOp, buf: []byte("dolorsitamet"), readAt: 10},
-				{kind: readOp, buf: []byte("loremipsumdolorsitamet"), readAt: 0},
-			},
-		},
 		{
 			name: "flush empty buffer",
 			size: 16,
@@ -160,19 +149,23 @@ func TestJournalWriter(t *testing.T) {
 			j, err := createJournalWriter(ctx, newTestFilePath(t))
 			require.NotNil(t, j)
 			require.NoError(t, err)
+			// set specific buffer size
+			j.buf = make([]byte, 0, test.size)
 
 			var off int64
 			for i, op := range test.ops {
 				switch op.kind {
 				case readOp:
 					act := make([]byte, len(op.buf))
-					n, err := j.ReadAt(act, op.readAt)
+					n, err := j.readAt(act, op.readAt)
 					assert.NoError(t, err, "operation %d errored", i)
 					assert.Equal(t, len(op.buf), n, "operation %d failed", i)
 					assert.Equal(t, op.buf, act, "operation %d failed", i)
 				case writeOp:
-					n, err := j.Write(op.buf)
-					assert.NoError(t, err, "operation %d errored", i)
+					var p []byte
+					p, err = j.getBytes(len(op.buf))
+					require.NoError(t, err, "operation %d errored", i)
+					n := copy(p, op.buf)
 					assert.Equal(t, len(op.buf), n, "operation %d failed", i)
 					off += int64(n)
 				case flushOp:
@@ -188,7 +181,7 @@ func TestJournalWriter(t *testing.T) {
 	}
 }
 
-func TestJournalWriterWriteChunk(t *testing.T) {
+func TestJournalWriterWriteCompressedChunk(t *testing.T) {
 	ctx := context.Background()
 	j, err := createJournalWriter(ctx, newTestFilePath(t))
 	require.NotNil(t, j)
@@ -197,7 +190,7 @@ func TestJournalWriterWriteChunk(t *testing.T) {
 	data := randomCompressedChunks()
 
 	for a, cc := range data {
-		err = j.WriteChunk(cc)
+		err = j.writeCompressedChunk(cc)
 		require.NoError(t, err)
 		l := j.lookups[a]
 		validateLookup(t, j, l, cc)
@@ -217,14 +210,14 @@ func TestJournalWriterBootstrap(t *testing.T) {
 
 	data := randomCompressedChunks()
 	for _, cc := range data {
-		err = j.WriteChunk(cc)
+		err = j.writeCompressedChunk(cc)
 		require.NoError(t, err)
 	}
 	assert.NoError(t, j.Close())
 
 	j, _, err = openJournalWriter(ctx, path)
 	require.NoError(t, err)
-	_, err = j.ProcessJournal(ctx)
+	_, err = j.bootstrapJournal(ctx)
 	require.NoError(t, err)
 
 	for a, l := range j.lookups {
@@ -244,7 +237,7 @@ func TestJournalWriterBootstrap(t *testing.T) {
 
 func validateLookup(t *testing.T, j *journalWriter, l recLookup, cc CompressedChunk) {
 	b := make([]byte, l.recordLen)
-	n, err := j.ReadAt(b, l.journalOff)
+	n, err := j.readAt(b, l.journalOff)
 	require.NoError(t, err)
 	assert.Equal(t, int(l.recordLen), n)
 	rec, err := readJournalRecord(b)
@@ -258,13 +251,14 @@ func TestJournalWriterSyncClose(t *testing.T) {
 	j, err := createJournalWriter(ctx, newTestFilePath(t))
 	require.NotNil(t, j)
 	require.NoError(t, err)
-	_, err = j.ProcessJournal(ctx)
+	_, err = j.bootstrapJournal(ctx)
 	require.NoError(t, err)
 
 	// close triggers flush
-	n, err := j.Write([]byte("sit"))
+	p := []byte("sit")
+	buf, err := j.getBytes(len(p))
 	require.NoError(t, err)
-	assert.Equal(t, 3, n)
+	copy(buf, p)
 	err = j.Close()
 	require.NoError(t, err)
 	assert.Equal(t, 0, len(j.buf))

From 3d4c0e3b82ebc91497193ab94855c83da55b8575 Mon Sep 17 00:00:00 2001
From: Stephanie You <stephanie@dolthub.com>
Date: Fri, 17 Feb 2023 13:59:06 -0800
Subject: [PATCH 37/40] dolt_diff respects qualified database

---
 go/libraries/doltcore/sqle/database.go           |  2 +-
 .../doltcore/sqle/dtables/unscoped_diff_table.go |  9 +++++----
 integration-tests/bats/sql.bats                  | 16 ++++++++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index a3491b5a26..22a3993fb1 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -425,7 +425,7 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds
 			}
 		}
 
-		dt, found = dtables.NewUnscopedDiffTable(ctx, db.ddb, head), true
+		dt, found = dtables.NewUnscopedDiffTable(ctx, db.ddb, db.name, head), true
 	case doltdb.TableOfTablesInConflictName:
 		dt, found = dtables.NewTableOfTablesInConflict(ctx, db.name, db.ddb), true
 	case doltdb.TableOfTablesWithViolationsName:
diff --git a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
index a81a719c80..bd88bcc3d4 100644
--- a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
+++ b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
@@ -46,6 +46,7 @@ var _ sql.FilteredTable = (*UnscopedDiffTable)(nil)
 // changed in each commit, across all branches.
 type UnscopedDiffTable struct {
 	ddb              *doltdb.DoltDB
+	dbName           string
 	head             *doltdb.Commit
 	partitionFilters []sql.Expression
 	commitCheck      doltdb.CommitFilter
@@ -60,8 +61,8 @@ type tableChange struct {
 }
 
 // NewUnscopedDiffTable creates an UnscopedDiffTable
-func NewUnscopedDiffTable(_ *sql.Context, ddb *doltdb.DoltDB, head *doltdb.Commit) sql.Table {
-	return &UnscopedDiffTable{ddb: ddb, head: head}
+func NewUnscopedDiffTable(_ *sql.Context, ddb *doltdb.DoltDB, dbName string, head *doltdb.Commit) sql.Table {
+	return &UnscopedDiffTable{ddb: ddb, dbName: dbName, head: head}
 }
 
 // Filters returns the list of filters that are applied to this table.
@@ -192,9 +193,9 @@ func (dt *UnscopedDiffTable) LookupPartitions(ctx *sql.Context, lookup sql.Index
 
 func (dt *UnscopedDiffTable) newWorkingSetRowItr(ctx *sql.Context) (sql.RowIter, error) {
 	sess := dsess.DSessFromSess(ctx.Session)
-	roots, ok := sess.GetRoots(ctx, ctx.GetCurrentDatabase())
+	roots, ok := sess.GetRoots(ctx, dt.dbName)
 	if !ok {
-		return nil, fmt.Errorf("unable to lookup roots for database %s", ctx.GetCurrentDatabase())
+		return nil, fmt.Errorf("unable to lookup roots for database %s", dt.dbName)
 	}
 
 	staged, unstaged, err := diff.GetStagedUnstagedTableDeltas(ctx, roots)
diff --git a/integration-tests/bats/sql.bats b/integration-tests/bats/sql.bats
index 9b5ba865e1..1eb0e25d7f 100755
--- a/integration-tests/bats/sql.bats
+++ b/integration-tests/bats/sql.bats
@@ -2581,6 +2581,22 @@ SQL
     [[ "$output" =~ "3" ]] || false
 }
 
+@test "sql: dolt diff table respects qualified database" {
+    dolt sql -q "CREATE DATABASE db01; CREATE DATABASE db02;"
+    dolt sql -q "USE db01; CREATE TABLE t01(pk int primary key);"
+    run dolt sql -q "USE db01; SELECT * FROM dolt_diff;"
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "t01" ]] || false
+
+    run dolt sql -q "USE db02; SELECT * FROM dolt_diff;"
+    [ "$status" -eq 0 ]
+    ! [[ "$output" =~ "t01" ]] || false
+
+    run dolt sql -q "USE db02; SELECT * FROM db01.dolt_diff;"
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "t01" ]] || false
+}
+
 @test "sql: sql print on order by returns the correct result" {
     dolt sql -q "CREATE TABLE mytable(pk int primary key);"
     dolt sql -q "INSERT INTO mytable VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10),(11),(12),(13),(14),(15),(16),(17),(18),(19),(20)"

From ec9d0c8516a0fd65314bcdfb41c4631b0de6cccb Mon Sep 17 00:00:00 2001
From: reltuk <aaron@dolthub.com>
Date: Fri, 17 Feb 2023 22:56:57 +0000
Subject: [PATCH 38/40] [ga-bump-dep] Bump dependency in Dolt by reltuk

---
 go/go.mod | 4 ++--
 go/go.sum | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/go/go.mod b/go/go.mod
index ee67951029..0e22228018 100644
--- a/go/go.mod
+++ b/go/go.mod
@@ -15,7 +15,7 @@ require (
 	github.com/dolthub/fslock v0.0.3
 	github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488
 	github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
-	github.com/dolthub/vitess v0.0.0-20230210003150-3065f526d869
+	github.com/dolthub/vitess v0.0.0-20230216234925-189ffe819e56
 	github.com/dustin/go-humanize v1.0.0
 	github.com/fatih/color v1.13.0
 	github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568
@@ -58,7 +58,7 @@ require (
 	github.com/cenkalti/backoff/v4 v4.1.3
 	github.com/cespare/xxhash v1.1.0
 	github.com/creasty/defaults v1.6.0
-	github.com/dolthub/go-mysql-server v0.14.1-0.20230215204931-638b126c00f4
+	github.com/dolthub/go-mysql-server v0.14.1-0.20230217225532-09205e0f234f
 	github.com/google/flatbuffers v2.0.6+incompatible
 	github.com/jmoiron/sqlx v1.3.4
 	github.com/kch42/buzhash v0.0.0-20160816060738-9bdec3dec7c6
diff --git a/go/go.sum b/go/go.sum
index daf29e6b97..d63f9b36e7 100644
--- a/go/go.sum
+++ b/go/go.sum
@@ -166,16 +166,16 @@ github.com/dolthub/flatbuffers v1.13.0-dh.1 h1:OWJdaPep22N52O/0xsUevxJ6Qfw1M2txC
 github.com/dolthub/flatbuffers v1.13.0-dh.1/go.mod h1:CorYGaDmXjHz1Z7i50PYXG1Ricn31GcA2wNOTFIQAKE=
 github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
 github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
-github.com/dolthub/go-mysql-server v0.14.1-0.20230215204931-638b126c00f4 h1:5dfFSWThxmxyz726NI+2Rk+qXR9ughIge8apz+Y7VNI=
-github.com/dolthub/go-mysql-server v0.14.1-0.20230215204931-638b126c00f4/go.mod h1:3PGGtLcVPnJumgozqqAKZPae88QmvkOd1KGS+Z2/RXU=
+github.com/dolthub/go-mysql-server v0.14.1-0.20230217225532-09205e0f234f h1:yuOrpt0Gwf8aYe7SmimteAjt/eNyUvBeaNRCq0RCMfA=
+github.com/dolthub/go-mysql-server v0.14.1-0.20230217225532-09205e0f234f/go.mod h1:BRFyf6PUuoR+iSLZ+JdpjtqgHzo5cT+tF7oHIpVdytY=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto=
 github.com/dolthub/jsonpath v0.0.0-20210609232853-d49537a30474 h1:xTrR+l5l+1Lfq0NvhiEsctylXinUMFhhsqaEcl414p8=
 github.com/dolthub/jsonpath v0.0.0-20210609232853-d49537a30474/go.mod h1:kMz7uXOXq4qRriCEyZ/LUeTqraLJCjf0WVZcUi6TxUY=
 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE=
 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
-github.com/dolthub/vitess v0.0.0-20230210003150-3065f526d869 h1:RiSFAJqwBJmFbISgxWEdpljUak1uFtNCKG0zGT8xzA4=
-github.com/dolthub/vitess v0.0.0-20230210003150-3065f526d869/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
+github.com/dolthub/vitess v0.0.0-20230216234925-189ffe819e56 h1:dHuKfUwaDUe847BVN3Wo+4GUGUNdlhuUif4RWkvG3Go=
+github.com/dolthub/vitess v0.0.0-20230216234925-189ffe819e56/go.mod h1:oVFIBdqMFEkt4Xz2fzFJBNtzKhDEjwdCF0dzde39iKs=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=

From c7967673002b1312bff82df8b107456bc8bd6edc Mon Sep 17 00:00:00 2001
From: Stephanie You <stephanie@dolthub.com>
Date: Fri, 17 Feb 2023 16:31:04 -0800
Subject: [PATCH 39/40] edit param order on UnscopedDiffTable

---
 go/libraries/doltcore/sqle/database.go                    | 2 +-
 go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
index 22a3993fb1..f495bd9b0c 100644
--- a/go/libraries/doltcore/sqle/database.go
+++ b/go/libraries/doltcore/sqle/database.go
@@ -425,7 +425,7 @@ func (db Database) getTableInsensitive(ctx *sql.Context, head *doltdb.Commit, ds
 			}
 		}
 
-		dt, found = dtables.NewUnscopedDiffTable(ctx, db.ddb, db.name, head), true
+		dt, found = dtables.NewUnscopedDiffTable(ctx, db.name, db.ddb, head), true
 	case doltdb.TableOfTablesInConflictName:
 		dt, found = dtables.NewTableOfTablesInConflict(ctx, db.name, db.ddb), true
 	case doltdb.TableOfTablesWithViolationsName:
diff --git a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
index bd88bcc3d4..51fc87deeb 100644
--- a/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
+++ b/go/libraries/doltcore/sqle/dtables/unscoped_diff_table.go
@@ -45,8 +45,8 @@ var _ sql.FilteredTable = (*UnscopedDiffTable)(nil)
 // UnscopedDiffTable is a sql.Table implementation of a system table that shows which tables have
 // changed in each commit, across all branches.
 type UnscopedDiffTable struct {
-	ddb              *doltdb.DoltDB
 	dbName           string
+	ddb              *doltdb.DoltDB
 	head             *doltdb.Commit
 	partitionFilters []sql.Expression
 	commitCheck      doltdb.CommitFilter
@@ -61,8 +61,8 @@ type tableChange struct {
 }
 
 // NewUnscopedDiffTable creates an UnscopedDiffTable
-func NewUnscopedDiffTable(_ *sql.Context, ddb *doltdb.DoltDB, dbName string, head *doltdb.Commit) sql.Table {
-	return &UnscopedDiffTable{ddb: ddb, dbName: dbName, head: head}
+func NewUnscopedDiffTable(_ *sql.Context, dbName string, ddb *doltdb.DoltDB, head *doltdb.Commit) sql.Table {
+	return &UnscopedDiffTable{dbName: dbName, ddb: ddb, head: head}
 }
 
 // Filters returns the list of filters that are applied to this table.

From ec1bcab2f9d1b092ac9aad02bd9ac51df967805b Mon Sep 17 00:00:00 2001
From: Aaron Son <aaron@dolthub.com>
Date: Fri, 17 Feb 2023 16:38:52 -0800
Subject: [PATCH 40/40] go/store/nbs: aws_table_persister.go: Fix conjoin code
 to not error on a single short Read() by using io.ReadFull.

In #5307 we started returning a real io.Reader for chunkSource.reader(ctx).
That means the Read() calls can come back short if they are directly attached
to a net.TCPConn, for example. It is not an error for a Read() to come back
short. Use io.ReadFull to correctly express the intent in the conjoin code for
AWS remotes.

Fixes a bug where doltremoteapi fails commits sometimes.
---
 go/store/nbs/aws_table_persister.go      | 27 ++++++++++++++----------
 go/store/nbs/aws_table_persister_test.go |  2 +-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/go/store/nbs/aws_table_persister.go b/go/store/nbs/aws_table_persister.go
index 8f1cafd2d0..f27c85b10e 100644
--- a/go/store/nbs/aws_table_persister.go
+++ b/go/store/nbs/aws_table_persister.go
@@ -25,6 +25,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"net/url"
 	"sort"
@@ -392,9 +393,9 @@ func (s3p awsTablePersister) assembleTable(ctx context.Context, plan compactionP
 		readWg.Add(1)
 		go func(m manualPart) {
 			defer readWg.Done()
-			n, _ := m.srcR.Read(buff[m.dstStart:m.dstEnd])
-			if int64(n) < m.dstEnd-m.dstStart {
-				ae.SetIfError(errors.New("failed to read all the table data"))
+			err := m.run(ctx, buff)
+			if err != nil {
+				ae.SetIfError(fmt.Errorf("failed to read conjoin table data: %w", err))
 			}
 		}(man)
 	}
@@ -507,8 +508,17 @@ type copyPart struct {
 }
 
 type manualPart struct {
-	srcR             io.Reader
-	dstStart, dstEnd int64
+	src        chunkSource
+	start, end int64
+}
+
+func (mp manualPart) run(ctx context.Context, buff []byte) error {
+	reader, _, err := mp.src.reader(ctx)
+	if err != nil {
+		return err
+	}
+	_, err = io.ReadFull(reader, buff[mp.start:mp.end])
+	return err
 }
 
 // dividePlan assumes that plan.sources (which is of type chunkSourcesByDescendingDataSize) is correctly sorted by descending data size.
@@ -545,12 +555,7 @@ func dividePlan(ctx context.Context, plan compactionPlan, minPartSize, maxPartSi
 	var offset int64
 	for ; i < len(plan.sources.sws); i++ {
 		sws := plan.sources.sws[i]
-		rdr, _, err := sws.source.reader(ctx)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-
-		manuals = append(manuals, manualPart{rdr, offset, offset + int64(sws.dataLen)})
+		manuals = append(manuals, manualPart{sws.source, offset, offset + int64(sws.dataLen)})
 		offset += int64(sws.dataLen)
 		buffSize += sws.dataLen
 	}
diff --git a/go/store/nbs/aws_table_persister_test.go b/go/store/nbs/aws_table_persister_test.go
index 4eec8f821b..49df3f4ef8 100644
--- a/go/store/nbs/aws_table_persister_test.go
+++ b/go/store/nbs/aws_table_persister_test.go
@@ -304,7 +304,7 @@ func TestAWSTablePersisterDividePlan(t *testing.T) {
 	assert.Len(manuals, 1)
 	ti, err = tooSmall.index()
 	require.NoError(t, err)
-	assert.EqualValues(calcChunkRangeSize(ti), manuals[0].dstEnd-manuals[0].dstStart)
+	assert.EqualValues(calcChunkRangeSize(ti), manuals[0].end-manuals[0].start)
 }
 
 func TestAWSTablePersisterCalcPartSizes(t *testing.T) {