From e2b3efb991150e0fad1c14322612f2e013d79d1d Mon Sep 17 00:00:00 2001 From: elianddb Date: Wed, 3 Sep 2025 12:52:58 -0700 Subject: [PATCH 01/11] add bats and --skinny flag --- .../sqle/dtablefunctions/dolt_diff.go | 54 ++++++---- .../bats/helper/windows-compat.bash | 2 +- integration-tests/bats/sql-diff.bats | 102 ++++++++++++++++++ 3 files changed, 139 insertions(+), 19 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 07e49437b8..011ccb0ccf 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -46,6 +46,7 @@ var _ sql.AuthorizationCheckerNode = (*DiffTableFunction)(nil) type DiffTableFunction struct { tableDelta diff.TableDelta + skinnyExpr sql.Expression fromCommitExpr sql.Expression toCommitExpr sql.Expression dotCommitExpr sql.Expression @@ -100,20 +101,23 @@ func (dtf *DiffTableFunction) WithDatabase(database sql.Database) (sql.Node, err // Expressions implements the sql.Expressioner interface func (dtf *DiffTableFunction) Expressions() []sql.Expression { + exprs := []sql.Expression{} + if dtf.skinnyExpr != nil { + exprs = append(exprs, dtf.skinnyExpr) + } + if dtf.dotCommitExpr != nil { - return []sql.Expression{ - dtf.dotCommitExpr, dtf.tableNameExpr, - } - } - return []sql.Expression{ - dtf.fromCommitExpr, dtf.toCommitExpr, dtf.tableNameExpr, + exprs = append(exprs, dtf.dotCommitExpr, dtf.tableNameExpr) + } else { + exprs = append(exprs, dtf.fromCommitExpr, dtf.toCommitExpr, dtf.tableNameExpr) } + return exprs } // WithExpressions implements the sql.Expressioner interface func (dtf *DiffTableFunction) WithExpressions(expression ...sql.Expression) (sql.Node, error) { if len(expression) < 2 { - return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 3", len(expression)) + return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 4", len(expression)) } // TODO: For now, we will only support literal / fully-resolved arguments to the @@ -130,6 +134,12 @@ func (dtf *DiffTableFunction) WithExpressions(expression ...sql.Expression) (sql } newDtf := *dtf + + if len(expression) > 0 && expression[0].String() == "'--skinny'" { + newDtf.skinnyExpr = expression[0] + expression = expression[1:] // Remove the skinny parameter from the slice + } + if strings.Contains(expression[0].String(), "..") { if len(expression) != 2 { return nil, sql.ErrInvalidArgumentNumber.New(fmt.Sprintf("%v with .. or ...", newDtf.Name()), 2, len(expression)) @@ -559,10 +569,15 @@ func (dtf *DiffTableFunction) Schema() sql.Schema { // Resolved implements the sql.Resolvable interface func (dtf *DiffTableFunction) Resolved() bool { - if dtf.dotCommitExpr != nil { - return dtf.tableNameExpr.Resolved() && dtf.dotCommitExpr.Resolved() + resolved := true + if dtf.skinnyExpr != nil { + resolved = resolved && dtf.skinnyExpr.Resolved() } - return dtf.tableNameExpr.Resolved() && dtf.fromCommitExpr.Resolved() && dtf.toCommitExpr.Resolved() + + if dtf.dotCommitExpr != nil { + return resolved && dtf.tableNameExpr.Resolved() && dtf.dotCommitExpr.Resolved() + } + return resolved && dtf.tableNameExpr.Resolved() && dtf.fromCommitExpr.Resolved() && dtf.toCommitExpr.Resolved() } func (dtf *DiffTableFunction) IsReadOnly() bool { @@ -571,15 +586,18 @@ func (dtf *DiffTableFunction) IsReadOnly() bool { // String implements the Stringer interface func (dtf *DiffTableFunction) String() string { - if dtf.dotCommitExpr != nil { - return fmt.Sprintf("DOLT_DIFF(%s, %s)", - dtf.dotCommitExpr.String(), - dtf.tableNameExpr.String()) + args := []string{} + if dtf.skinnyExpr != nil { + args = append(args, dtf.skinnyExpr.String()) } - return fmt.Sprintf("DOLT_DIFF(%s, %s, %s)", - dtf.fromCommitExpr.String(), - dtf.toCommitExpr.String(), - dtf.tableNameExpr.String()) + + if dtf.dotCommitExpr != nil { + args = append(args, dtf.dotCommitExpr.String(), dtf.tableNameExpr.String()) + } else { + args = append(args, dtf.fromCommitExpr.String(), dtf.toCommitExpr.String(), dtf.tableNameExpr.String()) + } + + return fmt.Sprintf("DOLT_DIFF(%s)", strings.Join(args, ", ")) } // Name implements the sql.TableFunction interface diff --git a/integration-tests/bats/helper/windows-compat.bash b/integration-tests/bats/helper/windows-compat.bash index 1305bbc3f1..344fbcb476 100644 --- a/integration-tests/bats/helper/windows-compat.bash +++ b/integration-tests/bats/helper/windows-compat.bash @@ -5,7 +5,7 @@ skiponwindows() { :; } IS_WINDOWS=${IS_WINDOWS:-false} WINDOWS_BASE_DIR=${WINDOWS_BASE_DIR:-/mnt/c} -if [ -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then +if [ ! -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then IS_WINDOWS=true if [ ! -d "$WINDOWS_BASE_DIR"/batstmp ]; then mkdir "$WINDOWS_BASE_DIR"/batstmp diff --git a/integration-tests/bats/sql-diff.bats b/integration-tests/bats/sql-diff.bats index 3f1daf1a3c..78c79086a8 100644 --- a/integration-tests/bats/sql-diff.bats +++ b/integration-tests/bats/sql-diff.bats @@ -890,3 +890,105 @@ EOF [ "$status" -eq 1 ] [[ "$output" =~ "invalid output format: sql. SQL format diffs only rendered for schema or data changes" ]] || false } + +@test "sql-diff: skinny flag comparison between CLI and SQL table function" { + dolt sql < Date: Wed, 3 Sep 2025 15:57:39 -0700 Subject: [PATCH 02/11] fix flag parse --- .../sqle/dtablefunctions/dolt_diff.go | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 011ccb0ccf..7d69fec09d 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -115,15 +115,13 @@ func (dtf *DiffTableFunction) Expressions() []sql.Expression { } // WithExpressions implements the sql.Expressioner interface -func (dtf *DiffTableFunction) WithExpressions(expression ...sql.Expression) (sql.Node, error) { - if len(expression) < 2 { - return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 4", len(expression)) - } - +func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sql.Node, error) { + newDtf := *dtf + var filteredExprs []sql.Expression // TODO: For now, we will only support literal / fully-resolved arguments to the // DiffTableFunction to avoid issues where the schema is needed in the analyzer // before the arguments could be resolved. - for _, expr := range expression { + for _, expr := range expressions { if !expr.Resolved() { return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } @@ -131,28 +129,32 @@ func (dtf *DiffTableFunction) WithExpressions(expression ...sql.Expression) (sql if _, ok := expr.(sql.FunctionExpression); ok { return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } - } - newDtf := *dtf - - if len(expression) > 0 && expression[0].String() == "'--skinny'" { - newDtf.skinnyExpr = expression[0] - expression = expression[1:] // Remove the skinny parameter from the slice - } - - if strings.Contains(expression[0].String(), "..") { - if len(expression) != 2 { - return nil, sql.ErrInvalidArgumentNumber.New(fmt.Sprintf("%v with .. or ...", newDtf.Name()), 2, len(expression)) + switch expr.String() { + case "'--skinny'", "'-sk'": + newDtf.skinnyExpr = expr + default: + filteredExprs = append(filteredExprs, expr) } - newDtf.dotCommitExpr = expression[0] - newDtf.tableNameExpr = expression[1] + } + + if len(filteredExprs) < 2 { + return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 3", len(filteredExprs)) + } + + if strings.Contains(filteredExprs[0].String(), "..") { + if len(filteredExprs) != 2 { + return nil, sql.ErrInvalidArgumentNumber.New(fmt.Sprintf("%v with .. or ...", newDtf.Name()), 2, len(filteredExprs)) + } + newDtf.dotCommitExpr = filteredExprs[0] + newDtf.tableNameExpr = filteredExprs[1] } else { - if len(expression) != 3 { - return nil, sql.ErrInvalidArgumentNumber.New(newDtf.Name(), 3, len(expression)) + if len(filteredExprs) != 3 { + return nil, sql.ErrInvalidArgumentNumber.New(newDtf.Name(), 3, len(filteredExprs)) } - newDtf.fromCommitExpr = expression[0] - newDtf.toCommitExpr = expression[1] - newDtf.tableNameExpr = expression[2] + newDtf.fromCommitExpr = filteredExprs[0] + newDtf.toCommitExpr = filteredExprs[1] + newDtf.tableNameExpr = filteredExprs[2] } fromCommitVal, toCommitVal, dotCommitVal, tableName, err := newDtf.evaluateArguments() @@ -571,7 +573,7 @@ func (dtf *DiffTableFunction) Schema() sql.Schema { func (dtf *DiffTableFunction) Resolved() bool { resolved := true if dtf.skinnyExpr != nil { - resolved = resolved && dtf.skinnyExpr.Resolved() + resolved = dtf.skinnyExpr.Resolved() } if dtf.dotCommitExpr != nil { From 493e9c18c5e502d261c7ef96131b90ada6b76b5e Mon Sep 17 00:00:00 2001 From: elianddb Date: Thu, 4 Sep 2025 16:12:14 -0700 Subject: [PATCH 03/11] add --skinny impl --- .../sqle/dtablefunctions/dolt_diff.go | 139 ++++++++++++++++-- .../doltcore/sqle/dtables/diff_table.go | 5 - .../sqle/enginetest/dolt_queries_diff.go | 111 ++++++++++++++ integration-tests/bats/helper/sql-diff.bash | 139 ++++++++++++++++++ integration-tests/bats/sql-diff.bats | 76 ++-------- 5 files changed, 384 insertions(+), 86 deletions(-) create mode 100644 integration-tests/bats/helper/sql-diff.bash diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 7d69fec09d..5de4633b83 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -16,6 +16,8 @@ package dtablefunctions import ( "fmt" + table2 "github.com/dolthub/dolt/go/libraries/doltcore/table" + "io" "strings" "github.com/dolthub/go-mysql-server/sql" @@ -435,6 +437,114 @@ func (dtf *DiffTableFunction) evaluateArguments() (interface{}, interface{}, int return fromCommitVal, toCommitVal, nil, tableName, nil } +// FilterDeltaSchemaToSkinnyCols creates a filtered version of the table delta that omits columns which are identical +// in type and value across all rows in both schemas, except for primary key columns which are always included. Also +// updates dtf.tableDelta with the filtered result. +func (dtf *DiffTableFunction) FilterDeltaSchemaToSkinnyCols(ctx *sql.Context, delta diff.TableDelta) (diff.TableDelta, error) { + if delta.FromTable == nil || delta.ToTable == nil { + return delta, nil + } + + // potential cols for omission: + fromEqualColsIndices := map[string]int{} + toEqualColsIndices := map[string]int{} + for fromI, fromCol := range delta.FromSch.GetAllCols().GetColumns() { + col, ok := delta.ToSch.GetAllCols().GetByName(fromCol.Name) + if !ok { // column was dropped + continue + } + + if fromCol.TypeInfo.Equals(col.TypeInfo) { + fromEqualColsIndices[fromCol.Name] = fromI + toEqualColsIndices[col.Name] = delta.ToSch.GetAllCols().IndexOf(col.Name) + } + } + + fromRowData, err := delta.FromTable.GetRowData(ctx) + if err != nil { + return diff.TableDelta{}, err + } + + toRowData, err := delta.ToTable.GetRowData(ctx) + if err != nil { + return diff.TableDelta{}, err + } + + fromIter, err := table2.NewTableIterator(ctx, delta.FromSch, fromRowData) + if err != nil { + return diff.TableDelta{}, err + } + defer fromIter.Close(ctx) + + toIter, err := table2.NewTableIterator(ctx, delta.ToSch, toRowData) + if err != nil { + return diff.TableDelta{}, err + } + defer toIter.Close(ctx) + + for { + fromRow, fromErr := fromIter.Next(ctx) + toRow, toErr := toIter.Next(ctx) + + if fromErr == io.EOF && toErr == io.EOF { + break + } + + if fromErr != nil && fromErr != io.EOF { + return diff.TableDelta{}, fromErr + } + + if toErr != nil && toErr != io.EOF { + return diff.TableDelta{}, toErr + } + + // xor: if only one is nil, then all cols are diffs + if (fromRow == nil) != (toRow == nil) { + fromEqualColsIndices = map[string]int{} + toEqualColsIndices = map[string]int{} + break + } + + if fromRow == nil && toRow == nil { + continue + } + + for colName, fromI := range fromEqualColsIndices { + toI := toEqualColsIndices[colName] + fromVal := fromRow[fromI] + toVal := toRow[toI] + + if fromVal != toVal { + // rm from the diff cols, since we found a diff + delete(fromEqualColsIndices, colName) + delete(toEqualColsIndices, colName) + } + } + } + + var fromSkinnyCols []schema.Column + for _, col := range delta.FromSch.GetAllCols().GetColumns() { + _, ok := fromEqualColsIndices[col.Name] + if col.IsPartOfPK || !ok { + fromSkinnyCols = append(fromSkinnyCols, col) + } + } + + var toSkinnyCols []schema.Column + for _, col := range delta.ToSch.GetAllCols().GetColumns() { + _, ok := toEqualColsIndices[col.Name] + if col.IsPartOfPK || !ok { + toSkinnyCols = append(toSkinnyCols, col) + } + } + + skinnyDelta := delta + skinnyDelta.FromSch = schema.MustSchemaFromCols(schema.NewColCollection(fromSkinnyCols...)) + skinnyDelta.ToSch = schema.MustSchemaFromCols(schema.NewColCollection(toSkinnyCols...)) + dtf.tableDelta = skinnyDelta + return skinnyDelta, nil +} + func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, toCommitVal, dotCommitVal interface{}, tableName string) error { if !dtf.Resolved() { return nil @@ -450,27 +560,26 @@ func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, to return err } + if dtf.skinnyExpr != nil { + delta, err = dtf.FilterDeltaSchemaToSkinnyCols(ctx, delta) + if err != nil { + return err + } + } + fromTable, fromTableExists := delta.FromTable, delta.FromTable != nil toTable, toTableExists := delta.ToTable, delta.ToTable != nil - if !toTableExists && !fromTableExists { + var format *types.NomsBinFormat + if toTableExists { + format = toTable.Format() + } else if fromTableExists { + format = fromTable.Format() + } else { return sql.ErrTableNotFound.New(tableName) } - var toSchema, fromSchema schema.Schema - var format *types.NomsBinFormat - - if fromTableExists { - fromSchema = delta.FromSch - format = fromTable.Format() - } - - if toTableExists { - toSchema = delta.ToSch - format = toTable.Format() - } - - diffTableSch, j, err := dtables.GetDiffTableSchemaAndJoiner(format, fromSchema, toSchema) + diffTableSch, j, err := dtables.GetDiffTableSchemaAndJoiner(format, delta.FromSch, delta.ToSch) if err != nil { return err } diff --git a/go/libraries/doltcore/sqle/dtables/diff_table.go b/go/libraries/doltcore/sqle/dtables/diff_table.go index a60b52a062..be70f5f345 100644 --- a/go/libraries/doltcore/sqle/dtables/diff_table.go +++ b/go/libraries/doltcore/sqle/dtables/diff_table.go @@ -917,11 +917,6 @@ func GetDiffTableSchemaAndJoiner(format *types.NomsBinFormat, fromSch, toSch sch return nil, nil, err } } else { - fromSch, toSch, err = expandFromToSchemas(fromSch, toSch) - if err != nil { - return nil, nil, err - } - j, err = rowconv.NewJoiner( []rowconv.NamedSchema{{Name: diff.To, Sch: toSch}, {Name: diff.From, Sch: fromSch}}, map[string]rowconv.ColNamingFunc{ diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go index 19689f622c..9a0fd20c1f 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go @@ -15,6 +15,7 @@ package enginetest import ( + "fmt" "github.com/dolthub/go-mysql-server/enginetest/queries" "github.com/dolthub/go-mysql-server/sql" gmstypes "github.com/dolthub/go-mysql-server/sql/types" @@ -818,7 +819,117 @@ var Dolt1DiffSystemTableScripts = []queries.ScriptTest{ }, } +func countDataColsAssertions(viewName, diffSelectStar string, expected int64) []queries.ScriptTestAssertion { + return []queries.ScriptTestAssertion{ + {Query: fmt.Sprintf("DROP VIEW IF EXISTS %s;", viewName)}, + {Query: fmt.Sprintf("CREATE VIEW %s AS %s;", viewName, diffSelectStar)}, + { + Query: fmt.Sprintf(` + SELECT COUNT(DISTINCT REPLACE(REPLACE(column_name,'to_',''),'from_','')) + FROM information_schema.columns + WHERE table_schema = DATABASE() + AND table_name = '%s' + AND column_name NOT IN ('to_commit','from_commit','to_commit_date','from_commit_date','diff_type')`, viewName), + Expected: []sql.Row{{expected}}, + }, + {Query: fmt.Sprintf("DROP VIEW %s;", viewName)}, + } +} + var DiffTableFunctionScriptTests = []queries.ScriptTest{ + { + Name: "dolt_diff: SELECT * skinny schema visibility", + SetUpScript: []string{ + "CREATE TABLE t (" + + "pk BIGINT NOT NULL COMMENT 'tag:0'," + + "c1 BIGINT COMMENT 'tag:1'," + + "c2 BIGINT COMMENT 'tag:2'," + + "c3 BIGINT COMMENT 'tag:3'," + + "c4 BIGINT COMMENT 'tag:4'," + + "c5 BIGINT COMMENT 'tag:5'," + + "PRIMARY KEY (pk)" + + ");", + "call dolt_add('.')", + "set @C0 = '';", + "call dolt_commit_hash_out(@C0, '-m', 'Created table t');", + "INSERT INTO t VALUES (0,1,2,3,4,5), (1,1,2,3,4,5);", + "call dolt_add('.')", + "set @C1 = '';", + "call dolt_commit_hash_out(@C1, '-m', 'Added initial data');", + + "UPDATE t SET c1=100, c3=300 WHERE pk=0;", + "UPDATE t SET c2=200 WHERE pk=1;", + "call dolt_add('.')", + "set @C2 = '';", + "call dolt_commit_hash_out(@C2, '-m', 'Updated some columns');", + + "ALTER TABLE t ADD COLUMN c6 BIGINT;", + "UPDATE t SET c6=600 WHERE pk=0;", + "call dolt_add('.')", + "set @C3 = '';", + "call dolt_commit_hash_out(@C3, '-m', 'Added new column and updated it');", + + "DELETE FROM t WHERE pk=1;", + "call dolt_add('.')", + "set @C4 = '';", + "call dolt_commit_hash_out(@C4, '-m', 'Deleted a row');", + }, + Assertions: func() []queries.ScriptTestAssertion { + asserts := []queries.ScriptTestAssertion{ + { + Query: "SELECT d.to_pk, d.to_c1, d.to_c2, d.to_c3, d.to_c4, d.to_c5, d.from_pk, d.from_c1, d.from_c2, d.from_c3, d.from_c4, d.from_c5, d.diff_type " + + "FROM (SELECT * FROM dolt_diff('--skinny', @C0, @C1, 't')) d " + + "ORDER BY COALESCE(d.to_pk, d.from_pk)", + Expected: []sql.Row{ + {int64(0), int64(1), int64(2), int64(3), int64(4), int64(5), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), "added"}, + {int64(1), int64(1), int64(2), int64(3), int64(4), int64(5), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), interface{}(nil), "added"}, + }, + }, + { + Query: "SELECT d.to_pk, d.to_c1, d.to_c2, d.to_c3, d.from_pk, d.from_c1, d.from_c2, d.from_c3, d.diff_type " + + "FROM (SELECT * FROM dolt_diff(@C1, @C2, 't')) d " + + "ORDER BY COALESCE(d.to_pk, d.from_pk)", + Expected: []sql.Row{ + {int64(0), int64(100), int64(2), int64(300), int64(0), int64(1), int64(2), int64(3), "modified"}, + {int64(1), int64(1), int64(200), int64(3), int64(1), int64(1), int64(2), int64(3), "modified"}, + }, + }, + { + Query: "SELECT d.to_pk, d.to_c1, d.to_c2, d.to_c3, d.diff_type " + + "FROM (SELECT * FROM dolt_diff('--skinny', @C1, @C2, 't')) d " + + "ORDER BY d.to_pk", + Expected: []sql.Row{ + {int64(0), int64(100), int64(2), int64(300), "modified"}, + {int64(1), int64(1), int64(200), int64(3), "modified"}, + }, + }, + { + Query: "SELECT d.to_pk, d.to_c6, d.diff_type " + + "FROM (SELECT * FROM dolt_diff('--skinny', @C2, @C3, 't')) d", + Expected: []sql.Row{ + {int64(0), int64(600), "modified"}, + }, + }, + { + Query: "SELECT d.from_pk, d.from_c1, d.from_c2, d.from_c3, d.from_c4, d.from_c5, d.from_c6, d.diff_type " + + "FROM (SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')) d", + Expected: []sql.Row{ + {int64(1), int64(1), int64(200), int64(3), int64(4), int64(5), nil, "removed"}, + }, + }, + } + asserts = append(asserts, countDataColsAssertions("v_all_01", "SELECT * FROM dolt_diff(@C0, @C1, 't')", 6)...) + asserts = append(asserts, countDataColsAssertions("v_skinny_01", "SELECT * FROM dolt_diff('--skinny', @C0, @C1, 't')", 6)...) + asserts = append(asserts, countDataColsAssertions("v_all_12", "SELECT * FROM dolt_diff(@C1, @C2, 't')", 6)...) + asserts = append(asserts, countDataColsAssertions("v_skinny_12", "SELECT * FROM dolt_diff('--skinny', @C1, @C2, 't')", 4)...) + asserts = append(asserts, countDataColsAssertions("v_all_23", "SELECT * FROM dolt_diff(@C2, @C3, 't')", 7)...) + asserts = append(asserts, countDataColsAssertions("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', @C2, @C3, 't')", 2)...) + asserts = append(asserts, countDataColsAssertions("v_all_34", "SELECT * FROM dolt_diff(@C3, @C4, 't')", 7)...) + asserts = append(asserts, countDataColsAssertions("v_skinny_34", "SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')", 7)...) + + return asserts + }(), + }, { Name: "invalid arguments", SetUpScript: []string{ diff --git a/integration-tests/bats/helper/sql-diff.bash b/integration-tests/bats/helper/sql-diff.bash new file mode 100644 index 0000000000..9c59089c82 --- /dev/null +++ b/integration-tests/bats/helper/sql-diff.bash @@ -0,0 +1,139 @@ +# dolt/integration-tests/bats/helper/sql-diff.bash + +: "${SQL_DIFF_DEBUG:=}" # set to any value to enable debug output +_dbg() { [ -n "$SQL_DIFF_DEBUG" ] && printf '%s\n' "$*" >&2; } +_dbg_block() { [ -n "$SQL_DIFF_DEBUG" ] && { printf '%s\n' "$1" >&2; printf '%s\n' "$2" >&2; }; } + +# first table header row from CLI diff (data section), as newline list +_cli_header_cols() { + awk ' + /^\s*\|\s*[-+<>]\s*\|/ && last_header != "" { print last_header; exit } + /^\s*\|/ { last_header = $0 } + ' <<<"$1" \ + | tr '|' '\n' \ + | sed -e 's/^[[:space:]]*//;s/[[:space:]]*$//' \ + | grep -v -E '^(<|>|)$' \ + | grep -v '^$' +} + +# first table header row from SQL diff, strip to_/from_, drop metadata, as newline list +_sql_data_header_cols() { + echo "$1" \ + | awk '/^\|/ {print; exit}' \ + | tr '|' '\n' \ + | sed -e 's/^[[:space:] ]*//;s/[[:space:] ]*$//' \ + | grep -E '^(to_|from_)' \ + | sed -E 's/^(to_|from_)//' \ + | grep -Ev '^(commit|commit_date|diff_type)$' \ + | grep -v '^$' +} + +# count CLI changes by unique PK (includes +, -, <, >) +_cli_change_count() { + awk -F'|' ' + # start counting once we see a data row marker + /^\s*\|\s*[-+<>]\s*\|/ { in_table=1 } + in_table && $2 ~ /^[[:space:]]*[-+<>][[:space:]]*$/ { + pk=$3 + gsub(/^[[:space:]]+|[[:space:]]+$/, "", pk) + if (pk != "") seen[pk]=1 + } + END { c=0; for (k in seen) c++; print c+0 } + ' <<<"$1" +} + +# count SQL data rows (lines starting with '|' minus header) +_sql_row_count() { + echo "$1" | awk '/^\|/ {c++} END{print (c>0?c-1:0)}' +} + +# compare two newline lists as sets (sorted) +_compare_sets_or_err() { + local name="$1" cli_cols="$2" sql_cols="$3" cli_out="$4" sql_out="$5" + + local cli_sorted sql_sorted + cli_sorted=$(echo "$cli_cols" | sort -u) + sql_sorted=$(echo "$sql_cols" | sort -u) + + _dbg_block "$name CLI columns:" "$cli_sorted" + _dbg_block "$name SQL data columns:" "$sql_sorted" + + if [ "$cli_sorted" != "$sql_sorted" ]; then + echo "$name column set mismatch" + echo "--- $name CLI columns ---"; echo "$cli_sorted" + echo "--- $name SQL data columns ---"; echo "$sql_sorted" + echo "--- $name CLI output ---"; echo "$cli_out" + echo "--- $name SQL output ---"; echo "$sql_out" + return 1 + fi + return 0 +} + +# compare change/row counts; on mismatch, print both outputs +_compare_counts_or_err() { + local name="$1" cli_out="$2" sql_out="$3" cli_count="$4" sql_count="$5" + + _dbg "$name counts: CLI=$cli_count SQL=$sql_count" + + if [ "$cli_count" != "$sql_count" ]; then + echo "$name change count mismatch: CLI=$cli_count, SQL=$sql_count" + echo "--- $name CLI output ---"; echo "$cli_out" + echo "--- $name SQL output ---"; echo "$sql_out" + return 1 + fi + return 0 +} + +# ---- main entrypoint ---- + +# Compare CLI diff with SQL dolt_diff (both normal and skinny) +# Usage: compare_dolt_diff from_commit to_commit table_name +compare_dolt_diff() { + local from_commit="$1" + local to_commit="$2" + local table_name="$3" + + # --- normal mode --- + local cli_normal_output sql_normal_output cli_status sql_status + cli_normal_output=$(dolt diff "$from_commit" "$to_commit" "$table_name" 2>&1); cli_status=$? + sql_normal_output=$(dolt sql -q "SELECT * FROM dolt_diff('$from_commit', '$to_commit', '$table_name')" 2>&1); sql_status=$? + + echo "$cli_normal_output" + echo "$sql_normal_output" + + if [ $cli_status -ne 0 ]; then echo "CLI diff failed"; echo "$cli_normal_output"; return 1; fi + if [ $sql_status -ne 0 ]; then echo "SQL dolt_diff failed"; echo "$sql_normal_output"; return 1; fi + + local normal_cli_changes normal_sql_rows + normal_cli_changes=$(_cli_change_count "$cli_normal_output") + normal_sql_rows=$(_sql_row_count "$sql_normal_output") + _compare_counts_or_err "Normal diff" "$cli_normal_output" "$sql_normal_output" "$normal_cli_changes" "$normal_sql_rows" || return 1 + + local normal_cli_cols normal_sql_cols + normal_cli_cols=$(_cli_header_cols "$cli_normal_output") + normal_sql_cols=$(_sql_data_header_cols "$sql_normal_output") + _compare_sets_or_err "Normal diff" "$normal_cli_cols" "$normal_sql_cols" "$cli_normal_output" "$sql_normal_output" || return 1 + + # --- skinny mode --- + local cli_skinny_output sql_skinny_output + cli_skinny_output=$(dolt diff --skinny "$from_commit" "$to_commit" "$table_name" 2>&1); cli_status=$? + sql_skinny_output=$(dolt sql -q "SELECT * FROM dolt_diff('--skinny', '$from_commit', '$to_commit', '$table_name')" 2>&1); sql_status=$? + + echo "$cli_skinny_output" + echo "$sql_skinny_output" + + if [ $cli_status -ne 0 ]; then echo "CLI skinny diff failed"; echo "$cli_skinny_output"; return 1; fi + if [ $sql_status -ne 0 ]; then echo "SQL skinny dolt_diff failed"; echo "$sql_skinny_output"; return 1; fi + + local skinny_cli_changes skinny_sql_rows + skinny_cli_changes=$(_cli_change_count "$cli_skinny_output") + skinny_sql_rows=$(_sql_row_count "$sql_skinny_output") + _compare_counts_or_err "Skinny diff" "$cli_skinny_output" "$sql_skinny_output" "$skinny_cli_changes" "$skinny_sql_rows" || return 1 + + local skinny_cli_cols skinny_sql_cols + skinny_cli_cols=$(_cli_header_cols "$cli_skinny_output") + skinny_sql_cols=$(_sql_data_header_cols "$sql_skinny_output") + _compare_sets_or_err "Skinny diff" "$skinny_cli_cols" "$skinny_sql_cols" "$cli_skinny_output" "$sql_skinny_output" || return 1 + + return 0 +} \ No newline at end of file diff --git a/integration-tests/bats/sql-diff.bats b/integration-tests/bats/sql-diff.bats index 78c79086a8..4d164e80b2 100644 --- a/integration-tests/bats/sql-diff.bats +++ b/integration-tests/bats/sql-diff.bats @@ -1,5 +1,6 @@ #!/usr/bin/env bats load $BATS_TEST_DIRNAME/helper/common.bash +load $BATS_TEST_DIRNAME/helper/sql-diff.bash setup() { setup_common @@ -907,88 +908,31 @@ SQL dolt add test dolt commit -m "Added initial data" - # Make some changes to test skinny filtering + compare_dolt_diff "HEAD~1" "HEAD" "test" + dolt sql -q "UPDATE test SET c1=100, c3=300 WHERE pk=0" dolt sql -q "UPDATE test SET c2=200 WHERE pk=1" dolt add test dolt commit -m "Updated some columns" - # Get CLI skinny diff output - run dolt diff --skinny HEAD~1 - [ "$status" -eq 0 ] - cli_output="$output" + compare_dolt_diff "HEAD~1" "HEAD" "test" - # Get SQL table function skinny diff output - run dolt sql -q "SELECT * FROM dolt_diff('--skinny', 'HEAD~1', 'HEAD', 'test')" - [ "$status" -eq 0 ] - sql_output="$output" - - # Both should show only the changed columns (pk, c1, c2, c3) and not unchanged columns (c4, c5) - [[ "$cli_output" =~ 'pk' ]] || false - [[ "$cli_output" =~ 'c1' ]] || false - [[ "$cli_output" =~ 'c2' ]] || false - [[ "$cli_output" =~ 'c3' ]] || false - [[ ! "$cli_output" =~ 'c4' ]] || false - [[ ! "$cli_output" =~ 'c5' ]] || false - - [[ "$sql_output" =~ 'pk' ]] || false - [[ "$sql_output" =~ 'c1' ]] || false - [[ "$sql_output" =~ 'c2' ]] || false - [[ "$sql_output" =~ 'c3' ]] || false - [[ ! "$sql_output" =~ 'c4' ]] || false - [[ ! "$sql_output" =~ 'c5' ]] || false - - # Test with schema changes dolt sql -q "ALTER TABLE test ADD COLUMN c6 BIGINT" dolt sql -q "UPDATE test SET c6=600 WHERE pk=0" dolt add test dolt commit -m "Added new column and updated it" - # Get CLI skinny diff output with schema changes - run dolt diff --skinny HEAD~1 - [ "$status" -eq 0 ] - cli_output="$output" + compare_dolt_diff "HEAD~1" "HEAD" "test" - # Get SQL table function skinny diff output with schema changes - run dolt sql -q "SELECT * FROM dolt_diff('--skinny', 'HEAD~1', 'HEAD', 'test')" - [ "$status" -eq 0 ] - sql_output="$output" - - # Both should show the new column c6 and any other changed columns - [[ "$cli_output" =~ 'pk' ]] || false - [[ "$cli_output" =~ 'c6' ]] || false - [[ "$sql_output" =~ 'pk' ]] || false - [[ "$sql_output" =~ 'c6' ]] || false - - # Test with row deletion dolt sql -q "DELETE FROM test WHERE pk=1" dolt add test dolt commit -m "Deleted a row" - # Get CLI skinny diff output with row deletion - run dolt diff --skinny HEAD~1 + compare_dolt_diff "HEAD~1" "HEAD" "test" + + run dolt sql -q "SELECT * FROM dolt_diff('-sk', 'HEAD~1', 'HEAD', 'test')" [ "$status" -eq 0 ] - cli_output="$output" - # Get SQL table function skinny diff output with row deletion - run dolt sql -q "SELECT * FROM dolt_diff('--skinny', 'HEAD~1', 'HEAD', 'test')" - [ "$status" -eq 0 ] - sql_output="$output" - - # Both should show all columns for deleted rows - [[ "$cli_output" =~ 'pk' ]] || false - [[ "$cli_output" =~ 'c1' ]] || false - [[ "$cli_output" =~ 'c2' ]] || false - [[ "$cli_output" =~ 'c3' ]] || false - [[ "$cli_output" =~ 'c4' ]] || false - [[ "$cli_output" =~ 'c5' ]] || false - [[ "$cli_output" =~ 'c6' ]] || false - - [[ "$sql_output" =~ 'pk' ]] || false - [[ "$sql_output" =~ 'c1' ]] || false - [[ "$sql_output" =~ 'c2' ]] || false - [[ "$sql_output" =~ 'c3' ]] || false - [[ "$sql_output" =~ 'c4' ]] || false - [[ "$sql_output" =~ 'c5' ]] || false - [[ "$sql_output" =~ 'c6' ]] || false + run dolt sql -q "SELECT * FROM dolt_diff('-err', 'HEAD~1', 'HEAD', 'test')" + [ "$status" -eq 1 ] } From 63bba7a0309390ad63074ecbf57c63f57db00172 Mon Sep 17 00:00:00 2001 From: elianddb Date: Thu, 4 Sep 2025 16:13:58 -0700 Subject: [PATCH 04/11] rm tmp wsl fix for bats test --- integration-tests/bats/helper/windows-compat.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/helper/windows-compat.bash b/integration-tests/bats/helper/windows-compat.bash index 344fbcb476..1305bbc3f1 100644 --- a/integration-tests/bats/helper/windows-compat.bash +++ b/integration-tests/bats/helper/windows-compat.bash @@ -5,7 +5,7 @@ skiponwindows() { :; } IS_WINDOWS=${IS_WINDOWS:-false} WINDOWS_BASE_DIR=${WINDOWS_BASE_DIR:-/mnt/c} -if [ ! -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then +if [ -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then IS_WINDOWS=true if [ ! -d "$WINDOWS_BASE_DIR"/batstmp ]; then mkdir "$WINDOWS_BASE_DIR"/batstmp From 668f3cb3070ef311b51d8ca3bf44c11413d42a23 Mon Sep 17 00:00:00 2001 From: elianddb Date: Fri, 5 Sep 2025 10:23:01 -0700 Subject: [PATCH 05/11] amend docs and fix options parse --- .../sqle/dtablefunctions/dolt_diff.go | 53 +++++++++++------- .../sqle/enginetest/dolt_queries_diff.go | 54 +++++++++++-------- integration-tests/bats/sql-diff.bats | 6 +++ 3 files changed, 73 insertions(+), 40 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 5de4633b83..f9f36df096 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -17,6 +17,7 @@ package dtablefunctions import ( "fmt" table2 "github.com/dolthub/dolt/go/libraries/doltcore/table" + "github.com/dolthub/go-mysql-server/sql/expression" "io" "strings" @@ -119,11 +120,11 @@ func (dtf *DiffTableFunction) Expressions() []sql.Expression { // WithExpressions implements the sql.Expressioner interface func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sql.Node, error) { newDtf := *dtf - var filteredExprs []sql.Expression + newDtf.skinnyExpr = nil // analyzer may call multiple times // TODO: For now, we will only support literal / fully-resolved arguments to the // DiffTableFunction to avoid issues where the schema is needed in the analyzer // before the arguments could be resolved. - for _, expr := range expressions { + for i, expr := range expressions { if !expr.Resolved() { return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } @@ -132,31 +133,45 @@ func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sq return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } - switch expr.String() { - case "'--skinny'", "'-sk'": - newDtf.skinnyExpr = expr - default: - filteredExprs = append(filteredExprs, expr) + // only support --skinny for now as a special flag, this should suffice until other flags are needed + if lit, ok := expr.(*expression.Literal); ok { + if s, ok := lit.Value().(string); ok { + flag := strings.ToLower(strings.TrimSpace(s)) + if flag == "--skinny" || flag == "-sk" { + if i != 0 { + return nil, sql.ErrInvalidArgumentDetails.New(dtf.Name(), "%s must be the first argument", expr.String()) + } + if newDtf.skinnyExpr != nil { + return nil, sql.ErrInvalidArgumentDetails.New(dtf.Name(), "duplicate %s", expr.String()) + } + newDtf.skinnyExpr = expr + continue + } + } } } - if len(filteredExprs) < 2 { - return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 3", len(filteredExprs)) + if newDtf.skinnyExpr != nil { + expressions = expressions[1:] } - if strings.Contains(filteredExprs[0].String(), "..") { - if len(filteredExprs) != 2 { - return nil, sql.ErrInvalidArgumentNumber.New(fmt.Sprintf("%v with .. or ...", newDtf.Name()), 2, len(filteredExprs)) + if len(expressions) < 2 { + return nil, sql.ErrInvalidArgumentNumber.New(dtf.Name(), "2 to 3", len(expressions)) + } + + if strings.Contains(expressions[0].String(), "..") { + if len(expressions) != 2 { + return nil, sql.ErrInvalidArgumentNumber.New(fmt.Sprintf("%v with .. or ...", newDtf.Name()), 2, len(expressions)) } - newDtf.dotCommitExpr = filteredExprs[0] - newDtf.tableNameExpr = filteredExprs[1] + newDtf.dotCommitExpr = expressions[0] + newDtf.tableNameExpr = expressions[1] } else { - if len(filteredExprs) != 3 { - return nil, sql.ErrInvalidArgumentNumber.New(newDtf.Name(), 3, len(filteredExprs)) + if len(expressions) != 3 { + return nil, sql.ErrInvalidArgumentNumber.New(newDtf.Name(), 3, len(expressions)) } - newDtf.fromCommitExpr = filteredExprs[0] - newDtf.toCommitExpr = filteredExprs[1] - newDtf.tableNameExpr = filteredExprs[2] + newDtf.fromCommitExpr = expressions[0] + newDtf.toCommitExpr = expressions[1] + newDtf.tableNameExpr = expressions[2] } fromCommitVal, toCommitVal, dotCommitVal, tableName, err := newDtf.evaluateArguments() diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go index 9a0fd20c1f..645a29ca8d 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go @@ -19,6 +19,7 @@ import ( "github.com/dolthub/go-mysql-server/enginetest/queries" "github.com/dolthub/go-mysql-server/sql" gmstypes "github.com/dolthub/go-mysql-server/sql/types" + "strings" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtablefunctions" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtables" @@ -819,20 +820,31 @@ var Dolt1DiffSystemTableScripts = []queries.ScriptTest{ }, } -func countDataColsAssertions(viewName, diffSelectStar string, expected int64) []queries.ScriptTestAssertion { +// assertDoltDiffColumnCount returns assertions that verify a dolt_diff view +// has the expected number of distinct data columns (excluding commit metadata). +func assertDoltDiffColumnCount(view, selectStmt string, expected int64) []queries.ScriptTestAssertion { + excluded := []string{ + "'to_commit'", + "'from_commit'", + "'to_commit_date'", + "'from_commit_date'", + "'diff_type'", + } + + query := fmt.Sprintf(` + SELECT COUNT(DISTINCT REPLACE(REPLACE(column_name, 'to_', ''), 'from_', '')) + FROM information_schema.columns + WHERE table_schema = DATABASE() + AND table_name = '%s' + AND column_name NOT IN (%s)`, + view, strings.Join(excluded, ", "), + ) + return []queries.ScriptTestAssertion{ - {Query: fmt.Sprintf("DROP VIEW IF EXISTS %s;", viewName)}, - {Query: fmt.Sprintf("CREATE VIEW %s AS %s;", viewName, diffSelectStar)}, - { - Query: fmt.Sprintf(` - SELECT COUNT(DISTINCT REPLACE(REPLACE(column_name,'to_',''),'from_','')) - FROM information_schema.columns - WHERE table_schema = DATABASE() - AND table_name = '%s' - AND column_name NOT IN ('to_commit','from_commit','to_commit_date','from_commit_date','diff_type')`, viewName), - Expected: []sql.Row{{expected}}, - }, - {Query: fmt.Sprintf("DROP VIEW %s;", viewName)}, + {Query: fmt.Sprintf("DROP VIEW IF EXISTS %s;", view)}, + {Query: fmt.Sprintf("CREATE VIEW %s AS %s;", view, selectStmt)}, + {Query: query, Expected: []sql.Row{{expected}}}, + {Query: fmt.Sprintf("DROP VIEW %s;", view)}, } } @@ -918,14 +930,14 @@ var DiffTableFunctionScriptTests = []queries.ScriptTest{ }, }, } - asserts = append(asserts, countDataColsAssertions("v_all_01", "SELECT * FROM dolt_diff(@C0, @C1, 't')", 6)...) - asserts = append(asserts, countDataColsAssertions("v_skinny_01", "SELECT * FROM dolt_diff('--skinny', @C0, @C1, 't')", 6)...) - asserts = append(asserts, countDataColsAssertions("v_all_12", "SELECT * FROM dolt_diff(@C1, @C2, 't')", 6)...) - asserts = append(asserts, countDataColsAssertions("v_skinny_12", "SELECT * FROM dolt_diff('--skinny', @C1, @C2, 't')", 4)...) - asserts = append(asserts, countDataColsAssertions("v_all_23", "SELECT * FROM dolt_diff(@C2, @C3, 't')", 7)...) - asserts = append(asserts, countDataColsAssertions("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', @C2, @C3, 't')", 2)...) - asserts = append(asserts, countDataColsAssertions("v_all_34", "SELECT * FROM dolt_diff(@C3, @C4, 't')", 7)...) - asserts = append(asserts, countDataColsAssertions("v_skinny_34", "SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')", 7)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_all_01", "SELECT * FROM dolt_diff(@C0, @C1, 't')", 6)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_01", "SELECT * FROM dolt_diff('--skinny', @C0, @C1, 't')", 6)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_all_12", "SELECT * FROM dolt_diff(@C1, @C2, 't')", 6)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_12", "SELECT * FROM dolt_diff('--skinny', @C1, @C2, 't')", 4)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_all_23", "SELECT * FROM dolt_diff(@C2, @C3, 't')", 7)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', @C2, @C3, 't')", 2)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_all_34", "SELECT * FROM dolt_diff(@C3, @C4, 't')", 7)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_34", "SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')", 7)...) return asserts }(), diff --git a/integration-tests/bats/sql-diff.bats b/integration-tests/bats/sql-diff.bats index 4d164e80b2..4a440d4f71 100644 --- a/integration-tests/bats/sql-diff.bats +++ b/integration-tests/bats/sql-diff.bats @@ -935,4 +935,10 @@ SQL run dolt sql -q "SELECT * FROM dolt_diff('-err', 'HEAD~1', 'HEAD', 'test')" [ "$status" -eq 1 ] + + run dolt sql -q "SELECT * FROM dolt_diff('-sk', '--skinny', 'HEAD~1', 'HEAD', 'test')" + [ "$status" -eq 1 ] + + run dolt sql -q "SELECT * FROM dolt_diff('HEAD~1', 'HEAD', 'test', '-sk')" + [ "$status" -eq 1 ] } From 94ba17826679d1e5e9dd99f3b8f62ff034b3dafe Mon Sep 17 00:00:00 2001 From: elianddb Date: Fri, 5 Sep 2025 10:57:13 -0700 Subject: [PATCH 06/11] rm extra change --- go/libraries/doltcore/sqle/dtables/diff_table.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/libraries/doltcore/sqle/dtables/diff_table.go b/go/libraries/doltcore/sqle/dtables/diff_table.go index be70f5f345..a60b52a062 100644 --- a/go/libraries/doltcore/sqle/dtables/diff_table.go +++ b/go/libraries/doltcore/sqle/dtables/diff_table.go @@ -917,6 +917,11 @@ func GetDiffTableSchemaAndJoiner(format *types.NomsBinFormat, fromSch, toSch sch return nil, nil, err } } else { + fromSch, toSch, err = expandFromToSchemas(fromSch, toSch) + if err != nil { + return nil, nil, err + } + j, err = rowconv.NewJoiner( []rowconv.NamedSchema{{Name: diff.To, Sch: toSch}, {Name: diff.From, Sch: fromSch}}, map[string]rowconv.ColNamingFunc{ From 1420e9f70e478d92eef18bb242672d73fe689452 Mon Sep 17 00:00:00 2001 From: elianddb Date: Sat, 6 Sep 2025 19:51:33 -0700 Subject: [PATCH 07/11] amend impl of skinny cols filter --- go/cmd/dolt/cli/arg_parser_helpers.go | 12 ++++ .../sqle/dtablefunctions/dolt_diff.go | 66 +++++++++---------- .../bats/helper/windows-compat.bash | 2 +- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 4d8e5e5237..119f37de51 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -308,6 +308,18 @@ func CreateLogArgParser(isTableFunction bool) *argparser.ArgParser { return ap } +const ( + SkinnyFlag = "skinny" + IncludeColsFlag = "include-cols" +) + +func CreateDiffArgParser() *argparser.ArgParser { + ap := argparser.NewArgParserWithVariableArgs("DOLT_DIFF()") + ap.SupportsFlag(SkinnyFlag, "sk", "Shows only primary key columns and any columns with data changes.") + ap.SupportsString(IncludeColsFlag, "", "columns", "Comma-separated list of columns to include in the diff output.") + return ap +} + func CreateGCArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("gc", 0) ap.SupportsFlag(ShallowFlag, "s", "perform a fast, but incomplete garbage collection pass") diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index f9f36df096..afe7e1e32a 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -16,7 +16,7 @@ package dtablefunctions import ( "fmt" - table2 "github.com/dolthub/dolt/go/libraries/doltcore/table" + dolttable "github.com/dolthub/dolt/go/libraries/doltcore/table" "github.com/dolthub/go-mysql-server/sql/expression" "io" "strings" @@ -452,26 +452,24 @@ func (dtf *DiffTableFunction) evaluateArguments() (interface{}, interface{}, int return fromCommitVal, toCommitVal, nil, tableName, nil } -// FilterDeltaSchemaToSkinnyCols creates a filtered version of the table delta that omits columns which are identical +// filterDeltaSchemaToSkinnyCols creates a filtered version of the table delta that omits columns which are identical // in type and value across all rows in both schemas, except for primary key columns which are always included. Also // updates dtf.tableDelta with the filtered result. -func (dtf *DiffTableFunction) FilterDeltaSchemaToSkinnyCols(ctx *sql.Context, delta diff.TableDelta) (diff.TableDelta, error) { +func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, delta *diff.TableDelta) (diff.TableDelta, error) { if delta.FromTable == nil || delta.ToTable == nil { - return delta, nil + return *delta, nil } - // potential cols for omission: - fromEqualColsIndices := map[string]int{} - toEqualColsIndices := map[string]int{} - for fromI, fromCol := range delta.FromSch.GetAllCols().GetColumns() { + // potential cols for omission + equalDiffColsIndices := map[string][2]int{} + for fi, fromCol := range delta.FromSch.GetAllCols().GetColumns() { col, ok := delta.ToSch.GetAllCols().GetByName(fromCol.Name) if !ok { // column was dropped continue } if fromCol.TypeInfo.Equals(col.TypeInfo) { - fromEqualColsIndices[fromCol.Name] = fromI - toEqualColsIndices[col.Name] = delta.ToSch.GetAllCols().IndexOf(col.Name) + equalDiffColsIndices[fromCol.Name] = [2]int{fi, delta.ToSch.GetAllCols().IndexOf(col.Name)} } } @@ -485,19 +483,19 @@ func (dtf *DiffTableFunction) FilterDeltaSchemaToSkinnyCols(ctx *sql.Context, de return diff.TableDelta{}, err } - fromIter, err := table2.NewTableIterator(ctx, delta.FromSch, fromRowData) + fromIter, err := dolttable.NewTableIterator(ctx, delta.FromSch, fromRowData) if err != nil { return diff.TableDelta{}, err } defer fromIter.Close(ctx) - toIter, err := table2.NewTableIterator(ctx, delta.ToSch, toRowData) + toIter, err := dolttable.NewTableIterator(ctx, delta.ToSch, toRowData) if err != nil { return diff.TableDelta{}, err } defer toIter.Close(ctx) - for { + for len(equalDiffColsIndices) > 0 { fromRow, fromErr := fromIter.Next(ctx) toRow, toErr := toIter.Next(ctx) @@ -515,8 +513,7 @@ func (dtf *DiffTableFunction) FilterDeltaSchemaToSkinnyCols(ctx *sql.Context, de // xor: if only one is nil, then all cols are diffs if (fromRow == nil) != (toRow == nil) { - fromEqualColsIndices = map[string]int{} - toEqualColsIndices = map[string]int{} + equalDiffColsIndices = map[string][2]int{} break } @@ -524,40 +521,39 @@ func (dtf *DiffTableFunction) FilterDeltaSchemaToSkinnyCols(ctx *sql.Context, de continue } - for colName, fromI := range fromEqualColsIndices { - toI := toEqualColsIndices[colName] - fromVal := fromRow[fromI] - toVal := toRow[toI] - - if fromVal != toVal { + for colName, idx := range equalDiffColsIndices { + const ( + from = 0 + to = 1 + ) + if fromRow[idx[from]] != toRow[idx[to]] { // rm from the diff cols, since we found a diff - delete(fromEqualColsIndices, colName) - delete(toEqualColsIndices, colName) + delete(equalDiffColsIndices, colName) } } } - var fromSkinnyCols []schema.Column + var fromSkCols []schema.Column for _, col := range delta.FromSch.GetAllCols().GetColumns() { - _, ok := fromEqualColsIndices[col.Name] + _, ok := equalDiffColsIndices[col.Name] if col.IsPartOfPK || !ok { - fromSkinnyCols = append(fromSkinnyCols, col) + fromSkCols = append(fromSkCols, col) } } - var toSkinnyCols []schema.Column + var toSkCols []schema.Column for _, col := range delta.ToSch.GetAllCols().GetColumns() { - _, ok := toEqualColsIndices[col.Name] + _, ok := equalDiffColsIndices[col.Name] if col.IsPartOfPK || !ok { - toSkinnyCols = append(toSkinnyCols, col) + toSkCols = append(toSkCols, col) } } - skinnyDelta := delta - skinnyDelta.FromSch = schema.MustSchemaFromCols(schema.NewColCollection(fromSkinnyCols...)) - skinnyDelta.ToSch = schema.MustSchemaFromCols(schema.NewColCollection(toSkinnyCols...)) - dtf.tableDelta = skinnyDelta - return skinnyDelta, nil + skDelta := *delta + skDelta.FromSch = schema.MustSchemaFromCols(schema.NewColCollection(fromSkCols...)) + skDelta.ToSch = schema.MustSchemaFromCols(schema.NewColCollection(toSkCols...)) + dtf.tableDelta = skDelta + return skDelta, nil } func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, toCommitVal, dotCommitVal interface{}, tableName string) error { @@ -576,7 +572,7 @@ func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, to } if dtf.skinnyExpr != nil { - delta, err = dtf.FilterDeltaSchemaToSkinnyCols(ctx, delta) + delta, err = dtf.filterDeltaSchemaToSkinnyCols(ctx, &delta) if err != nil { return err } diff --git a/integration-tests/bats/helper/windows-compat.bash b/integration-tests/bats/helper/windows-compat.bash index 1305bbc3f1..344fbcb476 100644 --- a/integration-tests/bats/helper/windows-compat.bash +++ b/integration-tests/bats/helper/windows-compat.bash @@ -5,7 +5,7 @@ skiponwindows() { :; } IS_WINDOWS=${IS_WINDOWS:-false} WINDOWS_BASE_DIR=${WINDOWS_BASE_DIR:-/mnt/c} -if [ -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then +if [ ! -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then IS_WINDOWS=true if [ ! -d "$WINDOWS_BASE_DIR"/batstmp ]; then mkdir "$WINDOWS_BASE_DIR"/batstmp From fadb0af405f3b9c8f7ca933298b69e40ee255fd9 Mon Sep 17 00:00:00 2001 From: elianddb Date: Mon, 8 Sep 2025 10:26:40 -0700 Subject: [PATCH 08/11] add --include-cols option, cli arg parsing, and amend tests --- go/cmd/dolt/cli/arg_parser_helpers.go | 27 +++-- go/cmd/dolt/cli/flags.go | 16 +++ go/cmd/dolt/commands/diff.go | 81 +++++-------- go/cmd/dolt/commands/show.go | 22 ++-- .../sqle/dtablefunctions/dolt_diff.go | 110 +++++++++--------- .../sqle/enginetest/dolt_queries_diff.go | 28 +++-- integration-tests/bats/helper/sql-diff.bash | 84 ++++++------- .../bats/helper/windows-compat.bash | 2 +- integration-tests/bats/sql-diff.bats | 18 ++- 9 files changed, 203 insertions(+), 185 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 119f37de51..8b7115aea6 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -308,15 +308,26 @@ func CreateLogArgParser(isTableFunction bool) *argparser.ArgParser { return ap } -const ( - SkinnyFlag = "skinny" - IncludeColsFlag = "include-cols" -) - -func CreateDiffArgParser() *argparser.ArgParser { - ap := argparser.NewArgParserWithVariableArgs("DOLT_DIFF()") +func CreateDiffArgParser(isTableFunction bool) *argparser.ArgParser { + ap := argparser.NewArgParserWithVariableArgs("diff") ap.SupportsFlag(SkinnyFlag, "sk", "Shows only primary key columns and any columns with data changes.") - ap.SupportsString(IncludeColsFlag, "", "columns", "Comma-separated list of columns to include in the diff output.") + ap.SupportsStringList(IncludeCols, "ic", "columns", "A list of columns to include in the diff.") + if !isTableFunction { // TODO: support for table function + 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(StatFlag, "", "Show stats of data changes") + ap.SupportsFlag(SummaryFlag, "", "Show summary of data and schema changes") + 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(StagedFlag, "", "Show only the staged data changes.") + ap.SupportsFlag(CachedFlag, "c", "Synonym for --staged") + ap.SupportsFlag(MergeBase, "", "Uses merge base of the first commit and second commit (or HEAD if not supplied) as the first commit") + ap.SupportsString(DiffMode, "", "diff mode", "Determines how to display modified rows with tabular output. Valid values are row, line, in-place, context. Defaults to context.") + ap.SupportsFlag(ReverseFlag, "R", "Reverses the direction of the diff.") + ap.SupportsFlag(NameOnlyFlag, "", "Only shows table names.") + ap.SupportsFlag(SystemFlag, "", "Show system tables in addition to user tables") + } return ap } diff --git a/go/cmd/dolt/cli/flags.go b/go/cmd/dolt/cli/flags.go index 5eddc811d9..691ddbd973 100644 --- a/go/cmd/dolt/cli/flags.go +++ b/go/cmd/dolt/cli/flags.go @@ -88,3 +88,19 @@ const ( UpperCaseAllFlag = "ALL" UserFlag = "user" ) + +// Flags used by `dolt diff` command and `dolt_diff()` table function. +const ( + SkinnyFlag = "skinny" + IncludeCols = "include-cols" + DataFlag = "data" + SchemaFlag = "schema" + NameOnlyFlag = "name-only" + SummaryFlag = "summary" + WhereParam = "where" + LimitParam = "limit" + MergeBase = "merge-base" + DiffMode = "diff-mode" + ReverseFlag = "reverse" + FormatFlag = "result-format" +) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 68e57bb572..e25416d009 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -60,18 +60,6 @@ const ( TabularDiffOutput diffOutput = 1 SQLDiffOutput diffOutput = 2 JsonDiffOutput diffOutput = 3 - - DataFlag = "data" - SchemaFlag = "schema" - NameOnlyFlag = "name-only" - StatFlag = "stat" - SummaryFlag = "summary" - whereParam = "where" - limitParam = "limit" - SkinnyFlag = "skinny" - MergeBase = "merge-base" - DiffMode = "diff-mode" - ReverseFlag = "reverse" ) var diffDocs = cli.CommandDocumentationContent{ @@ -107,12 +95,13 @@ The {{.EmphasisLeft}}--diff-mode{{.EmphasisRight}} argument controls how modifie } type diffDisplaySettings struct { - diffParts diffPart - diffOutput diffOutput - diffMode diff.Mode - limit int - where string - skinny bool + diffParts diffPart + diffOutput diffOutput + diffMode diff.Mode + limit int + where string + skinny bool + includeCols []string } type diffDatasets struct { @@ -164,23 +153,7 @@ func (cmd DiffCmd) Docs() *cli.CommandDocumentation { } func (cmd DiffCmd) ArgParser() *argparser.ArgParser { - ap := argparser.NewArgParserWithVariableArgs(cmd.Name()) - 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(StatFlag, "", "Show stats of data changes") - ap.SupportsFlag(SummaryFlag, "", "Show summary of data and schema changes") - 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(cli.StagedFlag, "", "Show only the staged data changes.") - ap.SupportsFlag(cli.CachedFlag, "c", "Synonym for --staged") - ap.SupportsFlag(SkinnyFlag, "sk", "Shows only primary key columns and any columns with data changes.") - ap.SupportsFlag(MergeBase, "", "Uses merge base of the first commit and second commit (or HEAD if not supplied) as the first commit") - ap.SupportsString(DiffMode, "", "diff mode", "Determines how to display modified rows with tabular output. Valid values are row, line, in-place, context. Defaults to context.") - ap.SupportsFlag(ReverseFlag, "R", "Reverses the direction of the diff.") - ap.SupportsFlag(NameOnlyFlag, "", "Only shows table names.") - ap.SupportsFlag(cli.SystemFlag, "", "Show system tables in addition to user tables") - return ap + return cli.CreateDiffArgParser(false) } func (cmd DiffCmd) RequiresRepo() bool { @@ -228,14 +201,14 @@ func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _ } func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseError { - if apr.Contains(StatFlag) || apr.Contains(SummaryFlag) { - if apr.Contains(SchemaFlag) || apr.Contains(DataFlag) { + if apr.Contains(cli.StatFlag) || apr.Contains(cli.SummaryFlag) { + if apr.Contains(cli.SchemaFlag) || apr.Contains(cli.DataFlag) { return errhand.BuildDError("invalid Arguments: --stat and --summary cannot be combined with --schema or --data").Build() } } - if apr.Contains(NameOnlyFlag) { - if apr.Contains(SchemaFlag) || apr.Contains(DataFlag) || apr.Contains(StatFlag) || apr.Contains(SummaryFlag) { + if apr.Contains(cli.NameOnlyFlag) { + if apr.Contains(cli.SchemaFlag) || apr.Contains(cli.DataFlag) || apr.Contains(cli.StatFlag) || apr.Contains(cli.SummaryFlag) { return errhand.BuildDError("invalid Arguments: --name-only cannot be combined with --schema, --data, --stat, or --summary").Build() } } @@ -254,25 +227,29 @@ func parseDiffDisplaySettings(apr *argparser.ArgParseResults) *diffDisplaySettin displaySettings := &diffDisplaySettings{} displaySettings.diffParts = SchemaAndDataDiff - if apr.Contains(DataFlag) && !apr.Contains(SchemaFlag) { + if apr.Contains(cli.DataFlag) && !apr.Contains(cli.SchemaFlag) { displaySettings.diffParts = DataOnlyDiff - } else if apr.Contains(SchemaFlag) && !apr.Contains(DataFlag) { + } else if apr.Contains(cli.SchemaFlag) && !apr.Contains(cli.DataFlag) { displaySettings.diffParts = SchemaOnlyDiff - } else if apr.Contains(StatFlag) { + } else if apr.Contains(cli.StatFlag) { displaySettings.diffParts = Stat - } else if apr.Contains(SummaryFlag) { + } else if apr.Contains(cli.SummaryFlag) { displaySettings.diffParts = Summary - } else if apr.Contains(NameOnlyFlag) { + } else if apr.Contains(cli.NameOnlyFlag) { displaySettings.diffParts = NameOnlyDiff } - displaySettings.skinny = apr.Contains(SkinnyFlag) + displaySettings.skinny = apr.Contains(cli.SkinnyFlag) + + if cols, ok := apr.GetValueList(cli.IncludeCols); ok { + displaySettings.includeCols = cols + } f := apr.GetValueOrDefault(FormatFlag, "tabular") switch strings.ToLower(f) { case "tabular": displaySettings.diffOutput = TabularDiffOutput - switch strings.ToLower(apr.GetValueOrDefault(DiffMode, "context")) { + switch strings.ToLower(apr.GetValueOrDefault(cli.DiffMode, "context")) { case "row": displaySettings.diffMode = diff.ModeRow case "line": @@ -288,8 +265,8 @@ func parseDiffDisplaySettings(apr *argparser.ArgParseResults) *diffDisplaySettin displaySettings.diffOutput = JsonDiffOutput } - displaySettings.limit, _ = apr.GetInt(limitParam) - displaySettings.where = apr.GetValueOrDefault(whereParam, "") + displaySettings.limit, _ = apr.GetInt(cli.LimitParam) + displaySettings.where = apr.GetValueOrDefault(cli.WhereParam, "") return displaySettings } @@ -301,12 +278,12 @@ func parseDiffArgs(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.Ar staged := apr.Contains(cli.StagedFlag) || apr.Contains(cli.CachedFlag) - tableNames, err := dArgs.applyDiffRoots(queryist, sqlCtx, apr.Args, staged, apr.Contains(MergeBase)) + tableNames, err := dArgs.applyDiffRoots(queryist, sqlCtx, apr.Args, staged, apr.Contains(cli.MergeBase)) if err != nil { return nil, err } - if apr.Contains(ReverseFlag) { + if apr.Contains(cli.ReverseFlag) { dArgs.diffDatasets = &diffDatasets{ fromRef: dArgs.toRef, toRef: dArgs.fromRef, @@ -1556,7 +1533,9 @@ func diffRows( if err != nil { return errhand.BuildDError("Error running diff query:\n%s", interpolatedQuery).AddCause(err).Build() } - + for _, col := range dArgs.includeCols { + modifiedColNames[col] = true // ensure included columns are always present + } // instantiate a new schema that only contains the columns with changes var filteredUnionSch sql.Schema for _, s := range unionSch { diff --git a/go/cmd/dolt/commands/show.go b/go/cmd/dolt/commands/show.go index bc86246d53..858bef9740 100644 --- a/go/cmd/dolt/commands/show.go +++ b/go/cmd/dolt/commands/show.go @@ -83,17 +83,17 @@ func (cmd ShowCmd) ArgParser() *argparser.ArgParser { ap.SupportsFlag(cli.NoPrettyFlag, "", "Show the object without making it pretty.") // Flags inherited from Diff - 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(StatFlag, "", "Show stats of data changes") - ap.SupportsFlag(SummaryFlag, "", "Show summary of data and schema changes") + ap.SupportsFlag(cli.DataFlag, "d", "Show only the data changes, do not show the schema changes (Both shown by default).") + ap.SupportsFlag(cli.SchemaFlag, "s", "Show only the schema changes, do not show the data changes (Both shown by default).") + ap.SupportsFlag(cli.StatFlag, "", "Show stats of data changes") + ap.SupportsFlag(cli.SummaryFlag, "", "Show summary of data and schema changes") 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.SupportsString(cli.WhereParam, "", "column", "filters columns based on values in the diff. See {{.EmphasisLeft}}dolt diff --help{{.EmphasisRight}} for details.") + ap.SupportsInt(cli.LimitParam, "", "record_count", "limits to the first N diffs.") ap.SupportsFlag(cli.CachedFlag, "c", "Show only the staged data changes.") - ap.SupportsFlag(SkinnyFlag, "sk", "Shows only primary key columns and any columns with data changes.") - ap.SupportsFlag(MergeBase, "", "Uses merge base of the first commit and second commit (or HEAD if not supplied) as the first commit") - ap.SupportsString(DiffMode, "", "diff mode", "Determines how to display modified rows with tabular output. Valid values are row, line, in-place, context. Defaults to context.") + ap.SupportsFlag(cli.SkinnyFlag, "sk", "Shows only primary key columns and any columns with data changes.") + ap.SupportsFlag(cli.MergeBase, "", "Uses merge base of the first commit and second commit (or HEAD if not supplied) as the first commit") + ap.SupportsString(cli.DiffMode, "", "diff mode", "Determines how to display modified rows with tabular output. Valid values are row, line, in-place, context. Defaults to context.") return ap } @@ -275,8 +275,8 @@ func getValueFromRefSpec(ctx context.Context, dEnv *env.DoltEnv, specRef string) } func (cmd ShowCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseError { - if apr.Contains(StatFlag) || apr.Contains(SummaryFlag) { - if apr.Contains(SchemaFlag) || apr.Contains(DataFlag) { + if apr.Contains(cli.StatFlag) || apr.Contains(cli.SummaryFlag) { + if apr.Contains(cli.SchemaFlag) || apr.Contains(cli.DataFlag) { return errhand.BuildDError("invalid Arguments: --stat and --summary cannot be combined with --schema or --data").Build() } } diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index afe7e1e32a..d3ad6dd7bf 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -16,6 +16,7 @@ package dtablefunctions import ( "fmt" + "github.com/dolthub/dolt/go/cmd/dolt/cli" dolttable "github.com/dolthub/dolt/go/libraries/doltcore/table" "github.com/dolthub/go-mysql-server/sql/expression" "io" @@ -49,7 +50,6 @@ var _ sql.AuthorizationCheckerNode = (*DiffTableFunction)(nil) type DiffTableFunction struct { tableDelta diff.TableDelta - skinnyExpr sql.Expression fromCommitExpr sql.Expression toCommitExpr sql.Expression dotCommitExpr sql.Expression @@ -60,6 +60,8 @@ type DiffTableFunction struct { fromDate *types.Timestamp toDate *types.Timestamp sqlSch sql.Schema + showSkinny bool + includeCols map[string]struct{} } // NewInstance creates a new instance of TableFunction interface @@ -105,9 +107,6 @@ func (dtf *DiffTableFunction) WithDatabase(database sql.Database) (sql.Node, err // Expressions implements the sql.Expressioner interface func (dtf *DiffTableFunction) Expressions() []sql.Expression { exprs := []sql.Expression{} - if dtf.skinnyExpr != nil { - exprs = append(exprs, dtf.skinnyExpr) - } if dtf.dotCommitExpr != nil { exprs = append(exprs, dtf.dotCommitExpr, dtf.tableNameExpr) @@ -120,11 +119,12 @@ func (dtf *DiffTableFunction) Expressions() []sql.Expression { // WithExpressions implements the sql.Expressioner interface func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sql.Node, error) { newDtf := *dtf - newDtf.skinnyExpr = nil // analyzer may call multiple times // TODO: For now, we will only support literal / fully-resolved arguments to the // DiffTableFunction to avoid issues where the schema is needed in the analyzer // before the arguments could be resolved. - for i, expr := range expressions { + var literalArgs []string + strToExpr := map[string]sql.Expression{} + for _, expr := range expressions { if !expr.Resolved() { return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } @@ -132,27 +132,31 @@ func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sq if _, ok := expr.(sql.FunctionExpression); ok { return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } + strVal := expr.String() + if lit, ok := expr.(*expression.Literal); ok { // we only support literals for parsing + strVal = fmt.Sprintf("%v", lit.Value()) + literalArgs = append(literalArgs, strVal) + } + strToExpr[strVal] = expr + } - // only support --skinny for now as a special flag, this should suffice until other flags are needed - if lit, ok := expr.(*expression.Literal); ok { - if s, ok := lit.Value().(string); ok { - flag := strings.ToLower(strings.TrimSpace(s)) - if flag == "--skinny" || flag == "-sk" { - if i != 0 { - return nil, sql.ErrInvalidArgumentDetails.New(dtf.Name(), "%s must be the first argument", expr.String()) - } - if newDtf.skinnyExpr != nil { - return nil, sql.ErrInvalidArgumentDetails.New(dtf.Name(), "duplicate %s", expr.String()) - } - newDtf.skinnyExpr = expr - continue - } - } + apr, err := cli.CreateDiffArgParser(true).Parse(literalArgs) + if err != nil { + return nil, err + } + + newDtf.showSkinny = newDtf.showSkinny || apr.Contains(cli.SkinnyFlag) + + if cols, ok := apr.GetValueList(cli.IncludeCols); newDtf.includeCols == nil && ok { + newDtf.includeCols = make(map[string]struct{}) + for _, col := range cols { + newDtf.includeCols[col] = struct{}{} } } - if newDtf.skinnyExpr != nil { - expressions = expressions[1:] + expressions = []sql.Expression{} + for _, posArg := range apr.Args { + expressions = append(expressions, strToExpr[posArg]) } if len(expressions) < 2 { @@ -453,45 +457,50 @@ func (dtf *DiffTableFunction) evaluateArguments() (interface{}, interface{}, int } // filterDeltaSchemaToSkinnyCols creates a filtered version of the table delta that omits columns which are identical -// in type and value across all rows in both schemas, except for primary key columns which are always included. Also -// updates dtf.tableDelta with the filtered result. -func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, delta *diff.TableDelta) (diff.TableDelta, error) { +// in type and value across all rows in both schemas, except for primary key columns or explicitly included using the +// include-cols option. This also updates dtf.tableDelta with the filtered result. +func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, delta *diff.TableDelta) (*diff.TableDelta, error) { if delta.FromTable == nil || delta.ToTable == nil { - return *delta, nil + return delta, nil } - // potential cols for omission + // gather map of potential cols for removal from skinny diff equalDiffColsIndices := map[string][2]int{} - for fi, fromCol := range delta.FromSch.GetAllCols().GetColumns() { + toCols := delta.ToSch.GetAllCols() + for fromIdx, fromCol := range delta.FromSch.GetAllCols().GetColumns() { + if _, ok := dtf.includeCols[fromCol.Name]; ok { + continue // user explicitly included this column + } + col, ok := delta.ToSch.GetAllCols().GetByName(fromCol.Name) if !ok { // column was dropped continue } - if fromCol.TypeInfo.Equals(col.TypeInfo) { - equalDiffColsIndices[fromCol.Name] = [2]int{fi, delta.ToSch.GetAllCols().IndexOf(col.Name)} + toIdx := toCols.TagToIdx[toCols.NameToCol[fromCol.Name].Tag] + equalDiffColsIndices[fromCol.Name] = [2]int{fromIdx, toIdx} } } fromRowData, err := delta.FromTable.GetRowData(ctx) if err != nil { - return diff.TableDelta{}, err + return nil, err } toRowData, err := delta.ToTable.GetRowData(ctx) if err != nil { - return diff.TableDelta{}, err + return nil, err } fromIter, err := dolttable.NewTableIterator(ctx, delta.FromSch, fromRowData) if err != nil { - return diff.TableDelta{}, err + return nil, err } defer fromIter.Close(ctx) toIter, err := dolttable.NewTableIterator(ctx, delta.ToSch, toRowData) if err != nil { - return diff.TableDelta{}, err + return nil, err } defer toIter.Close(ctx) @@ -504,11 +513,11 @@ func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, de } if fromErr != nil && fromErr != io.EOF { - return diff.TableDelta{}, fromErr + return nil, fromErr } if toErr != nil && toErr != io.EOF { - return diff.TableDelta{}, toErr + return nil, toErr } // xor: if only one is nil, then all cols are diffs @@ -522,12 +531,7 @@ func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, de } for colName, idx := range equalDiffColsIndices { - const ( - from = 0 - to = 1 - ) - if fromRow[idx[from]] != toRow[idx[to]] { - // rm from the diff cols, since we found a diff + if fromRow[idx[0]] != toRow[idx[1]] { // same row and col, values differ delete(equalDiffColsIndices, colName) } } @@ -553,7 +557,7 @@ func (dtf *DiffTableFunction) filterDeltaSchemaToSkinnyCols(ctx *sql.Context, de skDelta.FromSch = schema.MustSchemaFromCols(schema.NewColCollection(fromSkCols...)) skDelta.ToSch = schema.MustSchemaFromCols(schema.NewColCollection(toSkCols...)) dtf.tableDelta = skDelta - return skDelta, nil + return &skDelta, nil } func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, toCommitVal, dotCommitVal interface{}, tableName string) error { @@ -571,11 +575,12 @@ func (dtf *DiffTableFunction) generateSchema(ctx *sql.Context, fromCommitVal, to return err } - if dtf.skinnyExpr != nil { - delta, err = dtf.filterDeltaSchemaToSkinnyCols(ctx, &delta) + if dtf.showSkinny { + skDelta, err := dtf.filterDeltaSchemaToSkinnyCols(ctx, &delta) if err != nil { return err } + delta = *skDelta } fromTable, fromTableExists := delta.FromTable, delta.FromTable != nil @@ -691,15 +696,10 @@ func (dtf *DiffTableFunction) Schema() sql.Schema { // Resolved implements the sql.Resolvable interface func (dtf *DiffTableFunction) Resolved() bool { - resolved := true - if dtf.skinnyExpr != nil { - resolved = dtf.skinnyExpr.Resolved() - } - if dtf.dotCommitExpr != nil { - return resolved && dtf.tableNameExpr.Resolved() && dtf.dotCommitExpr.Resolved() + return dtf.tableNameExpr.Resolved() && dtf.dotCommitExpr.Resolved() } - return resolved && dtf.tableNameExpr.Resolved() && dtf.fromCommitExpr.Resolved() && dtf.toCommitExpr.Resolved() + return dtf.tableNameExpr.Resolved() && dtf.fromCommitExpr.Resolved() && dtf.toCommitExpr.Resolved() } func (dtf *DiffTableFunction) IsReadOnly() bool { @@ -709,10 +709,6 @@ func (dtf *DiffTableFunction) IsReadOnly() bool { // String implements the Stringer interface func (dtf *DiffTableFunction) String() string { args := []string{} - if dtf.skinnyExpr != nil { - args = append(args, dtf.skinnyExpr.String()) - } - if dtf.dotCommitExpr != nil { args = append(args, dtf.dotCommitExpr.String(), dtf.tableNameExpr.String()) } else { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go index 645a29ca8d..a2c1ef6c55 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go @@ -852,15 +852,15 @@ var DiffTableFunctionScriptTests = []queries.ScriptTest{ { Name: "dolt_diff: SELECT * skinny schema visibility", SetUpScript: []string{ - "CREATE TABLE t (" + - "pk BIGINT NOT NULL COMMENT 'tag:0'," + - "c1 BIGINT COMMENT 'tag:1'," + - "c2 BIGINT COMMENT 'tag:2'," + - "c3 BIGINT COMMENT 'tag:3'," + - "c4 BIGINT COMMENT 'tag:4'," + - "c5 BIGINT COMMENT 'tag:5'," + - "PRIMARY KEY (pk)" + - ");", + `CREATE TABLE t ( + pk BIGINT NOT NULL COMMENT 'tag:0', + c1 BIGINT COMMENT 'tag:1', + c2 BIGINT COMMENT 'tag:2', + c3 BIGINT COMMENT 'tag:3', + c4 BIGINT COMMENT 'tag:4', + c5 BIGINT COMMENT 'tag:5', + PRIMARY KEY (pk) + );`, "call dolt_add('.')", "set @C0 = '';", "call dolt_commit_hash_out(@C0, '-m', 'Created table t');", @@ -922,6 +922,13 @@ var DiffTableFunctionScriptTests = []queries.ScriptTest{ {int64(0), int64(600), "modified"}, }, }, + { + Query: "SELECT d.to_pk, d.to_c1, d.to_c2, d.to_c6, d.diff_type " + + "FROM (SELECT * FROM dolt_diff('--skinny', '--include-cols=c1,c2', @C2, @C3, 't')) d", + Expected: []sql.Row{ + {int64(0), int64(100), int64(2), int64(600), "modified"}, + }, + }, { Query: "SELECT d.from_pk, d.from_c1, d.from_c2, d.from_c3, d.from_c4, d.from_c5, d.from_c6, d.diff_type " + "FROM (SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')) d", @@ -936,6 +943,9 @@ var DiffTableFunctionScriptTests = []queries.ScriptTest{ asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_12", "SELECT * FROM dolt_diff('--skinny', @C1, @C2, 't')", 4)...) asserts = append(asserts, assertDoltDiffColumnCount("v_all_23", "SELECT * FROM dolt_diff(@C2, @C3, 't')", 7)...) asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', @C2, @C3, 't')", 2)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_23", "SELECT * FROM dolt_diff(@C2, @C3, 't', '--skinny')", 2)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', '--include-cols=c1,c2', @C2, @C3, 't')", 4)...) + asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_23", "SELECT * FROM dolt_diff('--skinny', '--include-cols=c1,c2,c6', @C2, @C3, 't')", 4)...) asserts = append(asserts, assertDoltDiffColumnCount("v_all_34", "SELECT * FROM dolt_diff(@C3, @C4, 't')", 7)...) asserts = append(asserts, assertDoltDiffColumnCount("v_skinny_34", "SELECT * FROM dolt_diff('--skinny', @C3, @C4, 't')", 7)...) diff --git a/integration-tests/bats/helper/sql-diff.bash b/integration-tests/bats/helper/sql-diff.bash index 9c59089c82..defe2d51d7 100644 --- a/integration-tests/bats/helper/sql-diff.bash +++ b/integration-tests/bats/helper/sql-diff.bash @@ -86,54 +86,54 @@ _compare_counts_or_err() { # ---- main entrypoint ---- -# Compare CLI diff with SQL dolt_diff (both normal and skinny) -# Usage: compare_dolt_diff from_commit to_commit table_name +# Compare CLI diff with SQL dolt_diff +# Usage: compare_dolt_diff [all dolt diff args...] compare_dolt_diff() { - local from_commit="$1" - local to_commit="$2" - local table_name="$3" + local args=("$@") # all arguments - # --- normal mode --- - local cli_normal_output sql_normal_output cli_status sql_status - cli_normal_output=$(dolt diff "$from_commit" "$to_commit" "$table_name" 2>&1); cli_status=$? - sql_normal_output=$(dolt sql -q "SELECT * FROM dolt_diff('$from_commit', '$to_commit', '$table_name')" 2>&1); sql_status=$? + # --- normal diff --- + local cli_output sql_output cli_status sql_status + cli_output=$(dolt diff "${args[@]}" 2>&1) + cli_status=$? - echo "$cli_normal_output" - echo "$sql_normal_output" + # Build SQL argument list safely + local sql_args="" + for arg in "${args[@]}"; do + if [ -z "$sql_args" ]; then + sql_args="'$arg'" + else + sql_args+=", '$arg'" + fi + done + sql_output=$(dolt sql -q "SELECT * FROM dolt_diff($sql_args)" 2>&1) + sql_status=$? - if [ $cli_status -ne 0 ]; then echo "CLI diff failed"; echo "$cli_normal_output"; return 1; fi - if [ $sql_status -ne 0 ]; then echo "SQL dolt_diff failed"; echo "$sql_normal_output"; return 1; fi + # normally prints in bats using `run`, so no debug blocks here + echo "$cli_output" + echo "$sql_output" - local normal_cli_changes normal_sql_rows - normal_cli_changes=$(_cli_change_count "$cli_normal_output") - normal_sql_rows=$(_sql_row_count "$sql_normal_output") - _compare_counts_or_err "Normal diff" "$cli_normal_output" "$sql_normal_output" "$normal_cli_changes" "$normal_sql_rows" || return 1 + if [ $cli_status -ne 0 ]; then + echo "CLI diff failed" + _dbg "$cli_output" + return 1 + fi + if [ $sql_status -ne 0 ]; then + echo "SQL dolt_diff failed" + _dbg "$sql_output" + return 1 + fi - local normal_cli_cols normal_sql_cols - normal_cli_cols=$(_cli_header_cols "$cli_normal_output") - normal_sql_cols=$(_sql_data_header_cols "$sql_normal_output") - _compare_sets_or_err "Normal diff" "$normal_cli_cols" "$normal_sql_cols" "$cli_normal_output" "$sql_normal_output" || return 1 + # Compare counts + local cli_changes sql_rows + cli_changes=$(_cli_change_count "$cli_output") + sql_rows=$(_sql_row_count "$sql_output") + _compare_counts_or_err "Diff" "$cli_output" "$sql_output" "$cli_changes" "$sql_rows" || return 1 - # --- skinny mode --- - local cli_skinny_output sql_skinny_output - cli_skinny_output=$(dolt diff --skinny "$from_commit" "$to_commit" "$table_name" 2>&1); cli_status=$? - sql_skinny_output=$(dolt sql -q "SELECT * FROM dolt_diff('--skinny', '$from_commit', '$to_commit', '$table_name')" 2>&1); sql_status=$? - - echo "$cli_skinny_output" - echo "$sql_skinny_output" - - if [ $cli_status -ne 0 ]; then echo "CLI skinny diff failed"; echo "$cli_skinny_output"; return 1; fi - if [ $sql_status -ne 0 ]; then echo "SQL skinny dolt_diff failed"; echo "$sql_skinny_output"; return 1; fi - - local skinny_cli_changes skinny_sql_rows - skinny_cli_changes=$(_cli_change_count "$cli_skinny_output") - skinny_sql_rows=$(_sql_row_count "$sql_skinny_output") - _compare_counts_or_err "Skinny diff" "$cli_skinny_output" "$sql_skinny_output" "$skinny_cli_changes" "$skinny_sql_rows" || return 1 - - local skinny_cli_cols skinny_sql_cols - skinny_cli_cols=$(_cli_header_cols "$cli_skinny_output") - skinny_sql_cols=$(_sql_data_header_cols "$sql_skinny_output") - _compare_sets_or_err "Skinny diff" "$skinny_cli_cols" "$skinny_sql_cols" "$cli_skinny_output" "$sql_skinny_output" || return 1 + # Compare columns + local cli_cols sql_cols + cli_cols=$(_cli_header_cols "$cli_output") + sql_cols=$(_sql_data_header_cols "$sql_output") + _compare_sets_or_err "Diff" "$cli_cols" "$sql_cols" "$cli_output" "$sql_output" || return 1 return 0 -} \ No newline at end of file +} diff --git a/integration-tests/bats/helper/windows-compat.bash b/integration-tests/bats/helper/windows-compat.bash index 344fbcb476..1305bbc3f1 100644 --- a/integration-tests/bats/helper/windows-compat.bash +++ b/integration-tests/bats/helper/windows-compat.bash @@ -5,7 +5,7 @@ skiponwindows() { :; } IS_WINDOWS=${IS_WINDOWS:-false} WINDOWS_BASE_DIR=${WINDOWS_BASE_DIR:-/mnt/c} -if [ ! -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then +if [ -d "$WINDOWS_BASE_DIR"/Windows/System32 ] || [ "$IS_WINDOWS" == true ]; then IS_WINDOWS=true if [ ! -d "$WINDOWS_BASE_DIR"/batstmp ]; then mkdir "$WINDOWS_BASE_DIR"/batstmp diff --git a/integration-tests/bats/sql-diff.bats b/integration-tests/bats/sql-diff.bats index 4a440d4f71..f08eab8f01 100644 --- a/integration-tests/bats/sql-diff.bats +++ b/integration-tests/bats/sql-diff.bats @@ -909,6 +909,9 @@ SQL dolt commit -m "Added initial data" compare_dolt_diff "HEAD~1" "HEAD" "test" + compare_dolt_diff "-sk" "HEAD~1" "HEAD" "test" + compare_dolt_diff "--skinny" "HEAD~1" "HEAD" "test" + compare_dolt_diff "HEAD~1" "HEAD" "-sk" "test" dolt sql -q "UPDATE test SET c1=100, c3=300 WHERE pk=0" dolt sql -q "UPDATE test SET c2=200 WHERE pk=1" @@ -916,6 +919,8 @@ SQL dolt commit -m "Updated some columns" compare_dolt_diff "HEAD~1" "HEAD" "test" + compare_dolt_diff "--skinny" "HEAD~1" "HEAD" "test" + compare_dolt_diff "HEAD~1" "HEAD" "test" "--skinny" dolt sql -q "ALTER TABLE test ADD COLUMN c6 BIGINT" dolt sql -q "UPDATE test SET c6=600 WHERE pk=0" @@ -923,22 +928,23 @@ SQL dolt commit -m "Added new column and updated it" compare_dolt_diff "HEAD~1" "HEAD" "test" + compare_dolt_diff "--skinny" "HEAD~1" "HEAD" "test" + compare_dolt_diff "--skinny" "--include-cols=c1,c2" "HEAD~1" "HEAD" "test" + compare_dolt_diff "--skinny" "HEAD~2" "HEAD" "test" "--include-cols" "c1" "c2" + compare_dolt_diff "--skinny" "--include-cols=c1,c2" "HEAD~2" "HEAD" "test" dolt sql -q "DELETE FROM test WHERE pk=1" dolt add test dolt commit -m "Deleted a row" compare_dolt_diff "HEAD~1" "HEAD" "test" - - run dolt sql -q "SELECT * FROM dolt_diff('-sk', 'HEAD~1', 'HEAD', 'test')" - [ "$status" -eq 0 ] + compare_dolt_diff "--skinny" "HEAD~1" "HEAD" "test" run dolt sql -q "SELECT * FROM dolt_diff('-err', 'HEAD~1', 'HEAD', 'test')" + [[ "$output" =~ "unknown option \`err" ]] || false [ "$status" -eq 1 ] run dolt sql -q "SELECT * FROM dolt_diff('-sk', '--skinny', 'HEAD~1', 'HEAD', 'test')" - [ "$status" -eq 1 ] - - run dolt sql -q "SELECT * FROM dolt_diff('HEAD~1', 'HEAD', 'test', '-sk')" + [[ "$output" =~ "multiple values provided for \`skinny" ]] [ "$status" -eq 1 ] } From b0ddaede455e67b9ee30e4cb52949704c5c3a349 Mon Sep 17 00:00:00 2001 From: elianddb Date: Mon, 8 Sep 2025 11:33:39 -0700 Subject: [PATCH 09/11] fix missing bind var err on bats --- go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go | 8 ++++---- integration-tests/bats/sql-diff.bats | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index d3ad6dd7bf..04c6385d4d 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -122,7 +122,7 @@ func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sq // TODO: For now, we will only support literal / fully-resolved arguments to the // DiffTableFunction to avoid issues where the schema is needed in the analyzer // before the arguments could be resolved. - var literalArgs []string + var exprStrs []string strToExpr := map[string]sql.Expression{} for _, expr := range expressions { if !expr.Resolved() { @@ -133,14 +133,14 @@ func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sq return nil, ErrInvalidNonLiteralArgument.New(dtf.Name(), expr.String()) } strVal := expr.String() - if lit, ok := expr.(*expression.Literal); ok { // we only support literals for parsing + if lit, ok := expr.(*expression.Literal); ok { // rm quotes from string literals strVal = fmt.Sprintf("%v", lit.Value()) - literalArgs = append(literalArgs, strVal) } + exprStrs = append(exprStrs, strVal) // args extracted from apr later to filter out options strToExpr[strVal] = expr } - apr, err := cli.CreateDiffArgParser(true).Parse(literalArgs) + apr, err := cli.CreateDiffArgParser(true).Parse(exprStrs) if err != nil { return nil, err } diff --git a/integration-tests/bats/sql-diff.bats b/integration-tests/bats/sql-diff.bats index f08eab8f01..72579392b2 100644 --- a/integration-tests/bats/sql-diff.bats +++ b/integration-tests/bats/sql-diff.bats @@ -945,6 +945,6 @@ SQL [ "$status" -eq 1 ] run dolt sql -q "SELECT * FROM dolt_diff('-sk', '--skinny', 'HEAD~1', 'HEAD', 'test')" - [[ "$output" =~ "multiple values provided for \`skinny" ]] + [[ "$output" =~ "multiple values provided for \`skinny" ]] || false [ "$status" -eq 1 ] } From 4450754c331369eeaa652c799f85af448b8b748e Mon Sep 17 00:00:00 2001 From: elianddb Date: Mon, 8 Sep 2025 17:46:06 -0700 Subject: [PATCH 10/11] rm persistence on options --- go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go | 6 ++++-- integration-tests/bats/helper/sql-diff.bash | 2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 04c6385d4d..336cec0b3f 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -145,9 +145,11 @@ func (dtf *DiffTableFunction) WithExpressions(expressions ...sql.Expression) (sq return nil, err } - newDtf.showSkinny = newDtf.showSkinny || apr.Contains(cli.SkinnyFlag) + if apr.Contains(cli.SkinnyFlag) { + newDtf.showSkinny = true + } - if cols, ok := apr.GetValueList(cli.IncludeCols); newDtf.includeCols == nil && ok { + if cols, ok := apr.GetValueList(cli.IncludeCols); ok { newDtf.includeCols = make(map[string]struct{}) for _, col := range cols { newDtf.includeCols[col] = struct{}{} diff --git a/integration-tests/bats/helper/sql-diff.bash b/integration-tests/bats/helper/sql-diff.bash index defe2d51d7..ffb9d3020c 100644 --- a/integration-tests/bats/helper/sql-diff.bash +++ b/integration-tests/bats/helper/sql-diff.bash @@ -113,12 +113,10 @@ compare_dolt_diff() { echo "$sql_output" if [ $cli_status -ne 0 ]; then - echo "CLI diff failed" _dbg "$cli_output" return 1 fi if [ $sql_status -ne 0 ]; then - echo "SQL dolt_diff failed" _dbg "$sql_output" return 1 fi From 666f8a6bbf77f034ddac1246b9b852a95f4fae35 Mon Sep 17 00:00:00 2001 From: elianddb Date: Tue, 9 Sep 2025 00:54:38 +0000 Subject: [PATCH 11/11] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go | 6 +++--- go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index 336cec0b3f..541dc03bfa 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -16,16 +16,15 @@ package dtablefunctions import ( "fmt" - "github.com/dolthub/dolt/go/cmd/dolt/cli" - dolttable "github.com/dolthub/dolt/go/libraries/doltcore/table" - "github.com/dolthub/go-mysql-server/sql/expression" "io" "strings" "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" gmstypes "github.com/dolthub/go-mysql-server/sql/types" "gopkg.in/src-d/go-errors.v1" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/merge" @@ -36,6 +35,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtables" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/resolve" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" + dolttable "github.com/dolthub/dolt/go/libraries/doltcore/table" "github.com/dolthub/dolt/go/store/types" ) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go index a2c1ef6c55..e7ffd501b2 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go @@ -16,10 +16,11 @@ package enginetest import ( "fmt" + "strings" + "github.com/dolthub/go-mysql-server/enginetest/queries" "github.com/dolthub/go-mysql-server/sql" gmstypes "github.com/dolthub/go-mysql-server/sql/types" - "strings" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtablefunctions" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtables"