Skip to content

Commit

Permalink
Merge pull request #8411 from dolthub/taylor/fix-diff-fns
Browse files Browse the repository at this point in the history
Return `schema.table` for table name columns in dolt diff/status tables and functions for doltgres
  • Loading branch information
tbantle22 authored Oct 3, 2024
2 parents fdd2454 + 4587cad commit b407a9c
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 83 deletions.
4 changes: 2 additions & 2 deletions go/libraries/doltcore/diff/table_deltas.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,9 +446,9 @@ func (td TableDelta) HasChanges() (bool, error) {
// CurName returns the most recent name of the table.
func (td TableDelta) CurName() string {
if td.ToName.Name != "" {
return td.ToName.Name
return td.ToName.String()
}
return td.FromName.Name
return td.FromName.String()
}

func (td TableDelta) HasFKChanges() bool {
Expand Down
29 changes: 16 additions & 13 deletions go/libraries/doltcore/doltdb/workingset.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ type MergeState struct {
// the spec string that was used to specify |commit|
commitSpecStr string
preMergeWorking RootValue
unmergableTables []string // TODO: need schema name here
mergedTables []string // TODO: need schema name here
unmergableTables []TableName
mergedTables []TableName
// isCherryPick is set to true when the in-progress merge is a cherry-pick. This is needed so that
// commit knows to NOT create a commit with multiple parents when creating a commit for a cherry-pick.
isCherryPick bool
Expand Down Expand Up @@ -181,17 +181,17 @@ func (m MergeState) PreMergeWorkingRoot() RootValue {
return m.preMergeWorking
}

type SchemaConflictFn func(table string, conflict SchemaConflict) error
type SchemaConflictFn func(table TableName, conflict SchemaConflict) error

func (m MergeState) HasSchemaConflicts() bool {
return len(m.unmergableTables) > 0
}

func (m MergeState) TablesWithSchemaConflicts() []string {
func (m MergeState) TablesWithSchemaConflicts() []TableName {
return m.unmergableTables
}

func (m MergeState) MergedTables() []string {
func (m MergeState) MergedTables() []TableName {
return m.mergedTables
}

Expand Down Expand Up @@ -224,7 +224,7 @@ func (m MergeState) IterSchemaConflicts(ctx context.Context, ddb *DoltDB, cb Sch
for _, name := range m.unmergableTables {
var sc SchemaConflict
var hasToTable bool
if sc.toTbl, hasToTable, err = to.GetTable(ctx, TableName{Name: name}); err != nil {
if sc.toTbl, hasToTable, err = to.GetTable(ctx, name); err != nil {
return err
}
if hasToTable {
Expand All @@ -235,7 +235,7 @@ func (m MergeState) IterSchemaConflicts(ctx context.Context, ddb *DoltDB, cb Sch

var hasFromTable bool
// todo: handle schema conflicts for renamed tables
if sc.fromTbl, hasFromTable, err = from.GetTable(ctx, TableName{Name: name}); err != nil {
if sc.fromTbl, hasFromTable, err = from.GetTable(ctx, name); err != nil {
return err
}
if hasFromTable {
Expand All @@ -244,10 +244,10 @@ func (m MergeState) IterSchemaConflicts(ctx context.Context, ddb *DoltDB, cb Sch
}
}

sc.ToFks, _ = toFKs.KeysForTable(TableName{Name: name})
sc.ToFks, _ = toFKs.KeysForTable(name)
sc.ToParentSchemas = toSchemas

sc.FromFks, _ = fromFKs.KeysForTable(TableName{Name: name})
sc.FromFks, _ = fromFKs.KeysForTable(name)
sc.FromParentSchemas = fromSchemas

if err = cb(name, sc); err != nil {
Expand Down Expand Up @@ -296,12 +296,12 @@ func (ws WorkingSet) WithRebaseState(rebaseState *RebaseState) *WorkingSet {
return &ws
}

func (ws WorkingSet) WithUnmergableTables(tables []string) *WorkingSet {
func (ws WorkingSet) WithUnmergableTables(tables []TableName) *WorkingSet {
ws.mergeState.unmergableTables = tables
return &ws
}

func (ws WorkingSet) WithMergedTables(tables []string) *WorkingSet {
func (ws WorkingSet) WithMergedTables(tables []TableName) *WorkingSet {
ws.mergeState.mergedTables = tables
return &ws
}
Expand Down Expand Up @@ -502,11 +502,13 @@ func newWorkingSet(ctx context.Context, name string, vrw types.ValueReadWriter,
return nil, err
}

unmergableTableNames := ToTableNames(unmergableTables, DefaultSchemaName)

mergeState = &MergeState{
commit: commit,
commitSpecStr: commitSpec,
preMergeWorking: preMergeWorkingRoot,
unmergableTables: unmergableTables,
unmergableTables: unmergableTableNames,
isCherryPick: isCherryPick,
}
}
Expand Down Expand Up @@ -618,7 +620,8 @@ func (ws *WorkingSet) writeValues(ctx context.Context, db *DoltDB, meta *datas.W
return nil, err
}

mergeState, err = datas.NewMergeState(ctx, db.vrw, preMergeWorking, dCommit, ws.mergeState.commitSpecStr, ws.mergeState.unmergableTables, ws.mergeState.isCherryPick)
// TODO: Serialize the full TableName
mergeState, err = datas.NewMergeState(ctx, db.vrw, preMergeWorking, dCommit, ws.mergeState.commitSpecStr, FlattenTableNames(ws.mergeState.unmergableTables), ws.mergeState.isCherryPick)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/env/actions/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func GetCommitStaged(
if ws.MergeActive() {
schConflicts := ws.MergeState().TablesWithSchemaConflicts()
if len(schConflicts) > 0 {
return nil, NewTblSchemaConflictError(doltdb.ToTableNames(schConflicts, doltdb.DefaultSchemaName))
return nil, NewTblSchemaConflictError(schConflicts)
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions go/libraries/doltcore/sqle/dolt_diff_stat_table_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func (ds *DiffStatTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.Row
if errors.Is(err, diff.ErrPrimaryKeySetChanged) {
ctx.Warn(dtables.PrimaryKeyChangeWarningCode, fmt.Sprintf("stat for table %s cannot be determined. Primary key set changed.", tblName))
// Report an empty diff for tables that have primary key set changes
diffStats = append(diffStats, diffStatNode{tblName: tblName.Name})
diffStats = append(diffStats, diffStatNode{tblName: tblName})
continue
}
return nil, err
Expand Down Expand Up @@ -415,7 +415,7 @@ func getDiffStatNodeFromDelta(ctx *sql.Context, delta diff.TableDelta, fromRoot,
return diffStatNode{}, false, err
}

return diffStatNode{tableName.Name, diffStat, oldColLen, newColLen, keyless}, hasDiff, nil
return diffStatNode{tableName, diffStat, oldColLen, newColLen, keyless}, hasDiff, nil
}

// getDiffStat returns diff.DiffStatProgress object and whether there is a data diff or not.
Expand Down Expand Up @@ -491,7 +491,7 @@ func (d *diffStatTableFunctionRowIter) incrementIndexes() {
}

type diffStatNode struct {
tblName string
tblName doltdb.TableName
diffStat diff.DiffStatProgress
oldColLen int
newColLen int
Expand Down Expand Up @@ -525,11 +525,11 @@ func (d *diffStatTableFunctionRowIter) Close(context *sql.Context) error {
// getRowFromDiffStat takes diff.DiffStatProgress and calculates the row_modified, cell_added, cell_deleted.
// If the number of cell change from old to new cell count does not equal to cell_added and/or cell_deleted, there
// must be schema changes that affects cell_added and cell_deleted value addition to the row count * col length number.
func getRowFromDiffStat(tblName string, dsp diff.DiffStatProgress, newColLen, oldColLen int, keyless bool) sql.Row {
func getRowFromDiffStat(tblName doltdb.TableName, dsp diff.DiffStatProgress, newColLen, oldColLen int, keyless bool) sql.Row {
// if table is keyless table, match current CLI command result
if keyless {
return sql.Row{
tblName, // table_name
tblName.String(), // table_name
nil, // rows_unmodified
int64(dsp.Adds), // rows_added
int64(dsp.Removes), // rows_deleted
Expand All @@ -548,7 +548,7 @@ func getRowFromDiffStat(tblName string, dsp diff.DiffStatProgress, newColLen, ol
rowsUnmodified := dsp.OldRowSize - dsp.Changes - dsp.Removes

return sql.Row{
tblName, // table_name
tblName.String(), // table_name
int64(rowsUnmodified), // rows_unmodified
int64(dsp.Adds), // rows_added
int64(dsp.Removes), // rows_deleted
Expand Down
10 changes: 5 additions & 5 deletions go/libraries/doltcore/sqle/dolt_diff_summary_table_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,10 @@ func (d *diffSummaryTableFunctionRowIter) Close(context *sql.Context) error {

func getRowFromSummary(ds *diff.TableDeltaSummary) sql.Row {
return sql.Row{
ds.FromTableName.Name, // from_table_name
ds.ToTableName.Name, // to_table_name
ds.DiffType, // diff_type
ds.DataChange, // data_change
ds.SchemaChange, // schema_change
ds.FromTableName.String(), // from_table_name
ds.ToTableName.String(), // to_table_name
ds.DiffType, // diff_type
ds.DataChange, // data_change
ds.SchemaChange, // schema_change
}
}
28 changes: 14 additions & 14 deletions go/libraries/doltcore/sqle/dolt_diff_table_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,20 +486,6 @@ func (dtf *DiffTableFunction) cacheTableDelta(ctx *sql.Context, fromCommitVal, t
return diff.TableDelta{}, err
}

fromTableName, fromTable, fromTableExists, err := resolve.Table(ctx, fromRefDetails.root, tableName)
if err != nil {
return diff.TableDelta{}, err
}

toTableName, toTable, toTableExists, err := resolve.Table(ctx, toRefDetails.root, tableName)
if err != nil {
return diff.TableDelta{}, err
}

if !fromTableExists && !toTableExists {
return diff.TableDelta{}, sql.ErrTableNotFound.New(tableName)
}

// TODO: it would be nice to limit this to just the table under consideration, not all tables with a diff
deltas, err := diff.GetTableDeltas(ctx, fromRefDetails.root, toRefDetails.root)
if err != nil {
Expand All @@ -514,6 +500,20 @@ func (dtf *DiffTableFunction) cacheTableDelta(ctx *sql.Context, fromCommitVal, t
// We only get a delta if there's a diff. When there isn't one, construct a delta here with table and schema info
// TODO: schema name
if delta.FromTable == nil && delta.ToTable == nil {
fromTableName, fromTable, fromTableExists, err := resolve.Table(ctx, fromRefDetails.root, tableName)
if err != nil {
return diff.TableDelta{}, err
}

toTableName, toTable, toTableExists, err := resolve.Table(ctx, toRefDetails.root, tableName)
if err != nil {
return diff.TableDelta{}, err
}

if !fromTableExists && !toTableExists {
return diff.TableDelta{}, sql.ErrTableNotFound.New(tableName)
}

delta.FromName = fromTableName
delta.ToName = toTableName
delta.FromTable = fromTable
Expand Down
19 changes: 14 additions & 5 deletions go/libraries/doltcore/sqle/dolt_patch_table_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"golang.org/x/exp/slices"

"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/schema"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
Expand Down Expand Up @@ -475,7 +476,7 @@ func (p *PatchTableFunction) evaluateArguments() (interface{}, interface{}, inte
}

type patchNode struct {
tblName string
tblName doltdb.TableName
schemaPatchStmts []string
dataPatchStmts []string
}
Expand All @@ -500,7 +501,7 @@ func getPatchNodes(ctx *sql.Context, dbData env.DbData, tableDeltas []diff.Table
}
alterDBCollStmt := sqlfmt.AlterDatabaseCollateStmt(dbName, fromColl, toColl)
patches = append(patches, &patchNode{
tblName: td.FromName.Name,
tblName: td.FromName,
schemaPatchStmts: []string{alterDBCollStmt},
dataPatchStmts: []string{},
})
Expand Down Expand Up @@ -529,7 +530,7 @@ func getPatchNodes(ctx *sql.Context, dbData env.DbData, tableDeltas []diff.Table
}
}

patches = append(patches, &patchNode{tblName: tblName.Name, schemaPatchStmts: schemaStmts, dataPatchStmts: dataStmts})
patches = append(patches, &patchNode{tblName: tblName, schemaPatchStmts: schemaStmts, dataPatchStmts: dataStmts})
}

return patches, nil
Expand Down Expand Up @@ -739,7 +740,12 @@ func (itr *patchTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) {
return nil, err
} else {
itr.statementIdx++
r := sql.Row{itr.statementIdx, itr.fromRef, itr.toRef, itr.currentPatch.tblName}
r := sql.Row{
itr.statementIdx, // statement_order
itr.fromRef, // from_commit_hash
itr.toRef, // to_commit_hash
itr.currentPatch.tblName.String(), // table_name
}
return r.Append(row), nil
}
}
Expand Down Expand Up @@ -789,7 +795,10 @@ func (p *patchStatementsRowIter) Next(ctx *sql.Context) (sql.Row, error) {
diffType = diffTypeData
}

return sql.Row{diffType, stmt}, nil
return sql.Row{
diffType, // diff_type
stmt, // statement
}, nil
}

func (p *patchStatementsRowIter) Close(_ *sql.Context) error {
Expand Down
8 changes: 4 additions & 4 deletions go/libraries/doltcore/sqle/dolt_schema_diff_table_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,10 @@ func (ds *SchemaDiffTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.R
}

row := sql.Row{
fromName.Name, // 0
toName.Name, // 1
fromCreate, // 2
toCreate, // 3
fromName.String(), // from_table_name
toName.String(), // to_table_name
fromCreate, // from_create_statement
toCreate, // to_create_statement
}
dataRows = append(dataRows, row)
}
Expand Down
Loading

0 comments on commit b407a9c

Please sign in to comment.