From b06faccd940db66c9be5e6ddcf3b41375c58625e Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 09:29:22 -0700 Subject: [PATCH 01/27] First pass at json diff writer --- go/cmd/dolt/commands/diff.go | 17 +++- .../table/typed/json/json_diff_writer.go | 79 +++++++++++++++++++ .../doltcore/table/typed/json/writer.go | 40 +++++----- 3 files changed, 115 insertions(+), 21 deletions(-) create mode 100755 go/libraries/doltcore/table/typed/json/json_diff_writer.go diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 32055331d8..9cc1d9f20e 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -23,6 +23,7 @@ import ( "strings" textdiff "github.com/andreyvit/diff" + "github.com/dolthub/dolt/go/libraries/doltcore/table/typed/json" "github.com/dolthub/go-mysql-server/sql" humanize "github.com/dustin/go-humanize" "github.com/fatih/color" @@ -60,6 +61,7 @@ const ( TabularDiffOutput diffOutput = 1 SQLDiffOutput diffOutput = 2 + JsonDiffOutput diffOutput = 3 DataFlag = "data" SchemaFlag = "schema" @@ -178,7 +180,7 @@ func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseE f, _ := apr.GetValue(FormatFlag) switch strings.ToLower(f) { - case "tabular", "sql", "": + case "tabular", "sql", "json", "": default: return errhand.BuildDError("invalid output format: %s", f).Build() } @@ -204,6 +206,8 @@ func parseDiffArgs(ctx context.Context, dEnv *env.DoltEnv, apr *argparser.ArgPar dArgs.diffOutput = TabularDiffOutput case "sql": dArgs.diffOutput = SQLDiffOutput + case "json": + dArgs.diffOutput = JsonDiffOutput } dArgs.limit, _ = apr.GetInt(limitParam) @@ -410,6 +414,8 @@ func diffSchemas(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDel if dArgs.diffOutput == TabularDiffOutput { return printShowCreateTableDiff(ctx, td) + } else if dArgs.diffOutput == JsonDiffOutput { + return nil } return sqlSchemaDiff(ctx, td, toSchemas) @@ -607,6 +613,15 @@ func diffRows(ctx context.Context, se *engine.SqlEngine, td diff.TableDelta, dAr targetSch = td.FromSch } diffWriter = sqlexport.NewSqlDiffWriter(tableName, targetSch, iohelp.NopWrCloser(cli.CliOut)) + case JsonDiffOutput: + targetSch := td.ToSch + if targetSch == nil { + targetSch = td.FromSch + } + diffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), targetSch) + if err != nil { + return nil + } } err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, diffWriter) diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go new file mode 100755 index 0000000000..c313f0ece4 --- /dev/null +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -0,0 +1,79 @@ +// Copyright 2022 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package json + +import ( + "context" + "fmt" + "io" + + "github.com/dolthub/dolt/go/libraries/doltcore/diff" + "github.com/dolthub/dolt/go/libraries/doltcore/schema" + "github.com/dolthub/dolt/go/store/types" + "github.com/dolthub/go-mysql-server/sql" +) + +type JsonDiffWriter struct { + wr *JSONWriter +} + +var _ diff.SqlRowDiffWriter = (*JsonDiffWriter)(nil) + +func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter, error) { + // leading diff type column with empty name + cols := outSch.GetAllCols() + newCols := schema.NewColCollection() + newCols.Append(schema.NewColumn("diff_type", 0, types.StringKind, false)) + newCols.Append(cols.GetColumns()...) + + writer, err := NewJSONWriter(wr, outSch) + if err != nil { + return nil, err + } + + return &JsonDiffWriter{ + wr: writer, + }, nil +} + +func (j *JsonDiffWriter) WriteRow( + ctx context.Context, + row sql.Row, + rowDiffType diff.ChangeType, + colDiffTypes []diff.ChangeType, +) error { + if len(row) != len(colDiffTypes) { + return fmt.Errorf("expected the same size for columns and diff types, got %d and %d", len(row), len(colDiffTypes)) + } + + diffMarker := "" + switch rowDiffType { + case diff.Removed: + diffMarker = "removed" + case diff.Added: + diffMarker = "added" + case diff.ModifiedOld: + diffMarker = "old_modified" + case diff.ModifiedNew: + diffMarker = "new_modified" + } + + newRow := append(sql.Row{diffMarker}, row...) + return j.wr.WriteSqlRow(ctx, newRow) +} + +func (j *JsonDiffWriter) Close(ctx context.Context) error { + return j.wr.Close(ctx) +} diff --git a/go/libraries/doltcore/table/typed/json/writer.go b/go/libraries/doltcore/table/typed/json/writer.go index 452d73d9d4..f8078dfd70 100644 --- a/go/libraries/doltcore/table/typed/json/writer.go +++ b/go/libraries/doltcore/table/typed/json/writer.go @@ -56,13 +56,13 @@ func NewJSONWriter(wr io.WriteCloser, outSch schema.Schema) (*JSONWriter, error) return &JSONWriter{closer: wr, bWr: bwr, sch: outSch}, nil } -func (jsonw *JSONWriter) GetSchema() schema.Schema { - return jsonw.sch +func (j *JSONWriter) GetSchema() schema.Schema { + return j.sch } // WriteRow will write a row to a table -func (jsonw *JSONWriter) WriteRow(ctx context.Context, r row.Row) error { - allCols := jsonw.sch.GetAllCols() +func (j *JSONWriter) WriteRow(ctx context.Context, r row.Row) error { + allCols := j.sch.GetAllCols() colValMap := make(map[string]interface{}, allCols.Size()) if err := allCols.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { val, ok := r.GetColVal(tag) @@ -108,25 +108,25 @@ func (jsonw *JSONWriter) WriteRow(ctx context.Context, r row.Row) error { return errors.New("marshaling did not work") } - if jsonw.rowsWritten != 0 { - _, err := jsonw.bWr.WriteRune(',') + if j.rowsWritten != 0 { + _, err := j.bWr.WriteRune(',') if err != nil { return err } } - newErr := iohelp.WriteAll(jsonw.bWr, data) + newErr := iohelp.WriteAll(j.bWr, data) if newErr != nil { return newErr } - jsonw.rowsWritten++ + j.rowsWritten++ return nil } -func (jsonw *JSONWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { - allCols := jsonw.sch.GetAllCols() +func (j *JSONWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { + allCols := j.sch.GetAllCols() colValMap := make(map[string]interface{}, allCols.Size()) if err := allCols.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { val := row[allCols.TagToIdx[tag]] @@ -172,35 +172,35 @@ func (jsonw *JSONWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { return errors.New("marshaling did not work") } - if jsonw.rowsWritten != 0 { - _, err := jsonw.bWr.WriteRune(',') + if j.rowsWritten != 0 { + _, err := j.bWr.WriteRune(',') if err != nil { return err } } - newErr := iohelp.WriteAll(jsonw.bWr, data) + newErr := iohelp.WriteAll(j.bWr, data) if newErr != nil { return newErr } - jsonw.rowsWritten++ + j.rowsWritten++ return nil } // Close should flush all writes, release resources being held -func (jsonw *JSONWriter) Close(ctx context.Context) error { - if jsonw.closer != nil { - err := iohelp.WriteAll(jsonw.bWr, []byte(jsonFooter)) +func (j *JSONWriter) Close(ctx context.Context) error { + if j.closer != nil { + err := iohelp.WriteAll(j.bWr, []byte(jsonFooter)) if err != nil { return err } - errFl := jsonw.bWr.Flush() - errCl := jsonw.closer.Close() - jsonw.closer = nil + errFl := j.bWr.Flush() + errCl := j.closer.Close() + j.closer = nil if errCl != nil { return errCl From 4e7f392dbdb3980750ba8a5d49df29324d201316 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 10:03:31 -0700 Subject: [PATCH 02/27] Bug fixes, new schema diff interface --- go/libraries/doltcore/diff/diff.go | 13 +++++++++- .../table/typed/json/json_diff_writer.go | 24 ++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/diff/diff.go b/go/libraries/doltcore/diff/diff.go index 4250cce468..01bbd98769 100755 --- a/go/libraries/doltcore/diff/diff.go +++ b/go/libraries/doltcore/diff/diff.go @@ -57,7 +57,7 @@ type RowDiffer interface { Close() error } -// SqlRowDiffWriter knows how to write diff rows to an arbitrary format and destination. +// SqlRowDiffWriter knows how to write diff rows for a table to an arbitrary format and destination. type SqlRowDiffWriter interface { // WriteRow writes the diff row given, of the diff type provided. colDiffTypes is guaranteed to be the same length as // the input row. @@ -67,5 +67,16 @@ type SqlRowDiffWriter interface { Close(ctx context.Context) error } +// SchemaDiffWriter knows how to write SQL DDL statements for a schema diff for a table to an arbitrary format and +// destination. +type SchemaDiffWriter interface { + // WriteSchemaDiff writes the schema diff given (a SQL statement) and returns any error. A single table may have + // many SQL statements for a single diff. + WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error + + // Close finalizes the work of this writer. + Close(ctx context.Context) error +} + // ColorFunc is a function that can color a format string type ColorFunc func(a ...interface{}) string diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index c313f0ece4..e36a6b3c47 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -35,10 +35,15 @@ func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter // leading diff type column with empty name cols := outSch.GetAllCols() newCols := schema.NewColCollection() - newCols.Append(schema.NewColumn("diff_type", 0, types.StringKind, false)) - newCols.Append(cols.GetColumns()...) + newCols = newCols.Append(schema.NewColumn("diff_type", 0, types.StringKind, false)) + newCols = newCols.AppendColl(cols) - writer, err := NewJSONWriter(wr, outSch) + newSchema, err := schema.SchemaFromCols(newCols) + if err != nil { + return nil, err + } + + writer, err := NewJSONWriter(wr, newSchema) if err != nil { return nil, err } @@ -77,3 +82,16 @@ func (j *JsonDiffWriter) WriteRow( func (j *JsonDiffWriter) Close(ctx context.Context) error { return j.wr.Close(ctx) } + type JsonSchemaDiffWriter struct { + wr io.WriteCloser + } + +var _ diff.SchemaDiffWriter = (*JsonSchemaDiffWriter)(nil) + +func (j JsonSchemaDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error { + return nil +} + +func (j JsonSchemaDiffWriter) Close(ctx context.Context) error { + return j.wr.Close() +} From b05235dae8bba092521392a60a806b3d19122d50 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 10:26:49 -0700 Subject: [PATCH 03/27] Fleshing out table diff writer interface and its needs, did some renaming of related interfaces --- go/cmd/dolt/commands/dump.go | 2 +- go/cmd/dolt/commands/indexcmds/cat.go | 2 +- go/cmd/dolt/commands/tblcmds/export.go | 2 +- go/libraries/doltcore/diff/diff.go | 14 ++---- go/libraries/doltcore/mvdata/data_loc.go | 2 +- go/libraries/doltcore/mvdata/data_loc_test.go | 2 +- go/libraries/doltcore/mvdata/file_data_loc.go | 2 +- go/libraries/doltcore/mvdata/pipeline.go | 4 +- .../doltcore/mvdata/stream_data_loc.go | 2 +- go/libraries/doltcore/table/inmem_table.go | 4 +- go/libraries/doltcore/table/io.go | 4 +- .../doltcore/table/pipeline/procfunc_help.go | 2 +- go/libraries/doltcore/table/table_writer.go | 10 ++-- .../table/typed/json/json_diff_writer.go | 15 ++++-- .../doltcore/table/typed/json/writer.go | 49 +++++++++++++------ .../doltcore/table/typed/parquet/writer.go | 2 +- .../doltcore/table/untyped/csv/writer.go | 2 +- .../untyped/tabular/fixedwidth_tablewriter.go | 2 +- 18 files changed, 72 insertions(+), 50 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index 483ae75d2a..d8c9ed4e89 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -265,7 +265,7 @@ func dumpTable(ctx context.Context, dEnv *env.DoltEnv, tblOpts *tableOptions, fi return nil } -func getTableWriter(ctx context.Context, dEnv *env.DoltEnv, tblOpts *tableOptions, outSch schema.Schema, filePath string) (table.SqlTableWriter, errhand.VerboseError) { +func getTableWriter(ctx context.Context, dEnv *env.DoltEnv, tblOpts *tableOptions, outSch schema.Schema, filePath string) (table.SqlRowWriter, errhand.VerboseError) { opts := editor.Options{Deaf: dEnv.DbEaFactory(), Tempdir: dEnv.TempTableFilesDir()} writer, err := dEnv.FS.OpenForWriteAppend(filePath, os.ModePerm) diff --git a/go/cmd/dolt/commands/indexcmds/cat.go b/go/cmd/dolt/commands/indexcmds/cat.go index 5aa6c55010..9c96d4f9d9 100644 --- a/go/cmd/dolt/commands/indexcmds/cat.go +++ b/go/cmd/dolt/commands/indexcmds/cat.go @@ -176,7 +176,7 @@ func (cmd CatCmd) prettyPrintResults(ctx context.Context, doltSch schema.Schema, return nil } -func getTableWriter(format resultFormat, sch schema.Schema) (wr table.SqlTableWriter, err error) { +func getTableWriter(format resultFormat, sch schema.Schema) (wr table.SqlRowWriter, err error) { s, err := sqlutil.FromDoltSchema("", sch) if err != nil { return nil, err diff --git a/go/cmd/dolt/commands/tblcmds/export.go b/go/cmd/dolt/commands/tblcmds/export.go index a910c182b3..66eaffaf33 100644 --- a/go/cmd/dolt/commands/tblcmds/export.go +++ b/go/cmd/dolt/commands/tblcmds/export.go @@ -220,7 +220,7 @@ func (cmd ExportCmd) Exec(ctx context.Context, commandStr string, args []string, return 0 } -func getTableWriter(ctx context.Context, root *doltdb.RootValue, dEnv *env.DoltEnv, rdSchema schema.Schema, exOpts *exportOptions) (table.SqlTableWriter, errhand.VerboseError) { +func getTableWriter(ctx context.Context, root *doltdb.RootValue, dEnv *env.DoltEnv, rdSchema schema.Schema, exOpts *exportOptions) (table.SqlRowWriter, errhand.VerboseError) { ow, err := exOpts.checkOverwrite(ctx, root, dEnv.FS) if err != nil { return nil, errhand.VerboseErrorFromError(err) diff --git a/go/libraries/doltcore/diff/diff.go b/go/libraries/doltcore/diff/diff.go index 01bbd98769..939aa5ff3c 100755 --- a/go/libraries/doltcore/diff/diff.go +++ b/go/libraries/doltcore/diff/diff.go @@ -67,16 +67,12 @@ type SqlRowDiffWriter interface { Close(ctx context.Context) error } -// SchemaDiffWriter knows how to write SQL DDL statements for a schema diff for a table to an arbitrary format and +// TableDiffWriter knows how to write SQL DDL statements for a schema diff for a table to an arbitrary format and // destination. -type SchemaDiffWriter interface { +type TableDiffWriter interface { + SqlRowDiffWriter + // WriteSchemaDiff writes the schema diff given (a SQL statement) and returns any error. A single table may have - // many SQL statements for a single diff. + // many SQL statements for a single diff. WriteSchemaDiff will be called before any row diffs via |WriteRow| WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error - - // Close finalizes the work of this writer. - Close(ctx context.Context) error } - -// ColorFunc is a function that can color a format string -type ColorFunc func(a ...interface{}) string diff --git a/go/libraries/doltcore/mvdata/data_loc.go b/go/libraries/doltcore/mvdata/data_loc.go index 6500a518c5..9e1ce37169 100644 --- a/go/libraries/doltcore/mvdata/data_loc.go +++ b/go/libraries/doltcore/mvdata/data_loc.go @@ -93,7 +93,7 @@ type DataLocation interface { // NewCreatingWriter will create a TableWriteCloser for a DataLocation that will create a new table, or overwrite // an existing table. - NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlTableWriter, error) + NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlRowWriter, error) } // NewDataLocation creates a DataLocation object from a path and a format string. If the path is the name of a table diff --git a/go/libraries/doltcore/mvdata/data_loc_test.go b/go/libraries/doltcore/mvdata/data_loc_test.go index 770b8024a0..f8639b1815 100644 --- a/go/libraries/doltcore/mvdata/data_loc_test.go +++ b/go/libraries/doltcore/mvdata/data_loc_test.go @@ -188,7 +188,7 @@ func TestCreateRdWr(t *testing.T) { }{ {NewDataLocation("file.csv", ""), reflect.TypeOf((*csv.CSVReader)(nil)).Elem(), reflect.TypeOf((*csv.CSVWriter)(nil)).Elem()}, {NewDataLocation("file.psv", ""), reflect.TypeOf((*csv.CSVReader)(nil)).Elem(), reflect.TypeOf((*csv.CSVWriter)(nil)).Elem()}, - {NewDataLocation("file.json", ""), reflect.TypeOf((*json.JSONReader)(nil)).Elem(), reflect.TypeOf((*json.JSONWriter)(nil)).Elem()}, + {NewDataLocation("file.json", ""), reflect.TypeOf((*json.JSONReader)(nil)).Elem(), reflect.TypeOf((*json.RowWriter)(nil)).Elem()}, //{NewDataLocation("file.nbf", ""), reflect.TypeOf((*nbf.NBFReader)(nil)).Elem(), reflect.TypeOf((*nbf.NBFWriter)(nil)).Elem()}, } diff --git a/go/libraries/doltcore/mvdata/file_data_loc.go b/go/libraries/doltcore/mvdata/file_data_loc.go index 6d9576855e..65316375d7 100644 --- a/go/libraries/doltcore/mvdata/file_data_loc.go +++ b/go/libraries/doltcore/mvdata/file_data_loc.go @@ -178,7 +178,7 @@ func (dl FileDataLocation) NewReader(ctx context.Context, root *doltdb.RootValue // NewCreatingWriter will create a TableWriteCloser for a DataLocation that will create a new table, or overwrite // an existing table. -func (dl FileDataLocation) NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlTableWriter, error) { +func (dl FileDataLocation) NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlRowWriter, error) { switch dl.Format { case CsvFile: return csv.NewCSVWriter(wr, outSch, csv.NewCSVInfo()) diff --git a/go/libraries/doltcore/mvdata/pipeline.go b/go/libraries/doltcore/mvdata/pipeline.go index 4bb64b9f2a..7b99187774 100644 --- a/go/libraries/doltcore/mvdata/pipeline.go +++ b/go/libraries/doltcore/mvdata/pipeline.go @@ -30,10 +30,10 @@ type DataMoverPipeline struct { g *errgroup.Group ctx context.Context rd table.SqlRowReader - wr table.SqlTableWriter + wr table.SqlRowWriter } -func NewDataMoverPipeline(ctx context.Context, rd table.SqlRowReader, wr table.SqlTableWriter) *DataMoverPipeline { +func NewDataMoverPipeline(ctx context.Context, rd table.SqlRowReader, wr table.SqlRowWriter) *DataMoverPipeline { g, ctx := errgroup.WithContext(ctx) return &DataMoverPipeline{ g: g, diff --git a/go/libraries/doltcore/mvdata/stream_data_loc.go b/go/libraries/doltcore/mvdata/stream_data_loc.go index 3573684482..86ac6def4a 100644 --- a/go/libraries/doltcore/mvdata/stream_data_loc.go +++ b/go/libraries/doltcore/mvdata/stream_data_loc.go @@ -73,7 +73,7 @@ func (dl StreamDataLocation) NewReader(ctx context.Context, root *doltdb.RootVal // NewCreatingWriter will create a TableWriteCloser for a DataLocation that will create a new table, or overwrite // an existing table. -func (dl StreamDataLocation) NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlTableWriter, error) { +func (dl StreamDataLocation) NewCreatingWriter(ctx context.Context, mvOpts DataMoverOptions, root *doltdb.RootValue, outSch schema.Schema, opts editor.Options, wr io.WriteCloser) (table.SqlRowWriter, error) { switch dl.Format { case CsvFile: return csv.NewCSVWriter(iohelp.NopWrCloser(dl.Writer), outSch, csv.NewCSVInfo()) diff --git a/go/libraries/doltcore/table/inmem_table.go b/go/libraries/doltcore/table/inmem_table.go index 42ff302aed..65382b29d6 100644 --- a/go/libraries/doltcore/table/inmem_table.go +++ b/go/libraries/doltcore/table/inmem_table.go @@ -163,12 +163,12 @@ func (rd *InMemTableReader) VerifySchema(outSch schema.Schema) (bool, error) { return schema.VerifyInSchema(rd.tt.sch, outSch) } -// InMemTableWriter is an implementation of a TableWriter for an InMemTable +// InMemTableWriter is an implementation of a RowWriter for an InMemTable type InMemTableWriter struct { tt *InMemTable } -// NewInMemTableWriter creates an instance of a TableWriter from an InMemTable +// NewInMemTableWriter creates an instance of a RowWriter from an InMemTable func NewInMemTableWriter(imt *InMemTable) *InMemTableWriter { return &InMemTableWriter{imt} } diff --git a/go/libraries/doltcore/table/io.go b/go/libraries/doltcore/table/io.go index 42c41eaa2e..032b75f4ed 100644 --- a/go/libraries/doltcore/table/io.go +++ b/go/libraries/doltcore/table/io.go @@ -42,14 +42,14 @@ func GetRow(ctx context.Context, tbl *doltdb.Table, sch schema.Schema, key types return } -// PipeRows will read a row from given TableReader and write it to the provided TableWriter. It will do this +// PipeRows will read a row from given TableReader and write it to the provided RowWriter. It will do this // for every row until the TableReader's ReadRow method returns io.EOF or encounters an error in either reading // or writing. The caller will need to handle closing the tables as necessary. If contOnBadRow is true, errors reading // or writing will be ignored and the pipe operation will continue. // // Returns a tuple: (number of rows written, number of errors ignored, error). In the case that err is non-nil, the // row counter fields in the tuple will be set to -1. -func PipeRows(ctx context.Context, rd TableReader, wr TableWriter, contOnBadRow bool) (int, int, error) { +func PipeRows(ctx context.Context, rd TableReader, wr RowWriter, contOnBadRow bool) (int, int, error) { var numBad, numGood int for { r, err := rd.ReadRow(ctx) diff --git a/go/libraries/doltcore/table/pipeline/procfunc_help.go b/go/libraries/doltcore/table/pipeline/procfunc_help.go index b5ae226c0a..83707577b5 100644 --- a/go/libraries/doltcore/table/pipeline/procfunc_help.go +++ b/go/libraries/doltcore/table/pipeline/procfunc_help.go @@ -135,7 +135,7 @@ func SourceFuncForRows(rows []row.Row) SourceFunc { } // ProcFuncForWriter adapts a standard TableWriter to work as an OutFunc for a pipeline -func ProcFuncForWriter(ctx context.Context, wr table.TableWriter) OutFunc { +func ProcFuncForWriter(ctx context.Context, wr table.RowWriter) OutFunc { return ProcFuncForSinkFunc(func(r row.Row, props ReadableMap) error { return wr.WriteRow(ctx, r) }) diff --git a/go/libraries/doltcore/table/table_writer.go b/go/libraries/doltcore/table/table_writer.go index 988c2b5c9d..b3b657cae3 100644 --- a/go/libraries/doltcore/table/table_writer.go +++ b/go/libraries/doltcore/table/table_writer.go @@ -22,19 +22,19 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/row" ) -// TableWriteCloser is an interface for writing rows to a table -type TableWriter interface { - // WriteRow will write a row to a table +// RowWriter knows how to write table rows to some destination +type RowWriter interface { + // WriteRow writes a row to the destination of this writer WriteRow(ctx context.Context, r row.Row) error } // TableWriteCloser is an interface for writing rows to a table, that can be closed type TableWriteCloser interface { - TableWriter + RowWriter TableCloser } -type SqlTableWriter interface { +type SqlRowWriter interface { TableWriteCloser WriteSqlRow(ctx context.Context, r sql.Row) error } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index e36a6b3c47..98f2676d5d 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -21,17 +21,21 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/schema" + "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/dolt/go/store/types" "github.com/dolthub/go-mysql-server/sql" ) type JsonDiffWriter struct { - wr *JSONWriter + wr *RowWriter } +const jsonTableHeader = `{"table":{name: "%s",` +const jsonTableFooter = `]}` + var _ diff.SqlRowDiffWriter = (*JsonDiffWriter)(nil) -func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter, error) { +func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema) (*JsonDiffWriter, error) { // leading diff type column with empty name cols := outSch.GetAllCols() newCols := schema.NewColCollection() @@ -43,7 +47,12 @@ func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter return nil, err } - writer, err := NewJSONWriter(wr, newSchema) + err = iohelp.WriteAll(wr, []byte(fmt.Sprintf(jsonTableHeader, tableName))) + if err != nil { + return nil, err + } + + writer, err := NewJSONWriterWithHeader(wr, newSchema, `"rows":[`, "]") if err != nil { return nil, err } diff --git a/go/libraries/doltcore/table/typed/json/writer.go b/go/libraries/doltcore/table/typed/json/writer.go index f8078dfd70..b4d72059a5 100644 --- a/go/libraries/doltcore/table/typed/json/writer.go +++ b/go/libraries/doltcore/table/typed/json/writer.go @@ -38,30 +38,47 @@ const jsonFooter = `]}` var WriteBufSize = 256 * 1024 var defaultString = sql.MustCreateStringWithDefaults(sqltypes.VarChar, 16383) -type JSONWriter struct { +type RowWriter struct { closer io.Closer + header string + footer string bWr *bufio.Writer sch schema.Schema rowsWritten int } -var _ table.SqlTableWriter = (*JSONWriter)(nil) +var _ table.SqlRowWriter = (*RowWriter)(nil) -func NewJSONWriter(wr io.WriteCloser, outSch schema.Schema) (*JSONWriter, error) { - bwr := bufio.NewWriterSize(wr, WriteBufSize) - err := iohelp.WriteAll(bwr, []byte(jsonHeader)) - if err != nil { - return nil, err - } - return &JSONWriter{closer: wr, bWr: bwr, sch: outSch}, nil +// NewJSONWriter returns a new writer that encodes rows as a single JSON object with a single key: "rows", which is a +// slice of all rows. To customize the output of the JSON object emitted, use |NewJSONWriterWithHeader| +func NewJSONWriter(wr io.WriteCloser, outSch schema.Schema) (*RowWriter, error) { + return NewJSONWriterWithHeader(wr, outSch, jsonHeader, jsonFooter) } -func (j *JSONWriter) GetSchema() schema.Schema { +func NewJSONWriterWithHeader(wr io.WriteCloser, outSch schema.Schema, header, footer string) (*RowWriter, error) { + bwr := bufio.NewWriterSize(wr, WriteBufSize) + return &RowWriter{ + closer: wr, + bWr: bwr, + sch: outSch, + header: header, + footer: footer, + }, nil +} + +func (j *RowWriter) GetSchema() schema.Schema { return j.sch } -// WriteRow will write a row to a table -func (j *JSONWriter) WriteRow(ctx context.Context, r row.Row) error { +// WriteRow encodes the row given into JSON format and writes it, returning any error +func (j *RowWriter) WriteRow(ctx context.Context, r row.Row) error { + if j.rowsWritten == 0 { + err := iohelp.WriteAll(j.bWr, []byte(j.header)) + if err != nil { + return err + } + } + allCols := j.sch.GetAllCols() colValMap := make(map[string]interface{}, allCols.Size()) if err := allCols.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { @@ -125,7 +142,7 @@ func (j *JSONWriter) WriteRow(ctx context.Context, r row.Row) error { return nil } -func (j *JSONWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { +func (j *RowWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { allCols := j.sch.GetAllCols() colValMap := make(map[string]interface{}, allCols.Size()) if err := allCols.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { @@ -190,9 +207,9 @@ func (j *JSONWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { } // Close should flush all writes, release resources being held -func (j *JSONWriter) Close(ctx context.Context) error { +func (j *RowWriter) Close(ctx context.Context) error { if j.closer != nil { - err := iohelp.WriteAll(j.bWr, []byte(jsonFooter)) + err := iohelp.WriteAll(j.bWr, []byte(j.footer)) if err != nil { return err @@ -208,8 +225,8 @@ func (j *JSONWriter) Close(ctx context.Context) error { return errFl } - return errors.New("already closed") + return errors.New("already closed") } func marshalToJson(valMap interface{}) ([]byte, error) { diff --git a/go/libraries/doltcore/table/typed/parquet/writer.go b/go/libraries/doltcore/table/typed/parquet/writer.go index a532e73b4a..c2393c80cc 100644 --- a/go/libraries/doltcore/table/typed/parquet/writer.go +++ b/go/libraries/doltcore/table/typed/parquet/writer.go @@ -37,7 +37,7 @@ type ParquetWriter struct { sch schema.Schema } -var _ table.SqlTableWriter = (*ParquetWriter)(nil) +var _ table.SqlRowWriter = (*ParquetWriter)(nil) var typeMap = map[typeinfo.Identifier]string{ typeinfo.DatetimeTypeIdentifier: "type=INT64, convertedtype=TIMESTAMP_MICROS", diff --git a/go/libraries/doltcore/table/untyped/csv/writer.go b/go/libraries/doltcore/table/untyped/csv/writer.go index 6ad5acbe8e..96f24b67b6 100644 --- a/go/libraries/doltcore/table/untyped/csv/writer.go +++ b/go/libraries/doltcore/table/untyped/csv/writer.go @@ -45,7 +45,7 @@ type CSVWriter struct { useCRLF bool // True to use \r\n as the line terminator } -var _ table.SqlTableWriter = (*CSVWriter)(nil) +var _ table.SqlRowWriter = (*CSVWriter)(nil) // NewCSVWriter writes rows to the given WriteCloser based on the Schema and CSVFileInfo provided func NewCSVWriter(wr io.WriteCloser, outSch schema.Schema, info *CSVFileInfo) (*CSVWriter, error) { diff --git a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go index 4224d39f8a..7bf141929e 100644 --- a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go +++ b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go @@ -59,7 +59,7 @@ type FixedWidthTableWriter struct { flushedSampleBuffer bool } -var _ table.SqlTableWriter = (*FixedWidthTableWriter)(nil) +var _ table.SqlRowWriter = (*FixedWidthTableWriter)(nil) type tableRow struct { columns []string From 4a5c568d086200aea46b773afd553292ca123e12 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 11:34:55 -0700 Subject: [PATCH 04/27] More renaming, consolidating interfaces --- go/cmd/dolt/commands/diff.go | 2 +- go/cmd/dolt/commands/schcmds/import.go | 2 +- .../doltcore/env/actions/infer_schema.go | 2 +- go/libraries/doltcore/merge/violations_fk.go | 4 +- go/libraries/doltcore/mvdata/data_mover.go | 4 +- .../doltcore/table/composite_table_reader.go | 4 +- .../table/composite_table_reader_test.go | 2 +- .../doltcore/table/editor/bulk_import_tea.go | 2 +- .../table/editor/index_edit_accumulator.go | 2 +- .../doltcore/table/inmem_table_test.go | 2 +- go/libraries/doltcore/table/io.go | 4 +- go/libraries/doltcore/table/keyless_reader.go | 2 +- .../doltcore/table/pipeline/procfunc_help.go | 2 +- go/libraries/doltcore/table/pk_reader.go | 2 +- .../doltcore/table/read_ahead_table_reader.go | 6 +-- go/libraries/doltcore/table/table_reader.go | 22 +++++------ go/libraries/doltcore/table/table_writer.go | 2 +- .../table/typed/json/json_diff_writer.go | 39 +++++++++++-------- 18 files changed, 55 insertions(+), 50 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 9cc1d9f20e..f7ad6cc20b 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -618,7 +618,7 @@ func diffRows(ctx context.Context, se *engine.SqlEngine, td diff.TableDelta, dAr if targetSch == nil { targetSch = td.FromSch } - diffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), targetSch) + diffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) if err != nil { return nil } diff --git a/go/cmd/dolt/commands/schcmds/import.go b/go/cmd/dolt/commands/schcmds/import.go index 5aa33531a8..088afab0e8 100644 --- a/go/cmd/dolt/commands/schcmds/import.go +++ b/go/cmd/dolt/commands/schcmds/import.go @@ -344,7 +344,7 @@ func inferSchemaFromFile(ctx context.Context, nbf *types.NomsBinFormat, impOpts impOpts.fileType = impOpts.fileType[1:] } - var rd table.TableReadCloser + var rd table.ReadCloser csvInfo := csv.NewCSVInfo().SetDelim(",") switch impOpts.fileType { diff --git a/go/libraries/doltcore/env/actions/infer_schema.go b/go/libraries/doltcore/env/actions/infer_schema.go index 6e05a419c4..8389611165 100644 --- a/go/libraries/doltcore/env/actions/infer_schema.go +++ b/go/libraries/doltcore/env/actions/infer_schema.go @@ -54,7 +54,7 @@ type InferenceArgs interface { } // InferColumnTypesFromTableReader will infer a data types from a table reader. -func InferColumnTypesFromTableReader(ctx context.Context, root *doltdb.RootValue, rd table.TableReadCloser, args InferenceArgs) (*schema.ColCollection, error) { +func InferColumnTypesFromTableReader(ctx context.Context, root *doltdb.RootValue, rd table.ReadCloser, args InferenceArgs) (*schema.ColCollection, error) { inferrer := newInferrer(rd.GetSchema(), args) var rowFailure *pipeline.TransformRowFailure diff --git a/go/libraries/doltcore/merge/violations_fk.go b/go/libraries/doltcore/merge/violations_fk.go index a147ff1523..91a28edb97 100644 --- a/go/libraries/doltcore/merge/violations_fk.go +++ b/go/libraries/doltcore/merge/violations_fk.go @@ -250,7 +250,7 @@ func nomsParentFkConstraintViolations( } shouldContinue, err := func() (bool, error) { - var mapIter table.TableReadCloser = noms.NewNomsRangeReader( + var mapIter table.ReadCloser = noms.NewNomsRangeReader( postParent.IndexSchema, durable.NomsMapFromIndex(postParent.IndexData), []*noms.ReadRange{{Start: postParentIndexPartialKey, Inclusive: true, Reverse: false, Check: noms.InRangeCheckPartial(postParentIndexPartialKey)}}) @@ -458,7 +458,7 @@ func childFkConstraintViolationsProcess( postChildCVMapEditor *types.MapEditor, vInfo types.JSON, ) (bool, error) { - var mapIter table.TableReadCloser = noms.NewNomsRangeReader( + var mapIter table.ReadCloser = noms.NewNomsRangeReader( postParent.IndexSchema, durable.NomsMapFromIndex(postParent.IndexData), []*noms.ReadRange{{Start: parentPartialKey, Inclusive: true, Reverse: false, Check: noms.InRangeCheckPartial(parentPartialKey)}}) diff --git a/go/libraries/doltcore/mvdata/data_mover.go b/go/libraries/doltcore/mvdata/data_mover.go index 7ed4fba915..a132e12f16 100644 --- a/go/libraries/doltcore/mvdata/data_mover.go +++ b/go/libraries/doltcore/mvdata/data_mover.go @@ -70,7 +70,7 @@ type DataMoverCloser interface { } type DataMover struct { - Rd table.TableReadCloser + Rd table.ReadCloser Transforms *pipeline.TransformCollection Wr table.TableWriteCloser ContOnErr bool @@ -121,7 +121,7 @@ func SchAndTableNameFromFile(ctx context.Context, path string, fs filesys.Readab } } -func InferSchema(ctx context.Context, root *doltdb.RootValue, rd table.TableReadCloser, tableName string, pks []string, args actions.InferenceArgs) (schema.Schema, error) { +func InferSchema(ctx context.Context, root *doltdb.RootValue, rd table.ReadCloser, tableName string, pks []string, args actions.InferenceArgs) (schema.Schema, error) { var err error infCols, err := actions.InferColumnTypesFromTableReader(ctx, root, rd, args) diff --git a/go/libraries/doltcore/table/composite_table_reader.go b/go/libraries/doltcore/table/composite_table_reader.go index 23065f4cea..4345e348ac 100644 --- a/go/libraries/doltcore/table/composite_table_reader.go +++ b/go/libraries/doltcore/table/composite_table_reader.go @@ -26,12 +26,12 @@ import ( // of multiple TableReader instances into a single set of results. type CompositeTableReader struct { sch schema.Schema - readers []TableReadCloser + readers []ReadCloser idx int } // NewCompositeTableReader creates a new CompositeTableReader instance from a slice of TableReadClosers. -func NewCompositeTableReader(readers []TableReadCloser) (*CompositeTableReader, error) { +func NewCompositeTableReader(readers []ReadCloser) (*CompositeTableReader, error) { if len(readers) == 0 { panic("nothing to iterate") } diff --git a/go/libraries/doltcore/table/composite_table_reader_test.go b/go/libraries/doltcore/table/composite_table_reader_test.go index f57123bd10..cae3643c83 100644 --- a/go/libraries/doltcore/table/composite_table_reader_test.go +++ b/go/libraries/doltcore/table/composite_table_reader_test.go @@ -42,7 +42,7 @@ func TestCompositeTableReader(t *testing.T) { sch, err := schema.SchemaFromCols(coll) require.NoError(t, err) - var readers []TableReadCloser + var readers []ReadCloser var expectedKeys []uint64 var expectedVals []int64 for i := 0; i < numReaders; i++ { diff --git a/go/libraries/doltcore/table/editor/bulk_import_tea.go b/go/libraries/doltcore/table/editor/bulk_import_tea.go index 811bba83fd..60c4fcc8ac 100644 --- a/go/libraries/doltcore/table/editor/bulk_import_tea.go +++ b/go/libraries/doltcore/table/editor/bulk_import_tea.go @@ -226,7 +226,7 @@ func (iea *BulkImportIEA) HasPartial(ctx context.Context, idxSch schema.Schema, var err error var matches []hashedTuple - var mapIter table.TableReadCloser = noms.NewNomsRangeReader(idxSch, iea.rowData, []*noms.ReadRange{ + var mapIter table.ReadCloser = noms.NewNomsRangeReader(idxSch, iea.rowData, []*noms.ReadRange{ {Start: partialKey, Inclusive: true, Reverse: false, Check: noms.InRangeCheckPartial(partialKey)}}) defer mapIter.Close(ctx) var r row.Row diff --git a/go/libraries/doltcore/table/editor/index_edit_accumulator.go b/go/libraries/doltcore/table/editor/index_edit_accumulator.go index aec1e17d93..efded4fb39 100644 --- a/go/libraries/doltcore/table/editor/index_edit_accumulator.go +++ b/go/libraries/doltcore/table/editor/index_edit_accumulator.go @@ -286,7 +286,7 @@ func (iea *indexEditAccumulatorImpl) HasPartial(ctx context.Context, idxSch sche var err error var matches []hashedTuple - var mapIter table.TableReadCloser = noms.NewNomsRangeReader(idxSch, iea.rowData, []*noms.ReadRange{ + var mapIter table.ReadCloser = noms.NewNomsRangeReader(idxSch, iea.rowData, []*noms.ReadRange{ {Start: partialKey, Inclusive: true, Reverse: false, Check: noms.InRangeCheckPartial(partialKey)}}) defer mapIter.Close(ctx) var r row.Row diff --git a/go/libraries/doltcore/table/inmem_table_test.go b/go/libraries/doltcore/table/inmem_table_test.go index 0f859d2a33..7903adc903 100644 --- a/go/libraries/doltcore/table/inmem_table_test.go +++ b/go/libraries/doltcore/table/inmem_table_test.go @@ -90,7 +90,7 @@ func TestInMemTable(t *testing.T) { }() func() { - var r TableReadCloser + var r ReadCloser r = NewInMemTableReader(imt) defer r.Close(context.Background()) diff --git a/go/libraries/doltcore/table/io.go b/go/libraries/doltcore/table/io.go index 032b75f4ed..a26d48f2bd 100644 --- a/go/libraries/doltcore/table/io.go +++ b/go/libraries/doltcore/table/io.go @@ -49,7 +49,7 @@ func GetRow(ctx context.Context, tbl *doltdb.Table, sch schema.Schema, key types // // Returns a tuple: (number of rows written, number of errors ignored, error). In the case that err is non-nil, the // row counter fields in the tuple will be set to -1. -func PipeRows(ctx context.Context, rd TableReader, wr RowWriter, contOnBadRow bool) (int, int, error) { +func PipeRows(ctx context.Context, rd Reader, wr RowWriter, contOnBadRow bool) (int, int, error) { var numBad, numGood int for { r, err := rd.ReadRow(ctx) @@ -82,7 +82,7 @@ func PipeRows(ctx context.Context, rd TableReader, wr RowWriter, contOnBadRow bo // ReadAllRows reads all rows from a TableReader and returns a slice containing those rows. Usually this is used // for testing, or with very small data sets. -func ReadAllRows(ctx context.Context, rd TableReader, contOnBadRow bool) ([]row.Row, int, error) { +func ReadAllRows(ctx context.Context, rd Reader, contOnBadRow bool) ([]row.Row, int, error) { var rows []row.Row var err error diff --git a/go/libraries/doltcore/table/keyless_reader.go b/go/libraries/doltcore/table/keyless_reader.go index 67105dd13e..0f94be3094 100644 --- a/go/libraries/doltcore/table/keyless_reader.go +++ b/go/libraries/doltcore/table/keyless_reader.go @@ -38,7 +38,7 @@ type keylessTableReader struct { } var _ SqlTableReader = &keylessTableReader{} -var _ TableReadCloser = &keylessTableReader{} +var _ ReadCloser = &keylessTableReader{} // GetSchema implements the TableReader interface. func (rdr *keylessTableReader) GetSchema() schema.Schema { diff --git a/go/libraries/doltcore/table/pipeline/procfunc_help.go b/go/libraries/doltcore/table/pipeline/procfunc_help.go index 83707577b5..56fe7d69a7 100644 --- a/go/libraries/doltcore/table/pipeline/procfunc_help.go +++ b/go/libraries/doltcore/table/pipeline/procfunc_help.go @@ -73,7 +73,7 @@ func ProcFuncForSourceFunc(sourceFunc SourceFunc) InFunc { } // ProcFuncForReader adapts a standard TableReader to work as an InFunc for a pipeline -func ProcFuncForReader(ctx context.Context, rd table.TableReader) InFunc { +func ProcFuncForReader(ctx context.Context, rd table.Reader) InFunc { return ProcFuncForSourceFunc(func() (row.Row, ImmutableProperties, error) { r, err := rd.ReadRow(ctx) diff --git a/go/libraries/doltcore/table/pk_reader.go b/go/libraries/doltcore/table/pk_reader.go index 8cb13817e4..9bdbd1eaf6 100644 --- a/go/libraries/doltcore/table/pk_reader.go +++ b/go/libraries/doltcore/table/pk_reader.go @@ -33,7 +33,7 @@ type pkTableReader struct { } var _ SqlTableReader = pkTableReader{} -var _ TableReadCloser = pkTableReader{} +var _ ReadCloser = pkTableReader{} // GetSchema implements the TableReader interface. func (rdr pkTableReader) GetSchema() schema.Schema { diff --git a/go/libraries/doltcore/table/read_ahead_table_reader.go b/go/libraries/doltcore/table/read_ahead_table_reader.go index 35f5967eac..125e5a94ed 100644 --- a/go/libraries/doltcore/table/read_ahead_table_reader.go +++ b/go/libraries/doltcore/table/read_ahead_table_reader.go @@ -22,17 +22,17 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/async" ) -var _ TableReadCloser = (*AsyncReadAheadTableReader)(nil) +var _ ReadCloser = (*AsyncReadAheadTableReader)(nil) // AsyncReadAheadTableReader is a TableReadCloser implementation that spins up a go routine to keep reading data into // a buffered channel so that it is ready when the caller wants it. type AsyncReadAheadTableReader struct { - backingReader TableReadCloser + backingReader ReadCloser reader *async.AsyncReader } // NewAsyncReadAheadTableReader creates a new AsyncReadAheadTableReader -func NewAsyncReadAheadTableReader(tr TableReadCloser, bufferSize int) *AsyncReadAheadTableReader { +func NewAsyncReadAheadTableReader(tr ReadCloser, bufferSize int) *AsyncReadAheadTableReader { read := func(ctx context.Context) (interface{}, error) { return tr.ReadRow(ctx) } diff --git a/go/libraries/doltcore/table/table_reader.go b/go/libraries/doltcore/table/table_reader.go index d3fb734009..412f2cc44e 100644 --- a/go/libraries/doltcore/table/table_reader.go +++ b/go/libraries/doltcore/table/table_reader.go @@ -24,8 +24,8 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/schema" ) -// TableReader is an interface for reading rows from a table -type TableReader interface { +// Reader is an interface for reading rows from a table +type Reader interface { // GetSchema gets the schema of the rows that this reader will return GetSchema() schema.Schema @@ -35,30 +35,30 @@ type TableReader interface { ReadRow(ctx context.Context) (row.Row, error) } -// TableCloser is an interface for a table stream that can be closed to release resources -type TableCloser interface { +// Closer is an interface for a writer that can be closed to release resources +type Closer interface { // Close should release resources being held Close(ctx context.Context) error } -// TableReadCloser is an interface for reading rows from a table, that can be closed. -type TableReadCloser interface { - TableReader - TableCloser +// ReadCloser is an interface for reading rows from a table, that can be closed. +type ReadCloser interface { + Reader + Closer } type SqlRowReader interface { - TableReadCloser + ReadCloser ReadSqlRow(ctx context.Context) (sql.Row, error) } -// SqlTableReader is a TableReader that can read rows as sql.Row. +// SqlTableReader is a Reader that can read rows as sql.Row. type SqlTableReader interface { // GetSchema gets the schema of the rows that this reader will return GetSchema() schema.Schema - // ReadRow reads a row from a table as go-mysql-server sql.Row. + // ReadSqlRow reads a row from a table as go-mysql-server sql.Row. ReadSqlRow(ctx context.Context) (sql.Row, error) } diff --git a/go/libraries/doltcore/table/table_writer.go b/go/libraries/doltcore/table/table_writer.go index b3b657cae3..9bdb049cfd 100644 --- a/go/libraries/doltcore/table/table_writer.go +++ b/go/libraries/doltcore/table/table_writer.go @@ -31,7 +31,7 @@ type RowWriter interface { // TableWriteCloser is an interface for writing rows to a table, that can be closed type TableWriteCloser interface { RowWriter - TableCloser + Closer } type SqlRowWriter interface { diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 98f2676d5d..578e201eac 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -27,13 +27,15 @@ import ( ) type JsonDiffWriter struct { - wr *RowWriter + rowWriter *RowWriter + wr io.WriteCloser + rowsWritten int } -const jsonTableHeader = `{"table":{name: "%s",` -const jsonTableFooter = `]}` +const jsonTableHeader = `{"tables":[{"table":{name: "%s","schema_diff":[` +const jsonTableFooter = `]}]}` -var _ diff.SqlRowDiffWriter = (*JsonDiffWriter)(nil) +var _ diff.TableDiffWriter = (*JsonDiffWriter)(nil) func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema) (*JsonDiffWriter, error) { // leading diff type column with empty name @@ -52,16 +54,22 @@ func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema return nil, err } + // TODO: no-op write closer here writer, err := NewJSONWriterWithHeader(wr, newSchema, `"rows":[`, "]") if err != nil { return nil, err } return &JsonDiffWriter{ - wr: writer, + rowWriter: writer, }, nil } +func (j *JsonDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error { + // TODO implement me + panic("implement me") +} + func (j *JsonDiffWriter) WriteRow( ctx context.Context, row sql.Row, @@ -85,22 +93,19 @@ func (j *JsonDiffWriter) WriteRow( } newRow := append(sql.Row{diffMarker}, row...) - return j.wr.WriteSqlRow(ctx, newRow) + return j.rowWriter.WriteSqlRow(ctx, newRow) } func (j *JsonDiffWriter) Close(ctx context.Context) error { - return j.wr.Close(ctx) -} - type JsonSchemaDiffWriter struct { - wr io.WriteCloser - } + err := j.rowWriter.Close(ctx) + if err != nil { + return err + } -var _ diff.SchemaDiffWriter = (*JsonSchemaDiffWriter)(nil) + err = iohelp.WriteAll(j.wr, []byte(jsonFooter)) + if err != nil { + return err + } -func (j JsonSchemaDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error { - return nil -} - -func (j JsonSchemaDiffWriter) Close(ctx context.Context) error { return j.wr.Close() } From 59ceef16b8bd37df59fb72f8cc072dc4bd67d424 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 11:48:18 -0700 Subject: [PATCH 05/27] Small bug fix --- .../doltcore/table/typed/json/json_diff_writer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 578e201eac..1d522ab1f8 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -32,8 +32,8 @@ type JsonDiffWriter struct { rowsWritten int } -const jsonTableHeader = `{"tables":[{"table":{name: "%s","schema_diff":[` -const jsonTableFooter = `]}]}` +const jsonTableHeader = `{"table":{name: "%s","schema_diff":[` +const jsonTableFooter = `]}` var _ diff.TableDiffWriter = (*JsonDiffWriter)(nil) @@ -54,14 +54,14 @@ func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema return nil, err } - // TODO: no-op write closer here - writer, err := NewJSONWriterWithHeader(wr, newSchema, `"rows":[`, "]") + writer, err := NewJSONWriterWithHeader(iohelp.NopWrCloser(wr), newSchema, `"rows":[`, "]") if err != nil { return nil, err } return &JsonDiffWriter{ rowWriter: writer, + wr: wr, }, nil } @@ -102,7 +102,7 @@ func (j *JsonDiffWriter) Close(ctx context.Context) error { return err } - err = iohelp.WriteAll(j.wr, []byte(jsonFooter)) + err = iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) if err != nil { return err } From c8edd4f381b9edfe65d6aacc063c3ab7380e0738 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 12:10:22 -0700 Subject: [PATCH 06/27] More refactoring --- go/cmd/dolt/commands/diff.go | 106 +++++++++++------- .../table/typed/json/json_diff_writer.go | 1 + 2 files changed, 65 insertions(+), 42 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index f7ad6cc20b..3096c60dad 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -336,7 +336,14 @@ func maybeResolve(ctx context.Context, dEnv *env.DoltEnv, spec string) (*doltdb. return root, true } -func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) (verr errhand.VerboseError) { +// diffWriter is an interface that lets us write diffs in a variety of output formats +type diffWriter interface { + io.Closer + // BeginTable is called when a new table is about to be written. + BeginTable(ctx context.Context) error +} + +func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { var err error tableDeltas, err := diff.GetTableDeltas(ctx, dArgs.fromRoot, dArgs.toRoot) @@ -352,52 +359,67 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) (ve sort.Slice(tableDeltas, func(i, j int) bool { return strings.Compare(tableDeltas[i].ToName, tableDeltas[j].ToName) < 0 }) + for _, td := range tableDeltas { - if !dArgs.tableSet.Contains(td.FromName) && !dArgs.tableSet.Contains(td.ToName) { - continue + verr := diffUserTable(ctx, td, engine, dArgs) + if verr != nil { + return verr + } + } + + return nil +} + +func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEngine, dArgs *diffArgs) errhand.VerboseError { + if !dArgs.tableSet.Contains(td.FromName) && !dArgs.tableSet.Contains(td.ToName) { + return nil + } + + tblName := td.ToName + fromTable := td.FromTable + toTable := td.ToTable + + if fromTable == nil && toTable == nil { + return errhand.BuildDError("error: both tables in tableDelta are nil").Build() + } + + if dArgs.diffOutput == TabularDiffOutput { + printTableDiffSummary(td) + } + + fromSch, toSch, err := td.GetSchemas(ctx) + if err != nil { + return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() + } + + if dArgs.diffParts&Summary != 0 { + numCols := fromSch.GetAllCols().Size() + verr := printDiffSummary(ctx, td, numCols) + if verr != nil { + return verr + } + } + + if dArgs.diffParts&SchemaOnlyDiff != 0 { + verr := diffSchemas(ctx, dArgs.toRoot, td, dArgs) + if verr != nil { + return verr + } + } + + if dArgs.diffParts&DataOnlyDiff != 0 { + if td.IsDrop() && dArgs.diffOutput == SQLDiffOutput { + return nil // don't output DELETE FROM statements after DROP TABLE + } else if td.IsAdd() { + fromSch = toSch } - tblName := td.ToName - fromTable := td.FromTable - toTable := td.ToTable - - if fromTable == nil && toTable == nil { - return errhand.BuildDError("error: both tables in tableDelta are nil").Build() - } - - if dArgs.diffOutput == TabularDiffOutput { - printTableDiffSummary(td) - } - - fromSch, toSch, err := td.GetSchemas(ctx) - if err != nil { - return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() - } - - if dArgs.diffParts&Summary != 0 { - numCols := fromSch.GetAllCols().Size() - verr = printDiffSummary(ctx, td, numCols) - } - - if dArgs.diffParts&SchemaOnlyDiff != 0 { - verr = diffSchemas(ctx, dArgs.toRoot, td, dArgs) - } - - if dArgs.diffParts&DataOnlyDiff != 0 { - if td.IsDrop() && dArgs.diffOutput == SQLDiffOutput { - continue // don't output DELETE FROM statements after DROP TABLE - } else if td.IsAdd() { - fromSch = toSch - } - - if !schema.ArePrimaryKeySetsDiffable(td.Format(), fromSch, toSch) { - cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", tblName) - continue - } - - verr = diffRows(ctx, engine, td, dArgs) + if !schema.ArePrimaryKeySetsDiffable(td.Format(), fromSch, toSch) { + cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", tblName) + return nil } + verr := diffRows(ctx, engine, td, dArgs) if verr != nil { return verr } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 1d522ab1f8..77742d44d6 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -30,6 +30,7 @@ type JsonDiffWriter struct { rowWriter *RowWriter wr io.WriteCloser rowsWritten int + schemaDiffsWritten int } const jsonTableHeader = `{"table":{name: "%s","schema_diff":[` From 3680149ca2f96c3ed1837408512fa9507a5efc30 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 14:00:45 -0700 Subject: [PATCH 07/27] Continuing to flesh out interface --- go/cmd/dolt/commands/diff.go | 81 ++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 3096c60dad..cc78617ff4 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -35,12 +35,10 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" - "github.com/dolthub/dolt/go/libraries/doltcore/row" "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/doltcore/sqle" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlfmt" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" - "github.com/dolthub/dolt/go/libraries/doltcore/table/pipeline" "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/sqlexport" "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/tabular" "github.com/dolthub/dolt/go/libraries/utils/argparser" @@ -72,12 +70,6 @@ const ( CachedFlag = "cached" ) -type DiffSink interface { - GetSchema() schema.Schema - ProcRowWithProps(r row.Row, props pipeline.ReadableMap) error - Close() error -} - var diffDocs = cli.CommandDocumentationContent{ ShortDesc: "Show changes between commits, commit and working tree, etc", LongDesc: ` @@ -338,9 +330,13 @@ func maybeResolve(ctx context.Context, dEnv *env.DoltEnv, spec string) (*doltdb. // diffWriter is an interface that lets us write diffs in a variety of output formats type diffWriter interface { - io.Closer - // BeginTable is called when a new table is about to be written. - BeginTable(ctx context.Context) error + // BeginTable is called when a new table is about to be written, before any schema or row diffs are written + BeginTable(ctx context.Context, td diff.TableDelta) error + // WriteSchemaDiff is called to write a schema diff for the table given (if requested by args) + WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error + // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are + // done being written. + RowWriter(ctx context.Context, td diff.TableDelta) diff.SqlRowDiffWriter } func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { @@ -360,8 +356,9 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return strings.Compare(tableDeltas[i].ToName, tableDeltas[j].ToName) < 0 }) + dw := createDiffWriter(ctx, engine, dArgs) for _, td := range tableDeltas { - verr := diffUserTable(ctx, td, engine, dArgs) + verr := diffUserTable(ctx, td, engine, dArgs, dw) if verr != nil { return verr } @@ -370,7 +367,17 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return nil } -func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEngine, dArgs *diffArgs) errhand.VerboseError { +func createDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) diffWriter { + return nil +} + +func diffUserTable( + ctx context.Context, + td diff.TableDelta, + engine *engine.SqlEngine, + dArgs *diffArgs, + dw diffWriter, +) errhand.VerboseError { if !dArgs.tableSet.Contains(td.FromName) && !dArgs.tableSet.Contains(td.ToName) { return nil } @@ -383,6 +390,12 @@ func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEn return errhand.BuildDError("error: both tables in tableDelta are nil").Build() } + err := dw.BeginTable(ctx, td) + if err != nil { + return nil + } + + // TODO: move into tabular writer BeginTable if dArgs.diffOutput == TabularDiffOutput { printTableDiffSummary(td) } @@ -401,7 +414,7 @@ func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEn } if dArgs.diffParts&SchemaOnlyDiff != 0 { - verr := diffSchemas(ctx, dArgs.toRoot, td, dArgs) + verr := diffSchemas(ctx, dArgs.toRoot, td, dArgs, dw) if verr != nil { return verr } @@ -419,7 +432,7 @@ func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEn return nil } - verr := diffRows(ctx, engine, td, dArgs) + verr := diffRows(ctx, engine, td, dArgs, dw) if verr != nil { return verr } @@ -428,12 +441,24 @@ func diffUserTable(ctx context.Context, td diff.TableDelta, engine *engine.SqlEn return nil } -func diffSchemas(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta, dArgs *diffArgs) errhand.VerboseError { +func diffSchemas( + ctx context.Context, + toRoot *doltdb.RootValue, + td diff.TableDelta, + dArgs *diffArgs, + dw diffWriter, +) errhand.VerboseError { toSchemas, err := toRoot.GetAllSchemas(ctx) if err != nil { return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() } + err = dw.WriteSchemaDiff(ctx, toRoot, td) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + + // TODO: remove if dArgs.diffOutput == TabularDiffOutput { return printShowCreateTableDiff(ctx, td) } else if dArgs.diffOutput == JsonDiffOutput { @@ -564,7 +589,13 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string return nil } -func diffRows(ctx context.Context, se *engine.SqlEngine, td diff.TableDelta, dArgs *diffArgs) errhand.VerboseError { +func diffRows( + ctx context.Context, + se *engine.SqlEngine, + td diff.TableDelta, + dArgs *diffArgs, + dw diffWriter, +) errhand.VerboseError { from, to := dArgs.fromRef, dArgs.toRef tableName := td.ToName @@ -624,29 +655,31 @@ func diffRows(ctx context.Context, se *engine.SqlEngine, td diff.TableDelta, dAr return nil } - var diffWriter diff.SqlRowDiffWriter + rowWriter := dw.RowWriter(ctx, td) + + // TODO: replace these switch dArgs.diffOutput { case TabularDiffOutput: // TODO: default sample size - diffWriter = tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100) + rowWriter = tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100) case SQLDiffOutput: targetSch := td.ToSch if targetSch == nil { targetSch = td.FromSch } - diffWriter = sqlexport.NewSqlDiffWriter(tableName, targetSch, iohelp.NopWrCloser(cli.CliOut)) + rowWriter = sqlexport.NewSqlDiffWriter(tableName, targetSch, iohelp.NopWrCloser(cli.CliOut)) case JsonDiffOutput: targetSch := td.ToSch if targetSch == nil { targetSch = td.FromSch } - diffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) + rowWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) if err != nil { return nil } } - err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, diffWriter) + err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } @@ -871,3 +904,7 @@ func pluralize(singular, plural string, n uint64) string { } return fmt.Sprintf("%s %s", humanize.Comma(int64(n)), noun) } + +type jsonDiffWriter struct { + +} From 3f60a3df4e37a313fbb02175b25bfe3d527e392f Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 14:33:14 -0700 Subject: [PATCH 08/27] Plugged tabular diff writer in --- go/cmd/dolt/commands/diff.go | 80 +++++++++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index cc78617ff4..f88aaa6926 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -336,7 +336,7 @@ type diffWriter interface { WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are // done being written. - RowWriter(ctx context.Context, td diff.TableDelta) diff.SqlRowDiffWriter + RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) } func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { @@ -356,7 +356,7 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return strings.Compare(tableDeltas[i].ToName, tableDeltas[j].ToName) < 0 }) - dw := createDiffWriter(ctx, engine, dArgs) + dw := newDiffWriter(ctx, engine, dArgs) for _, td := range tableDeltas { verr := diffUserTable(ctx, td, engine, dArgs, dw) if verr != nil { @@ -367,7 +367,15 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return nil } -func createDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) diffWriter { +func newDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) diffWriter { + switch args.diffOutput { + case TabularDiffOutput: + return tabularDiffWriter{} + case SQLDiffOutput: + case JsonDiffOutput: + default: + panic(fmt.Sprintf("unexpected diff output: %v", args.diffOutput)) + } return nil } @@ -395,11 +403,6 @@ func diffUserTable( return nil } - // TODO: move into tabular writer BeginTable - if dArgs.diffOutput == TabularDiffOutput { - printTableDiffSummary(td) - } - fromSch, toSch, err := td.GetSchemas(ctx) if err != nil { return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() @@ -460,7 +463,8 @@ func diffSchemas( // TODO: remove if dArgs.diffOutput == TabularDiffOutput { - return printShowCreateTableDiff(ctx, td) + // handled by writer + return nil } else if dArgs.diffOutput == JsonDiffOutput { return nil } @@ -655,13 +659,15 @@ func diffRows( return nil } - rowWriter := dw.RowWriter(ctx, td) + rowWriter, err := dw.RowWriter(ctx, td, unionSch) + if err != nil { + return errhand.VerboseErrorFromError(err) + } // TODO: replace these switch dArgs.diffOutput { case TabularDiffOutput: - // TODO: default sample size - rowWriter = tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100) + // handled by interface case SQLDiffOutput: targetSch := td.ToSch if targetSch == nil { @@ -905,6 +911,52 @@ func pluralize(singular, plural string, n uint64) string { return fmt.Sprintf("%s %s", humanize.Comma(int64(n)), noun) } -type jsonDiffWriter struct { - +type tabularDiffWriter struct {} + +func (t tabularDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + printTableDiffSummary(td) + return nil } + +func (t tabularDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + return printShowCreateTableDiff(ctx, td) +} + +func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + return tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100), nil +} + +type sqlDiffWriter struct {} + +func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + // TODO implement me + panic("implement me") +} + +func (s sqlDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + // TODO implement me + panic("implement me") +} + +func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + // TODO implement me + panic("implement me") +} + +type jsonDiffWriter struct {} + +func (j jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + // TODO implement me + panic("implement me") +} + +func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + // TODO implement me + panic("implement me") +} + +func (j jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + // TODO implement me + panic("implement me") +} + From 410a0cbfb1ee8b5253eac7a79e0d6ff9133c377a Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 14:52:32 -0700 Subject: [PATCH 09/27] nil implementations for other writer types --- go/cmd/dolt/commands/diff.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index f88aaa6926..d2741ae24b 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -372,7 +372,9 @@ func newDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffA case TabularDiffOutput: return tabularDiffWriter{} case SQLDiffOutput: + return sqlDiffWriter{} case JsonDiffOutput: + return jsonDiffWriter{} default: panic(fmt.Sprintf("unexpected diff output: %v", args.diffOutput)) } @@ -929,34 +931,28 @@ func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, un type sqlDiffWriter struct {} func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - // TODO implement me - panic("implement me") + return nil } func (s sqlDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - // TODO implement me - panic("implement me") + return nil } func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - // TODO implement me - panic("implement me") + return nil, nil } type jsonDiffWriter struct {} func (j jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - // TODO implement me - panic("implement me") + return nil } func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - // TODO implement me - panic("implement me") + return nil } func (j jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - // TODO implement me - panic("implement me") + return nil, nil } From e29e128d9a2c8e977c37269c22a4a6eb38c4bb59 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 23 Aug 2022 16:04:38 -0700 Subject: [PATCH 10/27] Factored out getting schema diffs for sql --- go/cmd/dolt/commands/diff.go | 109 +++++++++++++++++++++++++++-------- 1 file changed, 84 insertions(+), 25 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index d2741ae24b..e6ba3d3527 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -337,6 +337,7 @@ type diffWriter interface { // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are // done being written. RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) + // todo: close method } func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { @@ -468,10 +469,11 @@ func diffSchemas( // handled by writer return nil } else if dArgs.diffOutput == JsonDiffOutput { + // handled by writer return nil } - return sqlSchemaDiff(ctx, td, toSchemas) + return writeSqlSchemaDiff(ctx, td, toSchemas) } func printShowCreateTableDiff(ctx context.Context, td diff.TableDelta) errhand.VerboseError { @@ -508,31 +510,49 @@ func printShowCreateTableDiff(ctx context.Context, td diff.TableDelta) errhand.V return nil } -// TODO: this doesn't handle check constraints or triggers -func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string]schema.Schema) errhand.VerboseError { - fromSch, toSch, err := td.GetSchemas(ctx) +func writeSqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string]schema.Schema) errhand.VerboseError { + ddlStatements, err := sqlSchemaDiff(ctx, td, toSchemas) if err != nil { - return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() + return err } + for _, stmt := range ddlStatements { + cli.Println(stmt) + } + + return nil +} + +// sqlSchemaDiff returns a slice of DDL statements that will transform the schema in the from delta to the schema in +// the to delta. +// TODO: this doesn't handle constraints or triggers +// TODO: this should live in the diff package +func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string]schema.Schema) ([]string, errhand.VerboseError) { + fromSch, toSch, err := td.GetSchemas(ctx) + if err != nil { + return nil, errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() + } + + var ddlStatements []string + if td.IsDrop() { - cli.Println(sqlfmt.DropTableStmt(td.FromName)) + ddlStatements = append(ddlStatements, sqlfmt.DropTableStmt(td.FromName)) } else if td.IsAdd() { sqlDb := sqle.NewSingleTableDatabase(td.ToName, toSch, td.ToFks, td.ToFksParentSch) sqlCtx, engine, _ := sqle.PrepareCreateTableStmt(ctx, sqlDb) stmt, err := sqle.GetCreateTableStmt(sqlCtx, engine, td.ToName) if err != nil { - return errhand.VerboseErrorFromError(err) + return nil, errhand.VerboseErrorFromError(err) } - cli.Println(stmt) + ddlStatements = append(ddlStatements, stmt) } else { if td.FromName != td.ToName { - cli.Println(sqlfmt.RenameTableStmt(td.FromName, td.ToName)) + ddlStatements = append(ddlStatements, sqlfmt.RenameTableStmt(td.FromName, td.ToName)) } eq := schema.SchemasAreEqual(fromSch, toSch) if eq && !td.HasFKChanges() { - return nil + return ddlStatements, nil } colDiffs, unionTags := diff.DiffSchColumns(fromSch, toSch) @@ -541,25 +561,25 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string switch cd.DiffType { case diff.SchDiffNone: case diff.SchDiffAdded: - cli.Println(sqlfmt.AlterTableAddColStmt(td.ToName, sqlfmt.FmtCol(0, 0, 0, *cd.New))) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddColStmt(td.ToName, sqlfmt.FmtCol(0, 0, 0, *cd.New))) case diff.SchDiffRemoved: - cli.Println(sqlfmt.AlterTableDropColStmt(td.ToName, cd.Old.Name)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropColStmt(td.ToName, cd.Old.Name)) case diff.SchDiffModified: // Ignore any primary key set changes here if cd.Old.IsPartOfPK != cd.New.IsPartOfPK { continue } if cd.Old.Name != cd.New.Name { - cli.Println(sqlfmt.AlterTableRenameColStmt(td.ToName, cd.Old.Name, cd.New.Name)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableRenameColStmt(td.ToName, cd.Old.Name, cd.New.Name)) } } } // Print changes between a primary key set change. It contains an ALTER TABLE DROP and an ALTER TABLE ADD if !schema.ColCollsAreEqual(fromSch.GetPKCols(), toSch.GetPKCols()) { - cli.Println(sqlfmt.AlterTableDropPks(td.ToName)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropPks(td.ToName)) if toSch.GetPKCols().Size() > 0 { - cli.Println(sqlfmt.AlterTableAddPrimaryKeys(td.ToName, toSch.GetPKCols())) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddPrimaryKeys(td.ToName, toSch.GetPKCols())) } } @@ -567,12 +587,12 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string switch idxDiff.DiffType { case diff.SchDiffNone: case diff.SchDiffAdded: - cli.Println(sqlfmt.AlterTableAddIndexStmt(td.ToName, idxDiff.To)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddIndexStmt(td.ToName, idxDiff.To)) case diff.SchDiffRemoved: - cli.Println(sqlfmt.AlterTableDropIndexStmt(td.FromName, idxDiff.From)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropIndexStmt(td.FromName, idxDiff.From)) case diff.SchDiffModified: - cli.Println(sqlfmt.AlterTableDropIndexStmt(td.FromName, idxDiff.From)) - cli.Println(sqlfmt.AlterTableAddIndexStmt(td.ToName, idxDiff.To)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropIndexStmt(td.FromName, idxDiff.From)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddIndexStmt(td.ToName, idxDiff.To)) } } @@ -581,18 +601,19 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string case diff.SchDiffNone: case diff.SchDiffAdded: parentSch := toSchemas[fkDiff.To.ReferencedTableName] - cli.Println(sqlfmt.AlterTableAddForeignKeyStmt(fkDiff.To, toSch, parentSch)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddForeignKeyStmt(fkDiff.To, toSch, parentSch)) case diff.SchDiffRemoved: - cli.Println(sqlfmt.AlterTableDropForeignKeyStmt(fkDiff.From)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropForeignKeyStmt(fkDiff.From)) case diff.SchDiffModified: - cli.Println(sqlfmt.AlterTableDropForeignKeyStmt(fkDiff.From)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableDropForeignKeyStmt(fkDiff.From)) parentSch := toSchemas[fkDiff.To.ReferencedTableName] - cli.Println(sqlfmt.AlterTableAddForeignKeyStmt(fkDiff.To, toSch, parentSch)) + ddlStatements = append(ddlStatements, sqlfmt.AlterTableAddForeignKeyStmt(fkDiff.To, toSch, parentSch)) } } } - return nil + + return ddlStatements, nil } func diffRows( @@ -942,13 +963,51 @@ func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionS return nil, nil } -type jsonDiffWriter struct {} +type jsonDiffWriter struct { + wr io.WriteCloser + diffTableWriter diff.TableDiffWriter +} + +func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { + return &jsonDiffWriter{ + wr: wr, + }, nil +} func (j jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + if j.diffTableWriter == nil { + err := iohelp.WriteAll(j.wr, []byte(`{"tables":[`)) + if err != nil { + return err + } + } else { + err := iohelp.WriteAll(j.wr, []byte(`,`)) + if err != nil { + return err + } + } + return nil } func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + toSchemas, err := toRoot.GetAllSchemas(ctx) + if err != nil { + return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() + } + + stmts, err := sqlSchemaDiff(ctx, td, toSchemas) + if err != nil { + return err + } + + for _, stmt := range stmts { + err := j.diffTableWriter.WriteSchemaDiff(ctx, stmt) + if err != nil { + return err + } + } + return nil } From a07dceeb75219e4a74f47d26e77d03365bf4eedb Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 10:30:54 -0700 Subject: [PATCH 11/27] Moved start and close code for json writer around --- go/cmd/dolt/commands/diff.go | 88 +++++++++++++------ go/libraries/doltcore/diff/diff.go | 8 +- .../table/typed/json/json_diff_writer.go | 49 ++++++++--- 3 files changed, 105 insertions(+), 40 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index e6ba3d3527..04bf140a46 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -337,7 +337,8 @@ type diffWriter interface { // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are // done being written. RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) - // todo: close method + // Close finalizes the work of the writer + Close(ctx context.Context) error } func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { @@ -357,7 +358,11 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return strings.Compare(tableDeltas[i].ToName, tableDeltas[j].ToName) < 0 }) - dw := newDiffWriter(ctx, engine, dArgs) + dw, err := newDiffWriter(ctx, engine, dArgs) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + for _, td := range tableDeltas { verr := diffUserTable(ctx, td, engine, dArgs, dw) if verr != nil { @@ -365,23 +370,28 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err } } + err = dw.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil } -func newDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) diffWriter { +func newDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) (diffWriter, error) { switch args.diffOutput { case TabularDiffOutput: - return tabularDiffWriter{} + return tabularDiffWriter{}, nil case SQLDiffOutput: - return sqlDiffWriter{} + return sqlDiffWriter{}, nil case JsonDiffOutput: - return jsonDiffWriter{} + return newJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut)) default: panic(fmt.Sprintf("unexpected diff output: %v", args.diffOutput)) } - return nil } + func diffUserTable( ctx context.Context, td diff.TableDelta, @@ -698,14 +708,7 @@ func diffRows( } rowWriter = sqlexport.NewSqlDiffWriter(tableName, targetSch, iohelp.NopWrCloser(cli.CliOut)) case JsonDiffOutput: - targetSch := td.ToSch - if targetSch == nil { - targetSch = td.FromSch - } - rowWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) - if err != nil { - return nil - } + // handled by interface } err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter) @@ -936,6 +939,10 @@ func pluralize(singular, plural string, n uint64) string { type tabularDiffWriter struct {} +func (t tabularDiffWriter) Close(ctx context.Context) error { + return nil +} + func (t tabularDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { printTableDiffSummary(td) return nil @@ -951,6 +958,10 @@ func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, un type sqlDiffWriter struct {} +func (s sqlDiffWriter) Close(ctx context.Context) error { + return nil +} + func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { return nil } @@ -964,8 +975,9 @@ func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionS } type jsonDiffWriter struct { - wr io.WriteCloser - diffTableWriter diff.TableDiffWriter + wr io.WriteCloser + schemaDiffWriter diff.SchemaDiffWriter + rowDiffWriter diff.SqlRowDiffWriter } func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { @@ -974,23 +986,39 @@ func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { }, nil } -func (j jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - if j.diffTableWriter == nil { +const jsonTableHeader = `{"table":{name: "%s","schema_diff":` +const jsonTableFooter = `]}` + +func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + if j.schemaDiffWriter == nil { err := iohelp.WriteAll(j.wr, []byte(`{"tables":[`)) if err != nil { return err } } else { - err := iohelp.WriteAll(j.wr, []byte(`,`)) + err := j.rowDiffWriter.Close(ctx) + if err != nil { + return err + } + + err = iohelp.WriteAll(j.wr, []byte(`},`)) if err != nil { return err } } - return nil + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToTable))) + if err != nil { + return err + } + + + j.schemaDiffWriter, err = json.NewSchemaDiffWriter(iohelp.NopWrCloser(j.wr)) + + return err } -func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { +func (j *jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { toSchemas, err := toRoot.GetAllSchemas(ctx) if err != nil { return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() @@ -1002,7 +1030,7 @@ func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.Root } for _, stmt := range stmts { - err := j.diffTableWriter.WriteSchemaDiff(ctx, stmt) + err := j.schemaDiffWriter.WriteSchemaDiff(ctx, stmt) if err != nil { return err } @@ -1011,7 +1039,17 @@ func (j jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.Root return nil } -func (j jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - return nil, nil +func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + targetSch := td.ToSch + if targetSch == nil { + targetSch = td.FromSch + } + + var err error + j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) + return j.rowDiffWriter, err } +func (j *jsonDiffWriter) Close(ctx context.Context) error { + return iohelp.WriteAll(j.wr, []byte(`}`)) +} \ No newline at end of file diff --git a/go/libraries/doltcore/diff/diff.go b/go/libraries/doltcore/diff/diff.go index 939aa5ff3c..d00b9c49d1 100755 --- a/go/libraries/doltcore/diff/diff.go +++ b/go/libraries/doltcore/diff/diff.go @@ -67,12 +67,12 @@ type SqlRowDiffWriter interface { Close(ctx context.Context) error } -// TableDiffWriter knows how to write SQL DDL statements for a schema diff for a table to an arbitrary format and +// SchemaDiffWriter knows how to write SQL DDL statements for a schema diff for a table to an arbitrary format and // destination. -type TableDiffWriter interface { - SqlRowDiffWriter - +type SchemaDiffWriter interface { // WriteSchemaDiff writes the schema diff given (a SQL statement) and returns any error. A single table may have // many SQL statements for a single diff. WriteSchemaDiff will be called before any row diffs via |WriteRow| WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error + // Close finalizes the work of this writer. + Close(ctx context.Context) error } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 77742d44d6..1ed55232d1 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -30,13 +30,9 @@ type JsonDiffWriter struct { rowWriter *RowWriter wr io.WriteCloser rowsWritten int - schemaDiffsWritten int } -const jsonTableHeader = `{"table":{name: "%s","schema_diff":[` -const jsonTableFooter = `]}` - -var _ diff.TableDiffWriter = (*JsonDiffWriter)(nil) +var _ diff.SqlRowDiffWriter = (*JsonDiffWriter)(nil) func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema) (*JsonDiffWriter, error) { // leading diff type column with empty name @@ -50,11 +46,6 @@ func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema return nil, err } - err = iohelp.WriteAll(wr, []byte(fmt.Sprintf(jsonTableHeader, tableName))) - if err != nil { - return nil, err - } - writer, err := NewJSONWriterWithHeader(iohelp.NopWrCloser(wr), newSchema, `"rows":[`, "]") if err != nil { return nil, err @@ -103,7 +94,43 @@ func (j *JsonDiffWriter) Close(ctx context.Context) error { return err } - err = iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) + return j.wr.Close() +} + +type SchemaDiffWriter struct { + wr io.WriteCloser + schemaStmtsWritten int +} + +var _ diff.SchemaDiffWriter = (*SchemaDiffWriter)(nil) + +const jsonSchemaHeader = `[` +const jsonSchemaFooter = `]` + +func NewSchemaDiffWriter(wr io.WriteCloser) (*SchemaDiffWriter, error) { + err := iohelp.WriteAll(wr, []byte(jsonSchemaHeader)) + if err != nil { + return nil, err + } + + return &SchemaDiffWriter{ + wr: wr, + }, nil +} + +func (j *SchemaDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error { + if j.schemaStmtsWritten > 0 { + err := iohelp.WriteAll(j.wr, []byte(",")) + if err != nil { + return err + } + } + + return iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s"`, schemaDiffStatement))) +} + +func (j *SchemaDiffWriter) Close(ctx context.Context) error { + err := iohelp.WriteAll(j.wr, []byte(jsonSchemaFooter)) if err != nil { return err } From 88acb85148ff9b684d246edbaf5eeaeb150214b8 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 12:20:32 -0700 Subject: [PATCH 12/27] Bug fixes, first fully working version --- go/cmd/dolt/commands/diff.go | 22 +++++++++---------- .../table/typed/json/json_diff_writer.go | 12 +++++++++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 04bf140a46..2c2ec3d22c 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -413,7 +413,7 @@ func diffUserTable( err := dw.BeginTable(ctx, td) if err != nil { - return nil + return errhand.VerboseErrorFromError(err) } fromSch, toSch, err := td.GetSchemas(ctx) @@ -986,7 +986,7 @@ func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { }, nil } -const jsonTableHeader = `{"table":{name: "%s","schema_diff":` +const jsonTableHeader = `{"name":"%s","schema_diff":` const jsonTableFooter = `]}` func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { @@ -996,18 +996,13 @@ func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) err return err } } else { - err := j.rowDiffWriter.Close(ctx) - if err != nil { - return err - } - - err = iohelp.WriteAll(j.wr, []byte(`},`)) + err := iohelp.WriteAll(j.wr, []byte(`},`)) if err != nil { return err } } - err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToTable))) + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToName))) if err != nil { return err } @@ -1040,16 +1035,21 @@ func (j *jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.Roo } func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + // close off the schema diff block, start the data block + err := iohelp.WriteAll(j.wr, []byte(`],"data_diff":[`)) + if err != nil { + return nil, err + } + targetSch := td.ToSch if targetSch == nil { targetSch = td.FromSch } - var err error j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) return j.rowDiffWriter, err } func (j *jsonDiffWriter) Close(ctx context.Context) error { - return iohelp.WriteAll(j.wr, []byte(`}`)) + return iohelp.WriteAll(j.wr, []byte(`}]}`)) } \ No newline at end of file diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 1ed55232d1..743f19ad4b 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -16,6 +16,7 @@ package json import ( "context" + "encoding/json" "fmt" "io" @@ -126,7 +127,7 @@ func (j *SchemaDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatem } } - return iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s"`, schemaDiffStatement))) + return iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s"`, jsonEscape(schemaDiffStatement)))) } func (j *SchemaDiffWriter) Close(ctx context.Context) error { @@ -137,3 +138,12 @@ func (j *SchemaDiffWriter) Close(ctx context.Context) error { return j.wr.Close() } + +func jsonEscape(s string) string { + b, err := json.Marshal(s) + if err != nil { + panic(err) + } + // Trim the beginning and trailing " character + return string(b[1:len(b)-1]) +} \ No newline at end of file From 38d304c3c94a60906908fe27fcc30d8dcf839d71 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 13:33:09 -0700 Subject: [PATCH 13/27] Finished sql diff writer --- go/cmd/dolt/commands/diff.go | 68 ++++++++++-------------------------- 1 file changed, 18 insertions(+), 50 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 2c2ec3d22c..9e68514b4c 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -430,9 +430,9 @@ func diffUserTable( } if dArgs.diffParts&SchemaOnlyDiff != 0 { - verr := diffSchemas(ctx, dArgs.toRoot, td, dArgs, dw) - if verr != nil { - return verr + err := dw.WriteSchemaDiff(ctx, dArgs.toRoot, td) + if err != nil { + return errhand.VerboseErrorFromError(err) } } @@ -457,35 +457,6 @@ func diffUserTable( return nil } -func diffSchemas( - ctx context.Context, - toRoot *doltdb.RootValue, - td diff.TableDelta, - dArgs *diffArgs, - dw diffWriter, -) errhand.VerboseError { - toSchemas, err := toRoot.GetAllSchemas(ctx) - if err != nil { - return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() - } - - err = dw.WriteSchemaDiff(ctx, toRoot, td) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - - // TODO: remove - if dArgs.diffOutput == TabularDiffOutput { - // handled by writer - return nil - } else if dArgs.diffOutput == JsonDiffOutput { - // handled by writer - return nil - } - - return writeSqlSchemaDiff(ctx, td, toSchemas) -} - func printShowCreateTableDiff(ctx context.Context, td diff.TableDelta) errhand.VerboseError { fromSch, toSch, err := td.GetSchemas(ctx) if err != nil { @@ -688,6 +659,7 @@ func diffRows( if dArgs.diffOutput == SQLDiffOutput && (td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) { + // TODO: this is overly broad, we can absolutely do better _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff") return nil } @@ -697,20 +669,6 @@ func diffRows( return errhand.VerboseErrorFromError(err) } - // TODO: replace these - switch dArgs.diffOutput { - case TabularDiffOutput: - // handled by interface - case SQLDiffOutput: - targetSch := td.ToSch - if targetSch == nil { - targetSch = td.FromSch - } - rowWriter = sqlexport.NewSqlDiffWriter(tableName, targetSch, iohelp.NopWrCloser(cli.CliOut)) - case JsonDiffOutput: - // handled by interface - } - err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() @@ -967,11 +925,21 @@ func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error } func (s sqlDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - return nil + toSchemas, err := toRoot.GetAllSchemas(ctx) + if err != nil { + return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() + } + + return writeSqlSchemaDiff(ctx, td, toSchemas) } func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - return nil, nil + targetSch := td.ToSch + if targetSch == nil { + targetSch = td.FromSch + } + + return sqlexport.NewSqlDiffWriter(td.ToName, targetSch, iohelp.NopWrCloser(cli.CliOut)), nil } type jsonDiffWriter struct { @@ -987,7 +955,7 @@ func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { } const jsonTableHeader = `{"name":"%s","schema_diff":` -const jsonTableFooter = `]}` +const jsonTableFooter = `}]}` func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { if j.schemaDiffWriter == nil { @@ -1051,5 +1019,5 @@ func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unio } func (j *jsonDiffWriter) Close(ctx context.Context) error { - return iohelp.WriteAll(j.wr, []byte(`}]}`)) + return iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) } \ No newline at end of file From 116cbe2540ff31e04f1d7ff74487dc9d8cdbd3a6 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 13:43:18 -0700 Subject: [PATCH 14/27] Moved diff writers into their own file, cleaned up a little --- go/cmd/dolt/commands/diff.go | 387 ++-------------------------- go/cmd/dolt/commands/diff_output.go | 351 +++++++++++++++++++++++++ 2 files changed, 371 insertions(+), 367 deletions(-) create mode 100755 go/cmd/dolt/commands/diff_output.go diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 9e68514b4c..f749bcaa82 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -22,12 +22,6 @@ import ( "strconv" "strings" - textdiff "github.com/andreyvit/diff" - "github.com/dolthub/dolt/go/libraries/doltcore/table/typed/json" - "github.com/dolthub/go-mysql-server/sql" - humanize "github.com/dustin/go-humanize" - "github.com/fatih/color" - "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -39,12 +33,9 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/sqle" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlfmt" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" - "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/sqlexport" - "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/tabular" "github.com/dolthub/dolt/go/libraries/utils/argparser" - "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/dolt/go/libraries/utils/set" - "github.com/dolthub/dolt/go/store/atomicerr" + "github.com/dolthub/go-mysql-server/sql" ) type diffOutput int @@ -328,19 +319,6 @@ func maybeResolve(ctx context.Context, dEnv *env.DoltEnv, spec string) (*doltdb. return root, true } -// diffWriter is an interface that lets us write diffs in a variety of output formats -type diffWriter interface { - // BeginTable is called when a new table is about to be written, before any schema or row diffs are written - BeginTable(ctx context.Context, td diff.TableDelta) error - // WriteSchemaDiff is called to write a schema diff for the table given (if requested by args) - WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error - // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are - // done being written. - RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) - // Close finalizes the work of the writer - Close(ctx context.Context) error -} - func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) errhand.VerboseError { var err error @@ -358,7 +336,7 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return strings.Compare(tableDeltas[i].ToName, tableDeltas[j].ToName) < 0 }) - dw, err := newDiffWriter(ctx, engine, dArgs) + dw, err := newDiffWriter(dArgs.diffOutput) if err != nil { return errhand.VerboseErrorFromError(err) } @@ -378,20 +356,6 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err return nil } -func newDiffWriter(ctx context.Context, sqlEngine *engine.SqlEngine, args *diffArgs) (diffWriter, error) { - switch args.diffOutput { - case TabularDiffOutput: - return tabularDiffWriter{}, nil - case SQLDiffOutput: - return sqlDiffWriter{}, nil - case JsonDiffOutput: - return newJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut)) - default: - panic(fmt.Sprintf("unexpected diff output: %v", args.diffOutput)) - } -} - - func diffUserTable( ctx context.Context, td diff.TableDelta, @@ -423,10 +387,7 @@ func diffUserTable( if dArgs.diffParts&Summary != 0 { numCols := fromSch.GetAllCols().Size() - verr := printDiffSummary(ctx, td, numCols) - if verr != nil { - return verr - } + return printDiffSummary(ctx, td, numCols) } if dArgs.diffParts&SchemaOnlyDiff != 0 { @@ -457,40 +418,6 @@ func diffUserTable( return nil } -func printShowCreateTableDiff(ctx context.Context, td diff.TableDelta) errhand.VerboseError { - fromSch, toSch, err := td.GetSchemas(ctx) - if err != nil { - return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() - } - - var fromCreateStmt = "" - if td.FromTable != nil { - // TODO: use UserSpaceDatabase for these, no reason for this separate database implementation - sqlDb := sqle.NewSingleTableDatabase(td.FromName, fromSch, td.FromFks, td.FromFksParentSch) - sqlCtx, engine, _ := sqle.PrepareCreateTableStmt(ctx, sqlDb) - fromCreateStmt, err = sqle.GetCreateTableStmt(sqlCtx, engine, td.FromName) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - } - - var toCreateStmt = "" - if td.ToTable != nil { - sqlDb := sqle.NewSingleTableDatabase(td.ToName, toSch, td.ToFks, td.ToFksParentSch) - sqlCtx, engine, _ := sqle.PrepareCreateTableStmt(ctx, sqlDb) - toCreateStmt, err = sqle.GetCreateTableStmt(sqlCtx, engine, td.ToName) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - } - - if fromCreateStmt != toCreateStmt { - cli.Println(textdiff.LineDiff(fromCreateStmt, toCreateStmt)) - } - - return nil -} - func writeSqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string]schema.Schema) errhand.VerboseError { ddlStatements, err := sqlSchemaDiff(ctx, td, toSchemas) if err != nil { @@ -690,6 +617,23 @@ func unionSchemas(s1 sql.Schema, s2 sql.Schema) sql.Schema { return union } +func getColumnNamesString(fromSch, toSch schema.Schema) string { + var cols []string + if fromSch != nil { + fromSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { + cols = append(cols, fmt.Sprintf("`from_%s`", col.Name)) + return false, nil + }) + } + if toSch != nil { + toSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { + cols = append(cols, fmt.Sprintf("`to_%s`", col.Name)) + return false, nil + }) + } + return strings.Join(cols, ",") +} + func writeDiffResults( ctx *sql.Context, diffQuerySch sql.Schema, @@ -729,295 +673,4 @@ func writeDiffResults( } } } -} - -func getColumnNamesString(fromSch, toSch schema.Schema) string { - var cols []string - if fromSch != nil { - fromSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - cols = append(cols, fmt.Sprintf("`from_%s`", col.Name)) - return false, nil - }) - } - if toSch != nil { - toSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - cols = append(cols, fmt.Sprintf("`to_%s`", col.Name)) - return false, nil - }) - } - return strings.Join(cols, ",") -} - -func printDiffLines(bold *color.Color, lines []string) { - for _, line := range lines { - if string(line[0]) == string("+") { - cli.Println(color.GreenString("+ " + line[1:])) - } else if string(line[0]) == ("-") { - cli.Println(color.RedString("- " + line[1:])) - } else { - cli.Println(" " + line) - } - } -} - -func printTableDiffSummary(td diff.TableDelta) { - bold := color.New(color.Bold) - if td.IsDrop() { - _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.FromName, td.FromName) - _, _ = bold.Println("deleted table") - } else if td.IsAdd() { - _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.ToName, td.ToName) - _, _ = bold.Println("added table") - } else { - _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.FromName, td.ToName) - h1, err := td.FromTable.HashOf() - - if err != nil { - panic(err) - } - - _, _ = bold.Printf("--- a/%s @ %s\n", td.FromName, h1.String()) - - h2, err := td.ToTable.HashOf() - - if err != nil { - panic(err) - } - - _, _ = bold.Printf("+++ b/%s @ %s\n", td.ToName, h2.String()) - } -} - -func printDiffSummary(ctx context.Context, td diff.TableDelta, colLen int) errhand.VerboseError { - // todo: use errgroup.Group - ae := atomicerr.New() - ch := make(chan diff.DiffSummaryProgress) - go func() { - defer close(ch) - err := diff.SummaryForTableDelta(ctx, ch, td) - - ae.SetIfError(err) - }() - - acc := diff.DiffSummaryProgress{} - var count int64 - var pos int - eP := cli.NewEphemeralPrinter() - for p := range ch { - if ae.IsSet() { - break - } - - acc.Adds += p.Adds - acc.Removes += p.Removes - acc.Changes += p.Changes - acc.CellChanges += p.CellChanges - acc.NewSize += p.NewSize - acc.OldSize += p.OldSize - - if count%10000 == 0 { - eP.Printf("prev size: %d, new size: %d, adds: %d, deletes: %d, modifications: %d\n", acc.OldSize, acc.NewSize, acc.Adds, acc.Removes, acc.Changes) - eP.Display() - } - - count++ - } - - pos = cli.DeleteAndPrint(pos, "") - - if err := ae.Get(); err != nil { - return errhand.BuildDError("").AddCause(err).Build() - } - - keyless, err := td.IsKeyless(ctx) - if err != nil { - return nil - } - - if (acc.Adds + acc.Removes + acc.Changes) == 0 { - cli.Println("No data changes. See schema changes by using -s or --schema.") - return nil - } - - if keyless { - printKeylessSummary(acc) - } else { - printSummary(acc, colLen) - } - - return nil -} - -func printSummary(acc diff.DiffSummaryProgress, colLen int) { - rowsUnmodified := uint64(acc.OldSize - acc.Changes - acc.Removes) - unmodified := pluralize("Row Unmodified", "Rows Unmodified", rowsUnmodified) - insertions := pluralize("Row Added", "Rows Added", acc.Adds) - deletions := pluralize("Row Deleted", "Rows Deleted", acc.Removes) - changes := pluralize("Row Modified", "Rows Modified", acc.Changes) - cellChanges := pluralize("Cell Modified", "Cells Modified", acc.CellChanges) - - oldValues := pluralize("Entry", "Entries", acc.OldSize) - newValues := pluralize("Entry", "Entries", acc.NewSize) - - percentCellsChanged := float64(100*acc.CellChanges) / (float64(acc.OldSize) * float64(colLen)) - - safePercent := func(num, dom uint64) float64 { - // returns +Inf for x/0 where x > 0 - if num == 0 { - return float64(0) - } - return float64(100*num) / (float64(dom)) - } - - cli.Printf("%s (%.2f%%)\n", unmodified, safePercent(rowsUnmodified, acc.OldSize)) - cli.Printf("%s (%.2f%%)\n", insertions, safePercent(acc.Adds, acc.OldSize)) - cli.Printf("%s (%.2f%%)\n", deletions, safePercent(acc.Removes, acc.OldSize)) - cli.Printf("%s (%.2f%%)\n", changes, safePercent(acc.Changes, acc.OldSize)) - cli.Printf("%s (%.2f%%)\n", cellChanges, percentCellsChanged) - cli.Printf("(%s vs %s)\n\n", oldValues, newValues) -} - -func printKeylessSummary(acc diff.DiffSummaryProgress) { - insertions := pluralize("Row Added", "Rows Added", acc.Adds) - deletions := pluralize("Row Deleted", "Rows Deleted", acc.Removes) - - cli.Printf("%s\n", insertions) - cli.Printf("%s\n", deletions) -} - -func pluralize(singular, plural string, n uint64) string { - var noun string - if n != 1 { - noun = plural - } else { - noun = singular - } - return fmt.Sprintf("%s %s", humanize.Comma(int64(n)), noun) -} - -type tabularDiffWriter struct {} - -func (t tabularDiffWriter) Close(ctx context.Context) error { - return nil -} - -func (t tabularDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - printTableDiffSummary(td) - return nil -} - -func (t tabularDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - return printShowCreateTableDiff(ctx, td) -} - -func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - return tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100), nil -} - -type sqlDiffWriter struct {} - -func (s sqlDiffWriter) Close(ctx context.Context) error { - return nil -} - -func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - return nil -} - -func (s sqlDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - toSchemas, err := toRoot.GetAllSchemas(ctx) - if err != nil { - return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() - } - - return writeSqlSchemaDiff(ctx, td, toSchemas) -} - -func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - targetSch := td.ToSch - if targetSch == nil { - targetSch = td.FromSch - } - - return sqlexport.NewSqlDiffWriter(td.ToName, targetSch, iohelp.NopWrCloser(cli.CliOut)), nil -} - -type jsonDiffWriter struct { - wr io.WriteCloser - schemaDiffWriter diff.SchemaDiffWriter - rowDiffWriter diff.SqlRowDiffWriter -} - -func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { - return &jsonDiffWriter{ - wr: wr, - }, nil -} - -const jsonTableHeader = `{"name":"%s","schema_diff":` -const jsonTableFooter = `}]}` - -func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { - if j.schemaDiffWriter == nil { - err := iohelp.WriteAll(j.wr, []byte(`{"tables":[`)) - if err != nil { - return err - } - } else { - err := iohelp.WriteAll(j.wr, []byte(`},`)) - if err != nil { - return err - } - } - - err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToName))) - if err != nil { - return err - } - - - j.schemaDiffWriter, err = json.NewSchemaDiffWriter(iohelp.NopWrCloser(j.wr)) - - return err -} - -func (j *jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { - toSchemas, err := toRoot.GetAllSchemas(ctx) - if err != nil { - return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() - } - - stmts, err := sqlSchemaDiff(ctx, td, toSchemas) - if err != nil { - return err - } - - for _, stmt := range stmts { - err := j.schemaDiffWriter.WriteSchemaDiff(ctx, stmt) - if err != nil { - return err - } - } - - return nil -} - -func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { - // close off the schema diff block, start the data block - err := iohelp.WriteAll(j.wr, []byte(`],"data_diff":[`)) - if err != nil { - return nil, err - } - - targetSch := td.ToSch - if targetSch == nil { - targetSch = td.FromSch - } - - j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) - return j.rowDiffWriter, err -} - -func (j *jsonDiffWriter) Close(ctx context.Context) error { - return iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) } \ No newline at end of file diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go new file mode 100755 index 0000000000..348332f75e --- /dev/null +++ b/go/cmd/dolt/commands/diff_output.go @@ -0,0 +1,351 @@ +// Copyright 2022 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "context" + "fmt" + "io" + + textdiff "github.com/andreyvit/diff" + "github.com/dolthub/dolt/go/cmd/dolt/cli" + "github.com/dolthub/dolt/go/cmd/dolt/errhand" + "github.com/dolthub/dolt/go/libraries/doltcore/diff" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle" + "github.com/dolthub/dolt/go/libraries/doltcore/table/typed/json" + "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/sqlexport" + "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/tabular" + "github.com/dolthub/dolt/go/libraries/utils/iohelp" + "github.com/dolthub/dolt/go/store/atomicerr" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dustin/go-humanize" + "github.com/fatih/color" +) + +// diffWriter is an interface that lets us write diffs in a variety of output formats +type diffWriter interface { + // BeginTable is called when a new table is about to be written, before any schema or row diffs are written + BeginTable(ctx context.Context, td diff.TableDelta) error + // WriteSchemaDiff is called to write a schema diff for the table given (if requested by args) + WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error + // RowWriter returns a row writer for the table delta provided, which will have Close() called on it when rows are + // done being written. + RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) + // Close finalizes the work of the writer + Close(ctx context.Context) error +} + +// newDiffWriter returns a diffWriter for the output format given +func newDiffWriter(diffOutput diffOutput) (diffWriter, error) { + switch diffOutput { + case TabularDiffOutput: + return tabularDiffWriter{}, nil + case SQLDiffOutput: + return sqlDiffWriter{}, nil + case JsonDiffOutput: + return newJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut)) + default: + panic(fmt.Sprintf("unexpected diff output: %v", diffOutput)) + } +} + +func printDiffSummary(ctx context.Context, td diff.TableDelta, colLen int) errhand.VerboseError { + // todo: use errgroup.Group + ae := atomicerr.New() + ch := make(chan diff.DiffSummaryProgress) + go func() { + defer close(ch) + err := diff.SummaryForTableDelta(ctx, ch, td) + + ae.SetIfError(err) + }() + + acc := diff.DiffSummaryProgress{} + var count int64 + var pos int + eP := cli.NewEphemeralPrinter() + for p := range ch { + if ae.IsSet() { + break + } + + acc.Adds += p.Adds + acc.Removes += p.Removes + acc.Changes += p.Changes + acc.CellChanges += p.CellChanges + acc.NewSize += p.NewSize + acc.OldSize += p.OldSize + + if count%10000 == 0 { + eP.Printf("prev size: %d, new size: %d, adds: %d, deletes: %d, modifications: %d\n", acc.OldSize, acc.NewSize, acc.Adds, acc.Removes, acc.Changes) + eP.Display() + } + + count++ + } + + pos = cli.DeleteAndPrint(pos, "") + + if err := ae.Get(); err != nil { + return errhand.BuildDError("").AddCause(err).Build() + } + + keyless, err := td.IsKeyless(ctx) + if err != nil { + return nil + } + + if (acc.Adds + acc.Removes + acc.Changes) == 0 { + cli.Println("No data changes. See schema changes by using -s or --schema.") + return nil + } + + if keyless { + printKeylessSummary(acc) + } else { + printSummary(acc, colLen) + } + + return nil +} + +func printSummary(acc diff.DiffSummaryProgress, colLen int) { + rowsUnmodified := uint64(acc.OldSize - acc.Changes - acc.Removes) + unmodified := pluralize("Row Unmodified", "Rows Unmodified", rowsUnmodified) + insertions := pluralize("Row Added", "Rows Added", acc.Adds) + deletions := pluralize("Row Deleted", "Rows Deleted", acc.Removes) + changes := pluralize("Row Modified", "Rows Modified", acc.Changes) + cellChanges := pluralize("Cell Modified", "Cells Modified", acc.CellChanges) + + oldValues := pluralize("Entry", "Entries", acc.OldSize) + newValues := pluralize("Entry", "Entries", acc.NewSize) + + percentCellsChanged := float64(100*acc.CellChanges) / (float64(acc.OldSize) * float64(colLen)) + + safePercent := func(num, dom uint64) float64 { + // returns +Inf for x/0 where x > 0 + if num == 0 { + return float64(0) + } + return float64(100*num) / (float64(dom)) + } + + cli.Printf("%s (%.2f%%)\n", unmodified, safePercent(rowsUnmodified, acc.OldSize)) + cli.Printf("%s (%.2f%%)\n", insertions, safePercent(acc.Adds, acc.OldSize)) + cli.Printf("%s (%.2f%%)\n", deletions, safePercent(acc.Removes, acc.OldSize)) + cli.Printf("%s (%.2f%%)\n", changes, safePercent(acc.Changes, acc.OldSize)) + cli.Printf("%s (%.2f%%)\n", cellChanges, percentCellsChanged) + cli.Printf("(%s vs %s)\n\n", oldValues, newValues) +} + +func printKeylessSummary(acc diff.DiffSummaryProgress) { + insertions := pluralize("Row Added", "Rows Added", acc.Adds) + deletions := pluralize("Row Deleted", "Rows Deleted", acc.Removes) + + cli.Printf("%s\n", insertions) + cli.Printf("%s\n", deletions) +} + +func pluralize(singular, plural string, n uint64) string { + var noun string + if n != 1 { + noun = plural + } else { + noun = singular + } + return fmt.Sprintf("%s %s", humanize.Comma(int64(n)), noun) +} + +type tabularDiffWriter struct {} + +func (t tabularDiffWriter) Close(ctx context.Context) error { + return nil +} + +func (t tabularDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + bold := color.New(color.Bold) + if td.IsDrop() { + _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.FromName, td.FromName) + _, _ = bold.Println("deleted table") + } else if td.IsAdd() { + _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.ToName, td.ToName) + _, _ = bold.Println("added table") + } else { + _, _ = bold.Printf("diff --dolt a/%s b/%s\n", td.FromName, td.ToName) + h1, err := td.FromTable.HashOf() + + if err != nil { + panic(err) + } + + _, _ = bold.Printf("--- a/%s @ %s\n", td.FromName, h1.String()) + + h2, err := td.ToTable.HashOf() + + if err != nil { + panic(err) + } + + _, _ = bold.Printf("+++ b/%s @ %s\n", td.ToName, h2.String()) + } + return nil +} + +func (t tabularDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + fromSch, toSch, err := td.GetSchemas(ctx) + if err != nil { + return errhand.BuildDError("cannot retrieve schema for table %s", td.ToName).AddCause(err).Build() + } + + var fromCreateStmt = "" + if td.FromTable != nil { + // TODO: use UserSpaceDatabase for these, no reason for this separate database implementation + sqlDb := sqle.NewSingleTableDatabase(td.FromName, fromSch, td.FromFks, td.FromFksParentSch) + sqlCtx, engine, _ := sqle.PrepareCreateTableStmt(ctx, sqlDb) + fromCreateStmt, err = sqle.GetCreateTableStmt(sqlCtx, engine, td.FromName) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + } + + var toCreateStmt = "" + if td.ToTable != nil { + sqlDb := sqle.NewSingleTableDatabase(td.ToName, toSch, td.ToFks, td.ToFksParentSch) + sqlCtx, engine, _ := sqle.PrepareCreateTableStmt(ctx, sqlDb) + toCreateStmt, err = sqle.GetCreateTableStmt(sqlCtx, engine, td.ToName) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + } + + if fromCreateStmt != toCreateStmt { + cli.Println(textdiff.LineDiff(fromCreateStmt, toCreateStmt)) + } + + return nil +} + +func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + return tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100), nil +} + +type sqlDiffWriter struct {} + +func (s sqlDiffWriter) Close(ctx context.Context) error { + return nil +} + +func (s sqlDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + return nil +} + +func (s sqlDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + toSchemas, err := toRoot.GetAllSchemas(ctx) + if err != nil { + return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() + } + + return writeSqlSchemaDiff(ctx, td, toSchemas) +} + +func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + targetSch := td.ToSch + if targetSch == nil { + targetSch = td.FromSch + } + + return sqlexport.NewSqlDiffWriter(td.ToName, targetSch, iohelp.NopWrCloser(cli.CliOut)), nil +} + +type jsonDiffWriter struct { + wr io.WriteCloser + schemaDiffWriter diff.SchemaDiffWriter + rowDiffWriter diff.SqlRowDiffWriter +} + +func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { + return &jsonDiffWriter{ + wr: wr, + }, nil +} + +const jsonTableHeader = `{"name":"%s","schema_diff":` +const jsonTableFooter = `}]}` + +func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { + if j.schemaDiffWriter == nil { + err := iohelp.WriteAll(j.wr, []byte(`{"tables":[`)) + if err != nil { + return err + } + } else { + err := iohelp.WriteAll(j.wr, []byte(`},`)) + if err != nil { + return err + } + } + + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToName))) + if err != nil { + return err + } + + + j.schemaDiffWriter, err = json.NewSchemaDiffWriter(iohelp.NopWrCloser(j.wr)) + + return err +} + +func (j *jsonDiffWriter) WriteSchemaDiff(ctx context.Context, toRoot *doltdb.RootValue, td diff.TableDelta) error { + toSchemas, err := toRoot.GetAllSchemas(ctx) + if err != nil { + return errhand.BuildDError("could not read schemas from toRoot").AddCause(err).Build() + } + + stmts, err := sqlSchemaDiff(ctx, td, toSchemas) + if err != nil { + return err + } + + for _, stmt := range stmts { + err := j.schemaDiffWriter.WriteSchemaDiff(ctx, stmt) + if err != nil { + return err + } + } + + return nil +} + +func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionSch sql.Schema) (diff.SqlRowDiffWriter, error) { + // close off the schema diff block, start the data block + err := iohelp.WriteAll(j.wr, []byte(`],"data_diff":[`)) + if err != nil { + return nil, err + } + + targetSch := td.ToSch + if targetSch == nil { + targetSch = td.FromSch + } + + j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) + return j.rowDiffWriter, err +} + +func (j *jsonDiffWriter) Close(ctx context.Context) error { + return iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) +} From 162941542c67fb68f0df1fb519293100a29c78ff Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 15:56:15 -0700 Subject: [PATCH 15/27] Revamped diff to use from_row, to_row, fixed some bugs in that context --- go/cmd/dolt/commands/diff_output.go | 2 +- .../table/typed/json/json_diff_writer.go | 96 +++++++++++++------ .../doltcore/table/typed/json/writer.go | 23 +++-- 3 files changed, 87 insertions(+), 34 deletions(-) diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go index 348332f75e..fe4a6214b3 100755 --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -342,7 +342,7 @@ func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unio targetSch = td.FromSch } - j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), td.ToName, targetSch) + j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), targetSch) return j.rowDiffWriter, err } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 743f19ad4b..079167a68f 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -23,31 +23,20 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/utils/iohelp" - "github.com/dolthub/dolt/go/store/types" "github.com/dolthub/go-mysql-server/sql" ) type JsonDiffWriter struct { rowWriter *RowWriter wr io.WriteCloser + inModified bool rowsWritten int } var _ diff.SqlRowDiffWriter = (*JsonDiffWriter)(nil) -func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema) (*JsonDiffWriter, error) { - // leading diff type column with empty name - cols := outSch.GetAllCols() - newCols := schema.NewColCollection() - newCols = newCols.Append(schema.NewColumn("diff_type", 0, types.StringKind, false)) - newCols = newCols.AppendColl(cols) - - newSchema, err := schema.SchemaFromCols(newCols) - if err != nil { - return nil, err - } - - writer, err := NewJSONWriterWithHeader(iohelp.NopWrCloser(wr), newSchema, `"rows":[`, "]") +func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter, error) { + writer, err := NewJSONWriterWithHeader(iohelp.NopWrCloser(wr), outSch, "", "", "") if err != nil { return nil, err } @@ -58,11 +47,6 @@ func NewJsonDiffWriter(wr io.WriteCloser, tableName string, outSch schema.Schema }, nil } -func (j *JsonDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatement string) error { - // TODO implement me - panic("implement me") -} - func (j *JsonDiffWriter) WriteRow( ctx context.Context, row sql.Row, @@ -73,24 +57,82 @@ func (j *JsonDiffWriter) WriteRow( return fmt.Errorf("expected the same size for columns and diff types, got %d and %d", len(row), len(colDiffTypes)) } + prefix := "" + if j.inModified { + prefix = "," + } else if j.rowsWritten > 0 { + prefix = ",{" + } else { + prefix = "{" + } + + err := iohelp.WriteAll(j.wr, []byte(prefix)) + if err != nil { + return err + } + diffMarker := "" switch rowDiffType { case diff.Removed: - diffMarker = "removed" - case diff.Added: - diffMarker = "added" + diffMarker = "from_row" case diff.ModifiedOld: - diffMarker = "old_modified" + diffMarker = "from_row" + case diff.Added: + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s":{},`, "from_row"))) + if err != nil { + return err + } + diffMarker = "to_row" case diff.ModifiedNew: - diffMarker = "new_modified" + diffMarker = "to_row" } - newRow := append(sql.Row{diffMarker}, row...) - return j.rowWriter.WriteSqlRow(ctx, newRow) + err = iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s":`, diffMarker))) + if err != nil { + return err + } + + err = j.rowWriter.WriteSqlRow(ctx, row) + if err != nil { + return err + } + + // The row writer buffers its output and we share an underlying write stream with it, so we need to flush after + // every call to WriteSqlRow + err = j.rowWriter.Flush() + if err != nil { + return err + } + + switch rowDiffType { + case diff.ModifiedNew, diff.ModifiedOld: + j.inModified = !j.inModified + case diff.Added: + case diff.Removed: + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`,"%s":{}`, "to_row"))) + if err != nil { + return err + } + } + + if !j.inModified { + err := iohelp.WriteAll(j.wr, []byte("}")) + if err != nil { + return err + } + j.rowsWritten++ + } + + return nil } func (j *JsonDiffWriter) Close(ctx context.Context) error { - err := j.rowWriter.Close(ctx) + err := iohelp.WriteAll(j.wr, []byte("]")) + if err != nil { + return err + } + + err = j.rowWriter.Close(ctx) if err != nil { return err } diff --git a/go/libraries/doltcore/table/typed/json/writer.go b/go/libraries/doltcore/table/typed/json/writer.go index b4d72059a5..a36c503eec 100644 --- a/go/libraries/doltcore/table/typed/json/writer.go +++ b/go/libraries/doltcore/table/typed/json/writer.go @@ -42,6 +42,7 @@ type RowWriter struct { closer io.Closer header string footer string + separator string bWr *bufio.Writer sch schema.Schema rowsWritten int @@ -52,10 +53,10 @@ var _ table.SqlRowWriter = (*RowWriter)(nil) // NewJSONWriter returns a new writer that encodes rows as a single JSON object with a single key: "rows", which is a // slice of all rows. To customize the output of the JSON object emitted, use |NewJSONWriterWithHeader| func NewJSONWriter(wr io.WriteCloser, outSch schema.Schema) (*RowWriter, error) { - return NewJSONWriterWithHeader(wr, outSch, jsonHeader, jsonFooter) + return NewJSONWriterWithHeader(wr, outSch, jsonHeader, jsonFooter, ",") } -func NewJSONWriterWithHeader(wr io.WriteCloser, outSch schema.Schema, header, footer string) (*RowWriter, error) { +func NewJSONWriterWithHeader(wr io.WriteCloser, outSch schema.Schema, header, footer, separator string) (*RowWriter, error) { bwr := bufio.NewWriterSize(wr, WriteBufSize) return &RowWriter{ closer: wr, @@ -63,6 +64,7 @@ func NewJSONWriterWithHeader(wr io.WriteCloser, outSch schema.Schema, header, fo sch: outSch, header: header, footer: footer, + separator: separator, }, nil } @@ -126,8 +128,7 @@ func (j *RowWriter) WriteRow(ctx context.Context, r row.Row) error { } if j.rowsWritten != 0 { - _, err := j.bWr.WriteRune(',') - + _, err := j.bWr.WriteString(j.separator) if err != nil { return err } @@ -143,6 +144,13 @@ func (j *RowWriter) WriteRow(ctx context.Context, r row.Row) error { } func (j *RowWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { + if j.rowsWritten == 0 { + err := iohelp.WriteAll(j.bWr, []byte(j.header)) + if err != nil { + return err + } + } + allCols := j.sch.GetAllCols() colValMap := make(map[string]interface{}, allCols.Size()) if err := allCols.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { @@ -190,8 +198,7 @@ func (j *RowWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { } if j.rowsWritten != 0 { - _, err := j.bWr.WriteRune(',') - + _, err := j.bWr.WriteString(j.separator) if err != nil { return err } @@ -206,6 +213,10 @@ func (j *RowWriter) WriteSqlRow(ctx context.Context, row sql.Row) error { return nil } +func (j *RowWriter) Flush() error { + return j.bWr.Flush() +} + // Close should flush all writes, release resources being held func (j *RowWriter) Close(ctx context.Context) error { if j.closer != nil { From 96a3506721e8cedef888a03aa05b9fd39c4b427d Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 17:00:04 -0700 Subject: [PATCH 16/27] Bug fixes --- go/cmd/dolt/commands/diff_output.go | 31 ++++++++++++++----- .../table/typed/json/json_diff_writer.go | 4 ++- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go index fe4a6214b3..5813c84701 100755 --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -271,9 +271,11 @@ func (s sqlDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unionS } type jsonDiffWriter struct { - wr io.WriteCloser - schemaDiffWriter diff.SchemaDiffWriter - rowDiffWriter diff.SqlRowDiffWriter + wr io.WriteCloser + schemaDiffWriter diff.SchemaDiffWriter + rowDiffWriter diff.SqlRowDiffWriter + schemaDiffsWritten int + tablesWritten int } func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { @@ -282,8 +284,8 @@ func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { }, nil } -const jsonTableHeader = `{"name":"%s","schema_diff":` -const jsonTableFooter = `}]}` +const jsonDiffTableHeader = `{"name":"%s","schema_diff":` +const jsonDiffFooter = `}]}` func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) error { if j.schemaDiffWriter == nil { @@ -298,14 +300,14 @@ func (j *jsonDiffWriter) BeginTable(ctx context.Context, td diff.TableDelta) err } } - err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonTableHeader, td.ToName))) + err := iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(jsonDiffTableHeader, td.ToName))) if err != nil { return err } + j.tablesWritten++ j.schemaDiffWriter, err = json.NewSchemaDiffWriter(iohelp.NopWrCloser(j.wr)) - return err } @@ -347,5 +349,18 @@ func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unio } func (j *jsonDiffWriter) Close(ctx context.Context) error { - return iohelp.WriteAll(j.wr, []byte(jsonTableFooter)) + if j.tablesWritten > 0 { + err := iohelp.WriteLine(j.wr, jsonDiffFooter) + if err != nil { + return err + } + } else { + err := iohelp.WriteLine(j.wr, "") + if err != nil { + return err + } + } + + // Writer has already been closed here during row iteration, no need to close it here + return nil } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 079167a68f..72624af43d 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -141,7 +141,7 @@ func (j *JsonDiffWriter) Close(ctx context.Context) error { } type SchemaDiffWriter struct { - wr io.WriteCloser + wr io.WriteCloser schemaStmtsWritten int } @@ -169,6 +169,8 @@ func (j *SchemaDiffWriter) WriteSchemaDiff(ctx context.Context, schemaDiffStatem } } + j.schemaStmtsWritten++ + return iohelp.WriteAll(j.wr, []byte(fmt.Sprintf(`"%s"`, jsonEscape(schemaDiffStatement)))) } From f52071dcd3f643c9eb5df7c721510249f00d7c36 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 17:52:16 -0700 Subject: [PATCH 17/27] New bats test for json diff --- integration-tests/bats/json-diff.bats | 297 ++++++++++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 integration-tests/bats/json-diff.bats diff --git a/integration-tests/bats/json-diff.bats b/integration-tests/bats/json-diff.bats new file mode 100644 index 0000000000..2750706f9b --- /dev/null +++ b/integration-tests/bats/json-diff.bats @@ -0,0 +1,297 @@ +#!/usr/bin/env bats +load $BATS_TEST_DIRNAME/helper/common.bash + +setup() { + setup_common + + dolt sql </dev/null +} + +function no_stdout { + "$@" 1>/dev/null +} + +@test "json-diff: works with spaces in column names" { + dolt sql -q 'CREATE table t (pk int, `type of food` varchar(100));' + dolt sql -q "INSERT INTO t VALUES (1, 'ramen');" + + + EXPECTED_TABLE1=$(cat <<'EOF' +{"tables":[{"name":"t","schema_diff":["CREATE TABLE `t` (\n `pk` int,\n `type of food` varchar(100)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[{"from_row":{},"to_row":{"pk":1,"type of food":"ramen"}}]},{"name":"test","schema_diff":["CREATE TABLE `test` (\n `pk` bigint NOT NULL COMMENT 'tag:0',\n `c1` bigint COMMENT 'tag:1',\n `c2` bigint COMMENT 'tag:2',\n `c3` bigint COMMENT 'tag:3',\n `c4` bigint COMMENT 'tag:4',\n `c5` bigint COMMENT 'tag:5',\n PRIMARY KEY (`pk`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[]}]} +EOF +) + + dolt diff -r json + run dolt diff -r json + [ $status -eq 0 ] + [[ $output =~ "$EXPECTED" ]] || false +} From 0c8734502b595d9d1007d33d0480b8a023620f8b Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 17:56:08 -0700 Subject: [PATCH 18/27] Reconfigured how row diffing happens to ensure that RowWriter and Close always get called for all code paths --- go/cmd/dolt/commands/diff.go | 106 ++++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 95566fa08f..1fb5e8f2db 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -367,7 +367,6 @@ func diffUserTable( return nil } - tblName := td.ToName fromTable := td.FromTable toTable := td.ToTable @@ -397,22 +396,15 @@ func diffUserTable( } } - if dArgs.diffParts&DataOnlyDiff != 0 { - if td.IsDrop() && dArgs.diffOutput == SQLDiffOutput { - return nil // don't output DELETE FROM statements after DROP TABLE - } else if td.IsAdd() { - fromSch = toSch - } + if td.IsDrop() && dArgs.diffOutput == SQLDiffOutput { + return nil // don't output DELETE FROM statements after DROP TABLE + } else if td.IsAdd() { + fromSch = toSch + } - if !schema.ArePrimaryKeySetsDiffable(td.Format(), fromSch, toSch) { - cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", tblName) - return nil - } - - verr := diffRows(ctx, engine, td, dArgs, dw) - if verr != nil { - return verr - } + verr := diffRows(ctx, engine, td, dArgs, dw) + if verr != nil { + return verr } return nil @@ -533,35 +525,7 @@ func diffRows( ) errhand.VerboseError { from, to := dArgs.fromRef, dArgs.toRef - tableName := td.ToName - if len(tableName) == 0 { - tableName = td.FromName - } - - columns := getColumnNamesString(td.FromSch, td.ToSch) - query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s')", columns, "diff_type", tableName, from, to) - - if len(dArgs.where) > 0 { - query += " where " + dArgs.where - } - - if dArgs.limit >= 0 { - query += " limit " + strconv.Itoa(dArgs.limit) - } - - sqlCtx, err := engine.NewLocalSqlContext(ctx, se) - if err != nil { - return errhand.VerboseErrorFromError(err) - } - - sch, rowIter, err := se.Query(sqlCtx, query) - if sql.ErrSyntaxError.Is(err) { - return errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() - } else if err != nil { - return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() - } - - defer rowIter.Close(sqlCtx) + diffable := schema.ArePrimaryKeySetsDiffable(td.Format(), td.FromSch, td.ToSch) var toSch, fromSch sql.Schema if td.FromSch != nil { @@ -591,11 +555,63 @@ func diffRows( return nil } + // We always instantiate a RowWriter in case the diffWriter needs it to close off any work from schema output rowWriter, err := dw.RowWriter(ctx, td, unionSch) if err != nil { return errhand.VerboseErrorFromError(err) } + // can't diff + if !diffable { + // TODO: this messes up some structured output if the user didn't redirect it + cli.PrintErrf("Primary key sets differ between revisions for table %s, skipping data diff\n", td.ToName) + err := rowWriter.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil + } + + // no data diff requested + if dArgs.diffParts&DataOnlyDiff == 0 { + err := rowWriter.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil + } + + // do the data diff + tableName := td.ToName + if len(tableName) == 0 { + tableName = td.FromName + } + + columns := getColumnNamesString(td.FromSch, td.ToSch) + query := fmt.Sprintf("select %s, %s from dolt_diff('%s', '%s', '%s')", columns, "diff_type", tableName, from, to) + + if len(dArgs.where) > 0 { + query += " where " + dArgs.where + } + + if dArgs.limit >= 0 { + query += " limit " + strconv.Itoa(dArgs.limit) + } + + sqlCtx, err := engine.NewLocalSqlContext(ctx, se) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + + sch, rowIter, err := se.Query(sqlCtx, query) + if sql.ErrSyntaxError.Is(err) { + return errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build() + } else if err != nil { + return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() + } + + defer rowIter.Close(sqlCtx) + err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() From ad6e4a97a118fbce42140558a3ac4030008bbbb6 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 18:05:26 -0700 Subject: [PATCH 19/27] Slightly better error message for some sql diff issues --- go/cmd/dolt/commands/diff.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 1fb5e8f2db..8e76610e01 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -526,6 +526,7 @@ func diffRows( from, to := dArgs.fromRef, dArgs.toRef diffable := schema.ArePrimaryKeySetsDiffable(td.Format(), td.FromSch, td.ToSch) + canSqlDiff := !(td.ToSch == nil || (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) var toSch, fromSch sql.Schema if td.FromSch != nil { @@ -546,15 +547,6 @@ func diffRows( unionSch := unionSchemas(fromSch, toSch) - // In some cases we can't print SQL output diffs - if dArgs.diffOutput == SQLDiffOutput && - (td.ToSch == nil || - (td.FromSch != nil && !schema.SchemasAreEqual(td.FromSch, td.ToSch))) { - // TODO: this is overly broad, we can absolutely do better - _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff\n") - return nil - } - // We always instantiate a RowWriter in case the diffWriter needs it to close off any work from schema output rowWriter, err := dw.RowWriter(ctx, td, unionSch) if err != nil { @@ -570,6 +562,14 @@ func diffRows( return errhand.VerboseErrorFromError(err) } return nil + } else if dArgs.diffOutput == SQLDiffOutput && !canSqlDiff { + // TODO: this is overly broad, we can absolutely do better + _, _ = fmt.Fprintf(cli.CliErr, "Incompatible schema change, skipping data diff\n") + err := rowWriter.Close(ctx) + if err != nil { + return errhand.VerboseErrorFromError(err) + } + return nil } // no data diff requested From a0cfe4454c3264df83068a8e739280960ba9d565 Mon Sep 17 00:00:00 2001 From: zachmu Date: Thu, 25 Aug 2022 01:10:33 +0000 Subject: [PATCH 20/27] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/diff.go | 27 ++++++++++--------- go/cmd/dolt/commands/diff_output.go | 11 ++++---- .../table/typed/json/json_diff_writer.go | 17 ++++++------ .../doltcore/table/typed/json/writer.go | 10 +++---- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 8e76610e01..e8b7fbd317 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -22,6 +22,8 @@ import ( "strconv" "strings" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -35,7 +37,6 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/libraries/utils/set" - "github.com/dolthub/go-mysql-server/sql" ) type diffOutput int @@ -50,7 +51,7 @@ const ( TabularDiffOutput diffOutput = 1 SQLDiffOutput diffOutput = 2 - JsonDiffOutput diffOutput = 3 + JsonDiffOutput diffOutput = 3 DataFlag = "data" SchemaFlag = "schema" @@ -357,11 +358,11 @@ func diffUserTables(ctx context.Context, dEnv *env.DoltEnv, dArgs *diffArgs) err } func diffUserTable( - ctx context.Context, - td diff.TableDelta, - engine *engine.SqlEngine, - dArgs *diffArgs, - dw diffWriter, + ctx context.Context, + td diff.TableDelta, + engine *engine.SqlEngine, + dArgs *diffArgs, + dw diffWriter, ) errhand.VerboseError { if !dArgs.tableSet.Contains(td.FromName) && !dArgs.tableSet.Contains(td.ToName) { return nil @@ -517,11 +518,11 @@ func sqlSchemaDiff(ctx context.Context, td diff.TableDelta, toSchemas map[string } func diffRows( - ctx context.Context, - se *engine.SqlEngine, - td diff.TableDelta, - dArgs *diffArgs, - dw diffWriter, + ctx context.Context, + se *engine.SqlEngine, + td diff.TableDelta, + dArgs *diffArgs, + dw diffWriter, ) errhand.VerboseError { from, to := dArgs.fromRef, dArgs.toRef @@ -689,4 +690,4 @@ func writeDiffResults( } } } -} \ No newline at end of file +} diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go index 5813c84701..2bff8c3108 100755 --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -20,6 +20,10 @@ import ( "io" textdiff "github.com/andreyvit/diff" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dustin/go-humanize" + "github.com/fatih/color" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/diff" @@ -30,9 +34,6 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/tabular" "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/dolt/go/store/atomicerr" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dustin/go-humanize" - "github.com/fatih/color" ) // diffWriter is an interface that lets us write diffs in a variety of output formats @@ -169,7 +170,7 @@ func pluralize(singular, plural string, n uint64) string { return fmt.Sprintf("%s %s", humanize.Comma(int64(n)), noun) } -type tabularDiffWriter struct {} +type tabularDiffWriter struct{} func (t tabularDiffWriter) Close(ctx context.Context) error { return nil @@ -242,7 +243,7 @@ func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, un return tabular.NewFixedWidthDiffTableWriter(unionSch, iohelp.NopWrCloser(cli.CliOut), 100), nil } -type sqlDiffWriter struct {} +type sqlDiffWriter struct{} func (s sqlDiffWriter) Close(ctx context.Context) error { return nil diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index 72624af43d..ec2864a6cb 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -20,10 +20,11 @@ import ( "fmt" "io" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/utils/iohelp" - "github.com/dolthub/go-mysql-server/sql" ) type JsonDiffWriter struct { @@ -43,15 +44,15 @@ func NewJsonDiffWriter(wr io.WriteCloser, outSch schema.Schema) (*JsonDiffWriter return &JsonDiffWriter{ rowWriter: writer, - wr: wr, + wr: wr, }, nil } func (j *JsonDiffWriter) WriteRow( - ctx context.Context, - row sql.Row, - rowDiffType diff.ChangeType, - colDiffTypes []diff.ChangeType, + ctx context.Context, + row sql.Row, + rowDiffType diff.ChangeType, + colDiffTypes []diff.ChangeType, ) error { if len(row) != len(colDiffTypes) { return fmt.Errorf("expected the same size for columns and diff types, got %d and %d", len(row), len(colDiffTypes)) @@ -189,5 +190,5 @@ func jsonEscape(s string) string { panic(err) } // Trim the beginning and trailing " character - return string(b[1:len(b)-1]) -} \ No newline at end of file + return string(b[1 : len(b)-1]) +} diff --git a/go/libraries/doltcore/table/typed/json/writer.go b/go/libraries/doltcore/table/typed/json/writer.go index a36c503eec..b4d6d38a02 100644 --- a/go/libraries/doltcore/table/typed/json/writer.go +++ b/go/libraries/doltcore/table/typed/json/writer.go @@ -59,11 +59,11 @@ func NewJSONWriter(wr io.WriteCloser, outSch schema.Schema) (*RowWriter, error) func NewJSONWriterWithHeader(wr io.WriteCloser, outSch schema.Schema, header, footer, separator string) (*RowWriter, error) { bwr := bufio.NewWriterSize(wr, WriteBufSize) return &RowWriter{ - closer: wr, - bWr: bwr, - sch: outSch, - header: header, - footer: footer, + closer: wr, + bWr: bwr, + sch: outSch, + header: header, + footer: footer, separator: separator, }, nil } From 0a00e6b17b8cd4eb8cf1face645ee0edd95ef830 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 24 Aug 2022 19:08:28 -0700 Subject: [PATCH 21/27] Bug fix for writing no rows with json row writer --- go/libraries/doltcore/table/typed/json/writer.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/table/typed/json/writer.go b/go/libraries/doltcore/table/typed/json/writer.go index b4d6d38a02..b19858b536 100644 --- a/go/libraries/doltcore/table/typed/json/writer.go +++ b/go/libraries/doltcore/table/typed/json/writer.go @@ -220,10 +220,11 @@ func (j *RowWriter) Flush() error { // Close should flush all writes, release resources being held func (j *RowWriter) Close(ctx context.Context) error { if j.closer != nil { - err := iohelp.WriteAll(j.bWr, []byte(j.footer)) - - if err != nil { - return err + if j.rowsWritten > 0 { + err := iohelp.WriteAll(j.bWr, []byte(j.footer)) + if err != nil { + return err + } } errFl := j.bWr.Flush() From 05525ead054648c0eaa4d75b62b41e5a6dbc98f1 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 25 Aug 2022 13:02:17 -0700 Subject: [PATCH 22/27] Fixed keyless json diff test to accomodate non deterministic ordering for keyless diffs --- integration-tests/bats/json-diff.bats | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/integration-tests/bats/json-diff.bats b/integration-tests/bats/json-diff.bats index 2750706f9b..73ab36afd7 100644 --- a/integration-tests/bats/json-diff.bats +++ b/integration-tests/bats/json-diff.bats @@ -227,13 +227,18 @@ SQL dolt diff -r json run dolt diff -r json - EXPECTED=$(cat <<'EOF' -{"tables":[{"name":"t","schema_diff":[],"data_diff":[{"from_row":{"pk":1,"val":1},"to_row":{}},{"from_row":{"pk":1,"val":1},"to_row":{}},{"from_row":{},"to_row":{"pk":1,"val":2}},{"from_row":{},"to_row":{"pk":1,"val":2}}]}]} -EOF -) - + # The JSON output order for keyless diff isn't guaranteed, so we + # just count number of times the row diff strings occur + run count_string "$output" '{"from_row":{},"to_row":{"pk":1,"val":2}}' [ $status -eq 0 ] - [[ "$output" =~ "$EXPECTED" ]] || false + [ "$output" -eq "2" ] + + dolt diff -r json + run dolt diff -r json + + run count_string "$output" '{"from_row":{"pk":1,"val":1},"to_row":{}}' + [ $status -eq 0 ] + [ "$output" -eq "2" ] } @test "json-diff: adding and removing primary key" { @@ -260,7 +265,6 @@ SQL [[ "$output" =~ '{"tables":[{"name":"t","schema_diff":["ALTER TABLE `t` DROP PRIMARY KEY;","ALTER TABLE `t` ADD PRIMARY KEY (pk);"]' ]] || false [[ "$output" =~ 'Primary key sets differ between revisions for table t, skipping data diff' ]] || false - dolt commit -am 'added primary key' dolt sql -q "alter table t drop primary key" @@ -280,6 +284,11 @@ function no_stdout { "$@" 1>/dev/null } +function count_string { + cmd="echo '$1' | grep -o '$2' | wc -l" + eval "$cmd" +} + @test "json-diff: works with spaces in column names" { dolt sql -q 'CREATE table t (pk int, `type of food` varchar(100));' dolt sql -q "INSERT INTO t VALUES (1, 'ramen');" From 7ad34520e0175738c33107e83d2bcbe9d66679d3 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 25 Aug 2022 13:16:49 -0700 Subject: [PATCH 23/27] PR feedback --- go/cmd/dolt/commands/diff.go | 2 +- go/cmd/dolt/commands/diff_output.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index e8b7fbd317..17d7482aa9 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -125,7 +125,7 @@ func (cmd DiffCmd) ArgParser() *argparser.ArgParser { ap.SupportsFlag(DataFlag, "d", "Show only the data changes, do not show the schema changes (Both shown by default).") ap.SupportsFlag(SchemaFlag, "s", "Show only the schema changes, do not show the data changes (Both shown by default).") ap.SupportsFlag(SummaryFlag, "", "Show summary of data changes") - ap.SupportsString(FormatFlag, "r", "result output format", "How to format diff output. Valid values are tabular & sql. Defaults to tabular. ") + ap.SupportsString(FormatFlag, "r", "result output format", "How to format diff output. Valid values are tabular, sql, json. Defaults to tabular.") ap.SupportsString(whereParam, "", "column", "filters columns based on values in the diff. See {{.EmphasisLeft}}dolt diff --help{{.EmphasisRight}} for details.") ap.SupportsInt(limitParam, "", "record_count", "limits to the first N diffs.") ap.SupportsFlag(CachedFlag, "c", "Show only the unstaged data changes.") diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go index 2bff8c3108..d0a45e00c1 100755 --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -172,6 +172,8 @@ func pluralize(singular, plural string, n uint64) string { type tabularDiffWriter struct{} +var _ diffWriter = (*tabularDiffWriter)(nil) + func (t tabularDiffWriter) Close(ctx context.Context) error { return nil } @@ -245,6 +247,8 @@ func (t tabularDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, un type sqlDiffWriter struct{} +var _ diffWriter = (*tabularDiffWriter)(nil) + func (s sqlDiffWriter) Close(ctx context.Context) error { return nil } @@ -279,6 +283,8 @@ type jsonDiffWriter struct { tablesWritten int } +var _ diffWriter = (*tabularDiffWriter)(nil) + func newJsonDiffWriter(wr io.WriteCloser) (*jsonDiffWriter, error) { return &jsonDiffWriter{ wr: wr, From c6b6276704acdc2ca2e788100bf2bdec8a2140c0 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 25 Aug 2022 13:39:50 -0700 Subject: [PATCH 24/27] Bug fix for diffs with schema changes --- go/cmd/dolt/commands/diff_output.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go index d0a45e00c1..5da0e51cd7 100755 --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -20,6 +20,8 @@ import ( "io" textdiff "github.com/andreyvit/diff" + "github.com/dolthub/dolt/go/libraries/doltcore/schema" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" "github.com/dolthub/go-mysql-server/sql" "github.com/dustin/go-humanize" "github.com/fatih/color" @@ -346,12 +348,22 @@ func (j *jsonDiffWriter) RowWriter(ctx context.Context, td diff.TableDelta, unio return nil, err } - targetSch := td.ToSch - if targetSch == nil { - targetSch = td.FromSch + // Translate the union schema to its dolt version + cols := schema.NewColCollection() + for i, col := range unionSch { + doltCol, err := sqlutil.ToDoltCol(uint64(i), col) + if err != nil { + return nil, err + } + cols = cols.Append(doltCol) } - j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), targetSch) + sch, err := schema.SchemaFromCols(cols) + if err != nil { + return nil, err + } + + j.rowDiffWriter, err = json.NewJsonDiffWriter(iohelp.NopWrCloser(cli.CliOut), sch) return j.rowDiffWriter, err } From 4c45ae404a1fdee75ee6b14f73b9ddc64e0f0fa2 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 25 Aug 2022 13:48:24 -0700 Subject: [PATCH 25/27] Fixed bats tests for schema change output --- integration-tests/bats/json-diff.bats | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/json-diff.bats b/integration-tests/bats/json-diff.bats index 73ab36afd7..dfa58b1d3f 100644 --- a/integration-tests/bats/json-diff.bats +++ b/integration-tests/bats/json-diff.bats @@ -76,7 +76,7 @@ SQL run dolt diff -r json EXPECTED=$(cat <<'EOF' -{"tables":[{"name":"test","schema_diff":["ALTER TABLE `test` DROP `c2`;","ALTER TABLE `test` ADD `c3` varchar(10);"],"data_diff":[{"from_row":{"c1":2,"c3":3,"pk":1},"to_row":{}},{"from_row":{"c1":5,"c3":6,"pk":4},"to_row":{"c1":100,"pk":4}},{"from_row":{},"to_row":{"c1":8,"pk":7}}]}]} +{"tables":[{"name":"test","schema_diff":["ALTER TABLE `test` DROP `c2`;","ALTER TABLE `test` ADD `c3` varchar(10);"],"data_diff":[{"from_row":{"c1":2,"c2":3,"pk":1},"to_row":{}},{"from_row":{"c1":5,"c2":6,"pk":4},"to_row":{"c1":100,"pk":4}},{"from_row":{},"to_row":{"c1":8,"c3":"9","pk":7}}]}]} EOF ) @@ -99,11 +99,24 @@ EOF dolt diff -r json --data run dolt diff -r json --data EXPECTED=$(cat <<'EOF' -{"tables":[{"name":"test","schema_diff":[],"data_diff":[{"from_row":{"c1":2,"c3":3,"pk":1},"to_row":{}},{"from_row":{"c1":5,"c3":6,"pk":4},"to_row":{"c1":100,"pk":4}},{"from_row":{},"to_row":{"c1":8,"pk":7}}]}]} +{"tables":[{"name":"test","schema_diff":[],"data_diff":[{"from_row":{"c1":2,"c2":3,"pk":1},"to_row":{}},{"from_row":{"c1":5,"c2":6,"pk":4},"to_row":{"c1":100,"pk":4}},{"from_row":{},"to_row":{"c1":8,"c3":"9","pk":7}}]}]} EOF ) [[ "$output" =~ "$EXPECTED" ]] || false + + dolt commit -am "committing changes" + dolt sql -q "alter table test rename column c1 to c1new" + dolt sql -q "update test set c1new = c1new*2" + + dolt diff -r json --data + run dolt diff -r json --data + EXPECTED=$(cat <<'EOF' +{"tables":[{"name":"test","schema_diff":[],"data_diff":[{"from_row":{"c1":100,"pk":4},"to_row":{"c1new":200,"pk":4}},{"from_row":{"c1":8,"c3":"9","pk":7},"to_row":{"c1new":16,"c3":"9","pk":7}}]}]} +EOF +) + [ "$status" -eq 0 ] + [[ "$output" =~ "$EXPECTED" ]] || false } @test "json-diff: with table args" { From f63bb0242f18aa6816bbce529f9b45be5c6aa9b8 Mon Sep 17 00:00:00 2001 From: zachmu Date: Thu, 25 Aug 2022 21:01:54 +0000 Subject: [PATCH 26/27] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/diff_output.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) mode change 100755 => 100644 go/cmd/dolt/commands/diff_output.go diff --git a/go/cmd/dolt/commands/diff_output.go b/go/cmd/dolt/commands/diff_output.go old mode 100755 new mode 100644 index 5da0e51cd7..ff6cf8b7ca --- a/go/cmd/dolt/commands/diff_output.go +++ b/go/cmd/dolt/commands/diff_output.go @@ -20,8 +20,6 @@ import ( "io" textdiff "github.com/andreyvit/diff" - "github.com/dolthub/dolt/go/libraries/doltcore/schema" - "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" "github.com/dolthub/go-mysql-server/sql" "github.com/dustin/go-humanize" "github.com/fatih/color" @@ -30,7 +28,9 @@ import ( "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/doltcore/sqle" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" "github.com/dolthub/dolt/go/libraries/doltcore/table/typed/json" "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/sqlexport" "github.com/dolthub/dolt/go/libraries/doltcore/table/untyped/tabular" From a1aebfbabe842b8c1a0c67b2e853afe3d5eeea03 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 25 Aug 2022 15:04:54 -0700 Subject: [PATCH 27/27] Fixed faulty json test --- integration-tests/bats/json-diff.bats | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/integration-tests/bats/json-diff.bats b/integration-tests/bats/json-diff.bats index dfa58b1d3f..69ccb4924a 100644 --- a/integration-tests/bats/json-diff.bats +++ b/integration-tests/bats/json-diff.bats @@ -303,12 +303,11 @@ function count_string { } @test "json-diff: works with spaces in column names" { - dolt sql -q 'CREATE table t (pk int, `type of food` varchar(100));' + dolt sql -q 'CREATE table t (pk int primary key, `type of food` varchar(100));' dolt sql -q "INSERT INTO t VALUES (1, 'ramen');" - - EXPECTED_TABLE1=$(cat <<'EOF' -{"tables":[{"name":"t","schema_diff":["CREATE TABLE `t` (\n `pk` int,\n `type of food` varchar(100)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[{"from_row":{},"to_row":{"pk":1,"type of food":"ramen"}}]},{"name":"test","schema_diff":["CREATE TABLE `test` (\n `pk` bigint NOT NULL COMMENT 'tag:0',\n `c1` bigint COMMENT 'tag:1',\n `c2` bigint COMMENT 'tag:2',\n `c3` bigint COMMENT 'tag:3',\n `c4` bigint COMMENT 'tag:4',\n `c5` bigint COMMENT 'tag:5',\n PRIMARY KEY (`pk`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[]}]} + EXPECTED=$(cat <<'EOF' +{"tables":[{"name":"t","schema_diff":["CREATE TABLE `t` (\n `pk` int NOT NULL,\n `type of food` varchar(100),\n PRIMARY KEY (`pk`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[{"from_row":{},"to_row":{"pk":1,"type of food":"ramen"}}]},{"name":"test","schema_diff":["CREATE TABLE `test` (\n `pk` bigint NOT NULL COMMENT 'tag:0',\n `c1` bigint COMMENT 'tag:1',\n `c2` bigint COMMENT 'tag:2',\n `c3` bigint COMMENT 'tag:3',\n `c4` bigint COMMENT 'tag:4',\n `c5` bigint COMMENT 'tag:5',\n PRIMARY KEY (`pk`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;"],"data_diff":[]}]} EOF )