From b6e363295405a697adfa9c953212201e9f326e91 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Wed, 21 May 2025 11:16:59 -0700 Subject: [PATCH 01/12] Start on dolt_preview_merge_conflicts function --- .../dolt_preview_merge_conflicts.go | 438 ++++++++++++++++++ .../doltcore/sqle/dtablefunctions/init.go | 1 + .../sqle/dtables/conflicts_tables_prolly.go | 4 +- .../sqle/enginetest/dolt_queries_merge.go | 4 + 4 files changed, 445 insertions(+), 2 deletions(-) create mode 100644 go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go new file mode 100644 index 00000000000..cdd5c907dda --- /dev/null +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -0,0 +1,438 @@ +// Copyright 2025 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dtablefunctions + +import ( + "fmt" + "io" + + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/merge" + "github.com/dolthub/dolt/go/libraries/doltcore/schema" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtables" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" + "github.com/dolthub/go-mysql-server/sql/types" +) + +var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil) +var _ sql.ExecSourceRel = (*PreviewMergeConflictsTableFunction)(nil) +var _ sql.AuthorizationCheckerNode = (*PreviewMergeConflictsTableFunction)(nil) + +type PreviewMergeConflictsTableFunction struct { + ctx *sql.Context + leftBranchExpr sql.Expression + rightBranchExpr sql.Expression + tableNameExpr sql.Expression + sqlSch sql.Schema + database sql.Database + + tblName doltdb.TableName + leftRoot doltdb.RootValue + rightRoot doltdb.RootValue + baseRoot doltdb.RootValue +} + +// NewInstance creates a new instance of TableFunction interface +func (ds *PreviewMergeConflictsTableFunction) NewInstance(ctx *sql.Context, db sql.Database, expressions []sql.Expression) (sql.Node, error) { + newInstance := &PreviewMergeConflictsTableFunction{ + ctx: ctx, + database: db, + } + + node, err := newInstance.WithExpressions(expressions...) + if err != nil { + return nil, err + } + + return node, nil +} + +func (ds *PreviewMergeConflictsTableFunction) DataLength(ctx *sql.Context) (uint64, error) { + numBytesPerRow := schema.SchemaAvgLength(ds.Schema()) + numRows, _, err := ds.RowCount(ctx) + if err != nil { + return 0, err + } + return numBytesPerRow * numRows, nil +} + +func (ds *PreviewMergeConflictsTableFunction) RowCount(_ *sql.Context) (uint64, bool, error) { + return previewMergeConflictsDefaultRowCount, false, nil +} + +// Database implements the sql.Databaser interface +func (ds *PreviewMergeConflictsTableFunction) Database() sql.Database { + return ds.database +} + +// WithDatabase implements the sql.Databaser interface +func (ds *PreviewMergeConflictsTableFunction) WithDatabase(database sql.Database) (sql.Node, error) { + nds := *ds + nds.database = database + return &nds, nil +} + +// Name implements the sql.TableFunction interface +func (ds *PreviewMergeConflictsTableFunction) Name() string { + return "dolt_preview_merge_conflicts" +} + +// Resolved implements the sql.Resolvable interface +func (ds *PreviewMergeConflictsTableFunction) Resolved() bool { + return ds.leftBranchExpr.Resolved() && ds.rightBranchExpr.Resolved() && ds.tableNameExpr.Resolved() +} + +func (ds *PreviewMergeConflictsTableFunction) IsReadOnly() bool { + return true +} + +// String implements the Stringer interface +func (ds *PreviewMergeConflictsTableFunction) String() string { + return fmt.Sprintf("DOLT_PREVIEW_MERGE_CONFLICTS(%s, %s, %s)", ds.leftBranchExpr.String(), ds.rightBranchExpr.String(), ds.tableNameExpr.String()) +} + +// Schema implements the sql.Node interface. +func (ds *PreviewMergeConflictsTableFunction) Schema() sql.Schema { + if !ds.Resolved() { + return nil + } + + if ds.sqlSch == nil { + panic("schema hasn't been generated yet") + } + + return ds.sqlSch +} + +// Children implements the sql.Node interface. +func (ds *PreviewMergeConflictsTableFunction) Children() []sql.Node { + return nil +} + +// WithChildren implements the sql.Node interface. +func (ds *PreviewMergeConflictsTableFunction) WithChildren(children ...sql.Node) (sql.Node, error) { + if len(children) != 0 { + return nil, fmt.Errorf("unexpected children") + } + return ds, nil +} + +// CheckAuth implements the interface sql.AuthorizationCheckerNode. +func (pm *PreviewMergeConflictsTableFunction) CheckAuth(ctx *sql.Context, opChecker sql.PrivilegedOperationChecker) bool { + if !types.IsText(pm.tableNameExpr.Type()) { + return ExpressionIsDeferred(pm.tableNameExpr) + } + + tableNameVal, err := pm.tableNameExpr.Eval(pm.ctx, nil) + if err != nil { + return false + } + tableName, ok, err := sql.Unwrap[string](ctx, tableNameVal) + if err != nil { + return false + } + if !ok { + return false + } + + subject := sql.PrivilegeCheckSubject{Database: pm.database.Name(), Table: tableName} + // TODO: Add tests for privilege checking + return opChecker.UserHasPrivileges(ctx, sql.NewPrivilegedOperation(subject, sql.PrivilegeType_Select)) +} + +// Expressions implements the sql.Expressioner interface. +func (pm *PreviewMergeConflictsTableFunction) Expressions() []sql.Expression { + return []sql.Expression{pm.leftBranchExpr, pm.rightBranchExpr, pm.tableNameExpr} +} + +// WithExpressions implements the sql.Expressioner interface. +func (pm *PreviewMergeConflictsTableFunction) WithExpressions(exprs ...sql.Expression) (sql.Node, error) { + if len(exprs) != 3 { + return nil, sql.ErrInvalidArgumentNumber.New(pm.Name(), "3", len(exprs)) + } + + for _, expr := range exprs { + if !expr.Resolved() { + return nil, ErrInvalidNonLiteralArgument.New(pm.Name(), expr.String()) + } + // prepared statements resolve functions beforehand, so above check fails + if _, ok := expr.(sql.FunctionExpression); ok { + return nil, ErrInvalidNonLiteralArgument.New(pm.Name(), expr.String()) + } + } + + newPmcs := *pm + newPmcs.leftBranchExpr = exprs[0] + newPmcs.rightBranchExpr = exprs[1] + newPmcs.tableNameExpr = exprs[2] + + // validate the expressions + if !types.IsText(newPmcs.leftBranchExpr.Type()) && !expression.IsBindVar(newPmcs.leftBranchExpr) { + return nil, sql.ErrInvalidArgumentDetails.New(newPmcs.Name(), newPmcs.leftBranchExpr.String()) + } + if !types.IsText(newPmcs.rightBranchExpr.Type()) && !expression.IsBindVar(newPmcs.rightBranchExpr) { + return nil, sql.ErrInvalidArgumentDetails.New(newPmcs.Name(), newPmcs.rightBranchExpr.String()) + } + if !types.IsText(newPmcs.tableNameExpr.Type()) && !expression.IsBindVar(newPmcs.tableNameExpr) { + return nil, sql.ErrInvalidArgumentDetails.New(newPmcs.Name(), newPmcs.tableNameExpr.String()) + } + + leftBranchVal, rightBranchVal, tableName, err := newPmcs.evaluateArguments() + if err != nil { + return nil, err + } + + err = newPmcs.generateSchema(pm.ctx, leftBranchVal, rightBranchVal, tableName) + if err != nil { + return nil, err + } + + return &newPmcs, nil +} + +func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, leftBranchVal, rightBranchVal interface{}, tableName string) error { + if !pm.Resolved() { + return nil + } + + sqledb, ok := pm.database.(dsess.SqlDatabase) + if !ok { + return fmt.Errorf("unexpected database type: %T", pm.database) + } + + leftBranch, err := interfaceToString(leftBranchVal) + if err != nil { + return err + } + rightBranch, err := interfaceToString(rightBranchVal) + if err != nil { + return err + } + + leftRoot, rightRoot, baseRoot, err := resolveBranchesToRoots(ctx, sqledb, leftBranch, rightBranch) + if err != nil { + return err + } + + tblName := doltdb.TableName{Name: tableName, Schema: doltdb.DefaultSchemaName} + baseSch, ourSch, theirSch, err := getConflictSchemasFromRoots(ctx, tblName, leftRoot, rightRoot, baseRoot) + if err != nil { + return err + } + + confSch, _, err := dtables.CalculateConflictSchema(baseSch, ourSch, theirSch) + if err != nil { + return err + } + + sqlSch, err := sqlutil.FromDoltSchema(sqledb.Name(), tblName.Name, confSch) + if err != nil { + return err + } + + pm.sqlSch = sqlSch.Schema + pm.leftRoot = leftRoot + pm.rightRoot = rightRoot + pm.baseRoot = baseRoot + pm.tblName = tblName + + return nil +} + +func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, leftRoot, rightRoot, baseRoot doltdb.RootValue) (base, sch, mergeSch schema.Schema, err error) { + ourTbl, ourOk, err := leftRoot.GetTable(ctx, tblName) + if err != nil { + return nil, nil, nil, err + } + if !ourOk { + return nil, nil, nil, fmt.Errorf("could not find tbl %s in left root value", tblName) + } + + baseTbl, baseOk, err := baseRoot.GetTable(ctx, tblName) + if err != nil { + return nil, nil, nil, err + } + theirTbl, theirOK, err := rightRoot.GetTable(ctx, tblName) + if err != nil { + return nil, nil, nil, err + } + if !theirOK { + return nil, nil, nil, fmt.Errorf("could not find tbl %s in right root value", tblName) + } + + ourSch, err := ourTbl.GetSchema(ctx) + if err != nil { + return nil, nil, nil, err + } + + theirSch, err := theirTbl.GetSchema(ctx) + if err != nil { + return nil, nil, nil, err + } + + // If the table does not exist in the ancestor, pretend it existed and that + // it was completely empty. + if !baseOk { + if schema.SchemasAreEqual(ourSch, theirSch) { + return ourSch, ourSch, theirSch, nil + } else { + return nil, nil, nil, fmt.Errorf("expected our schema to equal their schema since the table did not exist in the ancestor") + } + } + + baseSch, err := baseTbl.GetSchema(ctx) + if err != nil { + return nil, nil, nil, err + } + + return baseSch, ourSch, theirSch, nil +} + +// RowIter implements the sql.Node interface +func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) { + sqledb, ok := pm.database.(dsess.SqlDatabase) + if !ok { + return nil, fmt.Errorf("unexpected database type: %T", pm.database) + } + + conflicts, err := pm.getConflictsForTable(ctx, sqledb) + if err != nil { + return nil, err + } + + return NewPreviewMergeConflictsTableFunctionRowIter(conflicts), nil +} + +func (pm *PreviewMergeConflictsTableFunction) getConflictsForTable(ctx *sql.Context, sqledb dsess.SqlDatabase) ([]tableConflict, error) { + merger, err := merge.NewMerger(pm.leftRoot, pm.rightRoot, pm.baseRoot, pm.rightRoot, pm.baseRoot, pm.leftRoot.VRW(), pm.leftRoot.NodeStore()) + if err != nil { + return nil, err + } + + mergeOpts := merge.MergeOpts{ + IsCherryPick: false, + KeepSchemaConflicts: true, + ReverifyAllConstraints: false, + } + + tblName := doltdb.TableName{Name: pm.tableNameExpr.String(), Schema: doltdb.DefaultSchemaName} + + tm, err := merger.MakeTableMerger(ctx, tblName, mergeOpts) + if err != nil { + return nil, err + } + + // short-circuit here if we can + finished, _, stats, err := merger.MaybeShortCircuit(ctx, tm, mergeOpts) + if err != nil { + return nil, err + } + if finished != nil || stats != nil { + continue + } + // Calculate a merge of the schemas, but don't apply it + mergeSch, schConflicts, _, diffInfo, err := tm.SchemaMerge(ctx, tblName) + if err != nil { + return nil, err + } + numSchemaConflicts := uint64(schConflicts.Count()) + if numSchemaConflicts > 0 { + conflicted = append(conflicted, tableConflict{tableName: tblName, numSchemaConflicts: &numSchemaConflicts}) + // Cannot calculate data conflicts if there are schema conflicts + continue + } + + dataConflicts, err := getDataConflictsForTable(ctx, tm, tblName, mergeSch, diffInfo) + if err != nil { + return nil, err + } + if dataConflicts != nil { + conflicted = append(conflicted, *dataConflicts) + } + +} + +// evaluateArguments returns leftBranchVal amd rightBranchVal. +// It evaluates the argument expressions to turn them into values this PreviewMergeConflictsTableFunction +// can use. Note that this method only evals the expressions, and doesn't validate the values. +func (pm *PreviewMergeConflictsTableFunction) evaluateArguments() (interface{}, interface{}, string, error) { + leftBranchVal, err := pm.leftBranchExpr.Eval(pm.ctx, nil) + if err != nil { + return nil, nil, "", err + } + + rightBranchVal, err := pm.rightBranchExpr.Eval(pm.ctx, nil) + if err != nil { + return nil, nil, "", err + } + + tableNameVal, err := pm.tableNameExpr.Eval(pm.ctx, nil) + if err != nil { + return nil, nil, "", err + } + + tableName, ok := tableNameVal.(string) + if !ok { + return nil, nil, "", ErrInvalidTableName.New(pm.tableNameExpr.String()) + } + + return leftBranchVal, rightBranchVal, tableName, nil +} + +//-------------------------------------------------- +// previewMergeConflictsTableFunctionRowIter +//-------------------------------------------------- + +var _ sql.RowIter = &previewMergeConflictsTableFunctionRowIter{} + +type previewMergeConflictsTableFunctionRowIter struct { + conflicts []tableConflict + conIdx int +} + +func (d *previewMergeConflictsTableFunctionRowIter) incrementIndexes() { + d.conIdx++ + if d.conIdx >= len(d.conflicts) { + d.conIdx = 0 + d.conflicts = nil + } +} + +func NewPreviewMergeConflictsTableFunctionRowIter(pm []tableConflict) sql.RowIter { + return &previewMergeConflictsTableFunctionRowIter{ + conflicts: pm, + } +} + +func (d *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) { + defer d.incrementIndexes() + if d.conIdx >= len(d.conflicts) { + return nil, io.EOF + } + + if d.conflicts == nil { + return nil, io.EOF + } + + pm := d.conflicts[d.conIdx] + return getRowFromConflict(pm), nil +} + +func (d *previewMergeConflictsTableFunctionRowIter) Close(context *sql.Context) error { + return nil +} diff --git a/go/libraries/doltcore/sqle/dtablefunctions/init.go b/go/libraries/doltcore/sqle/dtablefunctions/init.go index 2b3aac0682f..e9cff59a502 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/init.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/init.go @@ -23,6 +23,7 @@ var DoltTableFunctions = []sql.TableFunction{ &LogTableFunction{}, &PatchTableFunction{}, &PreviewMergeConflictsSummaryTableFunction{}, + &PreviewMergeConflictsTableFunction{}, &SchemaDiffTableFunction{}, &ReflogTableFunction{}, &QueryDiffTableFunction{}, diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index e22b6d97365..75482c63e7f 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -53,7 +53,7 @@ func newProllyConflictsTable( if err != nil { return nil, err } - confSch, versionMappings, err := calculateConflictSchema(baseSch, ourSch, theirSch) + confSch, versionMappings, err := CalculateConflictSchema(baseSch, ourSch, theirSch) if err != nil { return nil, err } @@ -689,7 +689,7 @@ type versionMappings struct { } // returns the schema of the rows returned by the conflicts table and a mappings between each version and the source table. -func calculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *versionMappings, error) { +func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *versionMappings, error) { keyless := schema.IsKeyless(ours) n := 4 + ours.GetAllCols().Size() + theirs.GetAllCols().Size() + base.GetAllCols().Size() if keyless { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 5bd8d4d3759..12555e4eeb9 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -265,6 +265,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'feature-branch')", Expected: []sql.Row{}, }, + { + Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + Expected: []sql.Row{}, + }, { // FF-Merge Query: "CALL DOLT_MERGE('feature-branch')", From 0a3f068ca5f5fb4e492a83b805fc9a75a6ae4128 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Wed, 28 May 2025 14:27:08 -0700 Subject: [PATCH 02/12] Two tests working --- .../doltcore/merge/merge_prolly_rows.go | 2 +- go/libraries/doltcore/merge/merge_rows.go | 7 +- .../dolt_preview_merge_conflicts.go | 394 +++++++++++++++--- .../dolt_preview_merge_conflicts_summary.go | 30 +- .../sqle/dtables/conflicts_tables_prolly.go | 11 +- .../sqle/enginetest/dolt_queries_merge.go | 8 + 6 files changed, 374 insertions(+), 78 deletions(-) diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index 39a5aa9fc0a..83cea355de1 100644 --- a/go/libraries/doltcore/merge/merge_prolly_rows.go +++ b/go/libraries/doltcore/merge/merge_prolly_rows.go @@ -78,7 +78,7 @@ func mergeProllyTable( if err != nil { return nil, nil, err } - valueMerger := NewValueMerger(mergedSch, tm.leftSch, tm.rightSch, tm.ancSch, leftRows.Pool(), tm.ns) + valueMerger := tm.GetNewValueMerger(mergedSch, leftRows) if !valueMerger.leftMapping.IsIdentityMapping() { mergeInfo.LeftNeedsRewrite = true diff --git a/go/libraries/doltcore/merge/merge_rows.go b/go/libraries/doltcore/merge/merge_rows.go index b56a85642e6..900c28e1b0a 100644 --- a/go/libraries/doltcore/merge/merge_rows.go +++ b/go/libraries/doltcore/merge/merge_rows.go @@ -112,6 +112,10 @@ func (tm TableMerger) AncRows(ctx context.Context) (prolly.Map, error) { return rowsFromTable(ctx, tm.ancTbl) } +func (tm TableMerger) InvolvesRootObjects() bool { + return tm.leftRootObj != nil || tm.rightRootObj != nil || tm.ancRootObj != nil +} + func (tm TableMerger) tableHashes(ctx context.Context) (left, right, anc hash.Hash, err error) { if tm.leftTbl != nil { if left, err = tm.leftTbl.HashOf(); err != nil { @@ -238,8 +242,7 @@ func (rm *RootMerger) MergeTable( var tbl *doltdb.Table var rootObj doltdb.RootObject - involvesRootObjects := tm.leftRootObj != nil || tm.rightRootObj != nil || tm.ancRootObj != nil - if !involvesRootObjects { + if !tm.InvolvesRootObjects() { if types.IsFormat_DOLT(tm.vrw.Format()) { tbl, stats, err = mergeProllyTable(ctx, tm, mergeSch, mergeInfo, diffInfo) } else { diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index cdd5c907dda..ecc950260bf 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -15,6 +15,7 @@ package dtablefunctions import ( + "encoding/base64" "fmt" "io" @@ -24,9 +25,15 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dtables" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" + "github.com/dolthub/dolt/go/store/hash" + "github.com/dolthub/dolt/go/store/prolly" + "github.com/dolthub/dolt/go/store/prolly/tree" + dtypes "github.com/dolthub/dolt/go/store/types" + "github.com/dolthub/dolt/go/store/val" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" "github.com/dolthub/go-mysql-server/sql/types" + "github.com/zeebo/xxh3" ) var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil) @@ -38,13 +45,13 @@ type PreviewMergeConflictsTableFunction struct { leftBranchExpr sql.Expression rightBranchExpr sql.Expression tableNameExpr sql.Expression - sqlSch sql.Schema database sql.Database - tblName doltdb.TableName - leftRoot doltdb.RootValue - rightRoot doltdb.RootValue - baseRoot doltdb.RootValue + root, leftRoot, rightRoot, baseRoot doltdb.RootValue + tblName doltdb.TableName + sqlSch sql.PrimaryKeySchema + baseSch, ourSch, theirSch schema.Schema + rightSrc, ancestorSrc doltdb.Rootish } // NewInstance creates a new instance of TableFunction interface @@ -112,11 +119,11 @@ func (ds *PreviewMergeConflictsTableFunction) Schema() sql.Schema { return nil } - if ds.sqlSch == nil { + if ds.sqlSch.Schema == nil { panic("schema hasn't been generated yet") } - return ds.sqlSch + return ds.sqlSch.Schema } // Children implements the sql.Node interface. @@ -224,7 +231,12 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l return err } - leftRoot, rightRoot, baseRoot, err := resolveBranchesToRoots(ctx, sqledb, leftBranch, rightBranch) + leftRoot, rightRoot, baseRoot, rightSrc, ancestorSrc, err := resolveBranchesToRoots(ctx, sqledb, leftBranch, rightBranch) + if err != nil { + return err + } + + root, err := sqledb.GetRoot(ctx) if err != nil { return err } @@ -240,16 +252,22 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l return err } - sqlSch, err := sqlutil.FromDoltSchema(sqledb.Name(), tblName.Name, confSch) + sqlSch, err := sqlutil.FromDoltSchema(sqledb.Name(), pm.Name(), confSch) if err != nil { return err } - pm.sqlSch = sqlSch.Schema + pm.sqlSch = sqlSch + pm.root = root pm.leftRoot = leftRoot pm.rightRoot = rightRoot pm.baseRoot = baseRoot + pm.rightSrc = rightSrc + pm.ancestorSrc = ancestorSrc pm.tblName = tblName + pm.baseSch = baseSch + pm.ourSch = ourSch + pm.theirSch = theirSch return nil } @@ -305,21 +323,11 @@ func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, lef // RowIter implements the sql.Node interface func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) { - sqledb, ok := pm.database.(dsess.SqlDatabase) - if !ok { - return nil, fmt.Errorf("unexpected database type: %T", pm.database) - } - - conflicts, err := pm.getConflictsForTable(ctx, sqledb) - if err != nil { - return nil, err + if pm.sqlSch.Schema == nil { + panic("schema hasn't been generated yet") } - return NewPreviewMergeConflictsTableFunctionRowIter(conflicts), nil -} - -func (pm *PreviewMergeConflictsTableFunction) getConflictsForTable(ctx *sql.Context, sqledb dsess.SqlDatabase) ([]tableConflict, error) { - merger, err := merge.NewMerger(pm.leftRoot, pm.rightRoot, pm.baseRoot, pm.rightRoot, pm.baseRoot, pm.leftRoot.VRW(), pm.leftRoot.NodeStore()) + merger, err := merge.NewMerger(pm.leftRoot, pm.rightRoot, pm.baseRoot, pm.rightSrc, pm.ancestorSrc, pm.leftRoot.VRW(), pm.leftRoot.NodeStore()) if err != nil { return nil, err } @@ -330,9 +338,7 @@ func (pm *PreviewMergeConflictsTableFunction) getConflictsForTable(ctx *sql.Cont ReverifyAllConstraints: false, } - tblName := doltdb.TableName{Name: pm.tableNameExpr.String(), Schema: doltdb.DefaultSchemaName} - - tm, err := merger.MakeTableMerger(ctx, tblName, mergeOpts) + tm, err := merger.MakeTableMerger(ctx, pm.tblName, mergeOpts) if err != nil { return nil, err } @@ -343,28 +349,108 @@ func (pm *PreviewMergeConflictsTableFunction) getConflictsForTable(ctx *sql.Cont return nil, err } if finished != nil || stats != nil { - continue + return &previewMergeConflictsTableFunctionRowIter{}, nil } // Calculate a merge of the schemas, but don't apply it - mergeSch, schConflicts, _, diffInfo, err := tm.SchemaMerge(ctx, tblName) + mergeSch, schConflicts, _, diffInfo, err := tm.SchemaMerge(ctx, pm.tblName) if err != nil { return nil, err } - numSchemaConflicts := uint64(schConflicts.Count()) - if numSchemaConflicts > 0 { - conflicted = append(conflicted, tableConflict{tableName: tblName, numSchemaConflicts: &numSchemaConflicts}) + if schConflicts.Count() > 0 { // Cannot calculate data conflicts if there are schema conflicts - continue + return nil, fmt.Errorf("schema conflicts found: %d", schConflicts.Count()) + } + + if !tm.InvolvesRootObjects() { + if !dtypes.IsFormat_DOLT(pm.leftRoot.VRW().Format()) { + return nil, fmt.Errorf("preview_merge_conflicts table function only supports dolt format") + } + } else { + return nil, fmt.Errorf("Dolt does not operate on root objects") } - dataConflicts, err := getDataConflictsForTable(ctx, tm, tblName, mergeSch, diffInfo) + keyless := schema.IsKeyless(mergeSch) + + leftRows, err := tm.LeftRows(ctx) if err != nil { return nil, err } - if dataConflicts != nil { - conflicted = append(conflicted, *dataConflicts) + rightRows, err := tm.RightRows(ctx) + if err != nil { + return nil, err + } + ancRows, err := tm.AncRows(ctx) + if err != nil { + return nil, err } + rightHash, err := pm.rightSrc.HashOf() + if err != nil { + return nil, err + } + + baseHash, err := pm.ancestorSrc.HashOf() + if err != nil { + return nil, err + } + + kd := pm.baseSch.GetKeyDescriptor(pm.root.NodeStore()) + baseVD := pm.baseSch.GetValueDescriptor(pm.root.NodeStore()) + oursVD := pm.ourSch.GetValueDescriptor(pm.root.NodeStore()) + theirsVD := pm.theirSch.GetValueDescriptor(pm.root.NodeStore()) + + b := 1 + var o, t, n int + if !keyless { + o = b + kd.Count() + baseVD.Count() + t = o + kd.Count() + oursVD.Count() + 1 + n = t + kd.Count() + theirsVD.Count() + 2 + } else { + o = b + baseVD.Count() - 1 + t = o + oursVD.Count() + n = t + theirsVD.Count() + 4 + } + + valueMerger := tm.GetNewValueMerger(mergeSch, leftRows) + + differ, err := tree.NewThreeWayDiffer( + ctx, + leftRows.NodeStore(), + leftRows.Tuples(), + rightRows.Tuples(), + ancRows.Tuples(), + valueMerger.TryMerge, + keyless, + diffInfo, + leftRows.Tuples().Order, + ) + if err != nil { + return nil, err + } + + return &previewMergeConflictsTableFunctionRowIter{ + itr: differ, + tblName: pm.tblName, + vrw: pm.leftRoot.VRW(), + ns: leftRows.NodeStore(), + ourRows: leftRows, + keyless: keyless, + ourSch: pm.ourSch, + kd: kd, + baseVD: baseVD, + oursVD: oursVD, + theirsVD: theirsVD, + b: b, + o: o, + t: t, + n: n, + baseRootish: baseHash, + theirRootish: rightHash, + baseHash: baseHash, + theirHash: rightHash, + baseRows: ancRows, + theirRows: rightRows, + }, nil } // evaluateArguments returns leftBranchVal amd rightBranchVal. @@ -401,38 +487,238 @@ func (pm *PreviewMergeConflictsTableFunction) evaluateArguments() (interface{}, var _ sql.RowIter = &previewMergeConflictsTableFunctionRowIter{} type previewMergeConflictsTableFunctionRowIter struct { - conflicts []tableConflict - conIdx int + itr *tree.ThreeWayDiffer[val.Tuple, val.TupleDesc] + tblName doltdb.TableName + vrw dtypes.ValueReadWriter + ns tree.NodeStore + ourRows prolly.Map + keyless bool + ourSch schema.Schema + + kd val.TupleDesc + baseVD, oursVD, theirsVD val.TupleDesc + // offsets for each version + b, o, t int + n int + + baseHash, theirHash hash.Hash + baseRows, theirRows prolly.Map + baseRootish, theirRootish hash.Hash } -func (d *previewMergeConflictsTableFunctionRowIter) incrementIndexes() { - d.conIdx++ - if d.conIdx >= len(d.conflicts) { - d.conIdx = 0 - d.conflicts = nil +func (itr *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) { + if itr.itr == nil { + return nil, io.EOF } + + r := make(sql.Row, itr.n) + c, exists, err := itr.nextConflictVals(ctx) + if err != nil { + return nil, err + } + if !exists { + // Move on to next conflict + return itr.Next(ctx) + } + + r[0] = c.h.String() + + if !itr.keyless { + for i := 0; i < itr.kd.Count(); i++ { + f, err := tree.GetField(ctx, itr.kd, i, c.k, itr.baseRows.NodeStore()) + if err != nil { + return nil, err + } + if c.bV != nil { + r[itr.b+i] = f + } + if c.oV != nil { + r[itr.o+i] = f + } + if c.tV != nil { + r[itr.t+i] = f + } + } + + err = itr.putConflictRowVals(ctx, c, r) + if err != nil { + return nil, err + } + } else { + err = itr.putKeylessConflictRowVals(ctx, c, r) + if err != nil { + return nil, err + } + } + + return r, nil } -func NewPreviewMergeConflictsTableFunctionRowIter(pm []tableConflict) sql.RowIter { - return &previewMergeConflictsTableFunctionRowIter{ - conflicts: pm, +type conf struct { + k, bV, oV, tV val.Tuple + h hash.Hash + id string +} + +func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql.Context) (c conf, exists bool, err error) { + ca, err := itr.itr.Next(ctx) + if err != nil { + return conf{}, false, err } + isConflict := ca.Op == tree.DiffOpDivergentModifyConflict || ca.Op == tree.DiffOpDivergentDeleteConflict + isKeylessConflict := itr.keyless && (ca.Op == tree.DiffOpConvergentAdd || ca.Op == tree.DiffOpConvergentModify || ca.Op == tree.DiffOpConvergentDelete) + if !isConflict && !isKeylessConflict { + // If this is not a conflict, then we don't need to return anything. + return conf{}, false, nil + } + + c.k = ca.Key + c.h = itr.theirRootish + + // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. + b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() + c.id = base64.RawStdEncoding.EncodeToString(b[:]) + + err = itr.baseRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + c.bV = v + return nil + }) + if err != nil { + return conf{}, false, err + } + err = itr.ourRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + c.oV = v + return nil + }) + if err != nil { + return conf{}, false, err + } + err = itr.theirRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + c.tV = v + return nil + }) + if err != nil { + return conf{}, false, err + } + + return c, true, nil } -func (d *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) { - defer d.incrementIndexes() - if d.conIdx >= len(d.conflicts) { - return nil, io.EOF +func getDiffType(base val.Tuple, other val.Tuple) string { + if base == nil { + return merge.ConflictDiffTypeAdded + } else if other == nil { + return merge.ConflictDiffTypeRemoved } - if d.conflicts == nil { - return nil, io.EOF + // There has to be some edit, otherwise it wouldn't be a conflict... + return merge.ConflictDiffTypeModified +} + +func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sql.Context, c conf, r sql.Row) error { + if c.bV != nil { + for i := 0; i < itr.baseVD.Count(); i++ { + f, err := tree.GetField(ctx, itr.baseVD, i, c.bV, itr.baseRows.NodeStore()) + if err != nil { + return err + } + r[itr.b+itr.kd.Count()+i] = f + } + } + + if c.oV != nil { + for i := 0; i < itr.oursVD.Count(); i++ { + f, err := tree.GetField(ctx, itr.oursVD, i, c.oV, itr.baseRows.NodeStore()) + if err != nil { + return err + } + r[itr.o+itr.kd.Count()+i] = f + } + } + r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = getDiffType(c.bV, c.oV) + + if c.tV != nil { + for i := 0; i < itr.theirsVD.Count(); i++ { + f, err := tree.GetField(ctx, itr.theirsVD, i, c.tV, itr.baseRows.NodeStore()) + if err != nil { + return err + } + r[itr.t+itr.kd.Count()+i] = f + } } + r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = getDiffType(c.bV, c.tV) + r[itr.t+itr.kd.Count()+itr.theirsVD.Count()+1] = c.id - pm := d.conflicts[d.conIdx] - return getRowFromConflict(pm), nil + return nil } -func (d *previewMergeConflictsTableFunctionRowIter) Close(context *sql.Context) error { +func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals(ctx *sql.Context, c conf, r sql.Row) (err error) { + ns := itr.baseRows.NodeStore() + + if c.bV != nil { + // Cardinality + r[itr.n-3], err = tree.GetField(ctx, itr.baseVD, 0, c.bV, ns) + if err != nil { + return err + } + + for i := 0; i < itr.baseVD.Count()-1; i++ { + f, err := tree.GetField(ctx, itr.baseVD, i+1, c.bV, ns) + if err != nil { + return err + } + r[itr.b+i] = f + } + } else { + r[itr.n-3] = uint64(0) + } + + if c.oV != nil { + r[itr.n-2], err = tree.GetField(ctx, itr.oursVD, 0, c.oV, ns) + if err != nil { + return err + } + + for i := 0; i < itr.oursVD.Count()-1; i++ { + f, err := tree.GetField(ctx, itr.oursVD, i+1, c.oV, ns) + if err != nil { + return err + } + r[itr.o+i] = f + } + } else { + r[itr.n-2] = uint64(0) + } + + r[itr.o+itr.oursVD.Count()-1] = getDiffType(c.bV, c.oV) + + if c.tV != nil { + r[itr.n-1], err = tree.GetField(ctx, itr.theirsVD, 0, c.tV, ns) + if err != nil { + return err + } + + for i := 0; i < itr.theirsVD.Count()-1; i++ { + f, err := tree.GetField(ctx, itr.theirsVD, i+1, c.tV, ns) + if err != nil { + return err + } + r[itr.t+i] = f + } + } else { + r[itr.n-1] = uint64(0) + } + + o := itr.t + itr.theirsVD.Count() - 1 + r[o] = getDiffType(c.bV, c.tV) + r[itr.n-4] = c.id + return nil } + +func (d *previewMergeConflictsTableFunctionRowIter) Close(context *sql.Context) error { + if d.itr == nil { + return nil + } + return d.itr.Close() +} diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go index c85e145e836..9e373d84600 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go @@ -289,49 +289,49 @@ func getRowFromConflict(conflict tableConflict) sql.Row { } // resolveBranchesToRoots resolves branch names to their corresponding root values -// and finds the common merge base. Returns left root, right root, and base root. -func resolveBranchesToRoots(ctx *sql.Context, db dsess.SqlDatabase, leftBranch, rightBranch string) (doltdb.RootValue, doltdb.RootValue, doltdb.RootValue, error) { +// and finds the common merge base. +func resolveBranchesToRoots(ctx *sql.Context, db dsess.SqlDatabase, leftBranch, rightBranch string) (doltdb.RootValue, doltdb.RootValue, doltdb.RootValue, doltdb.Rootish, doltdb.Rootish, error) { sess := dsess.DSessFromSess(ctx.Session) headRef, err := sess.CWBHeadRef(ctx, db.Name()) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } leftCm, err := resolveCommit(ctx, db.DbData().Ddb, headRef, leftBranch) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } rightCm, err := resolveCommit(ctx, db.DbData().Ddb, headRef, rightBranch) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - optCmt, err := doltdb.GetCommitAncestor(ctx, leftCm, rightCm) + optCm, err := doltdb.GetCommitAncestor(ctx, leftCm, rightCm) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - mergeBase, ok := optCmt.ToCommit() + ancCm, ok := optCm.ToCommit() if !ok { - return nil, nil, nil, doltdb.ErrGhostCommitEncountered + return nil, nil, nil, nil, nil, doltdb.ErrGhostCommitEncountered } rightRoot, err := rightCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } leftRoot, err := leftCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - baseRoot, err := mergeBase.GetRootValue(ctx) + baseRoot, err := ancCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - return leftRoot, rightRoot, baseRoot, nil + return leftRoot, rightRoot, baseRoot, rightCm, ancCm, nil } type tableConflict struct { @@ -344,7 +344,7 @@ type tableConflict struct { // a list of tables that would have conflicts. It performs a dry-run merge // to identify both schema and data conflicts without modifying the database. func getTablesWithConflicts(ctx *sql.Context, db dsess.SqlDatabase, baseBranch, mergeBranch string) ([]tableConflict, error) { - leftRoot, rightRoot, baseRoot, err := resolveBranchesToRoots(ctx, db, baseBranch, mergeBranch) + leftRoot, rightRoot, baseRoot, _, _, err := resolveBranchesToRoots(ctx, db, baseBranch, mergeBranch) if err != nil { return nil, err } diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 75482c63e7f..49a9c4677db 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -88,7 +88,7 @@ type ProllyConflictsTable struct { rs RootSetter artM prolly.ArtifactMap sqlTable sql.UpdatableTable - versionMappings *versionMappings + versionMappings *VersionMappings } var _ sql.UpdatableTable = ProllyConflictsTable{} @@ -233,7 +233,6 @@ func (itr *prollyConflictRowIter) Next(ctx *sql.Context) (sql.Row, error) { return nil, err } } else { - err = itr.putKeylessConflictRowVals(ctx, c, r) if err != nil { return nil, err @@ -473,7 +472,7 @@ func (itr *prollyConflictRowIter) Close(ctx *sql.Context) error { type prollyConflictOurTableUpdater struct { baseSch, ourSch, theirSch schema.Schema srcUpdater sql.RowUpdater - versionMappings *versionMappings + versionMappings *VersionMappings pkOrdinals []int schemaOK bool } @@ -684,12 +683,12 @@ func (cd *prollyConflictDeleter) Close(ctx *sql.Context) error { return cd.ct.rs.SetRoot(ctx, updatedRoot) } -type versionMappings struct { +type VersionMappings struct { ourMapping, theirMapping, baseMapping val.OrdinalMapping } // returns the schema of the rows returned by the conflicts table and a mappings between each version and the source table. -func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *versionMappings, error) { +func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *VersionMappings, error) { keyless := schema.IsKeyless(ours) n := 4 + ours.GetAllCols().Size() + theirs.GetAllCols().Size() + base.GetAllCols().Size() if keyless { @@ -774,7 +773,7 @@ func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * } return sch, - &versionMappings{ + &VersionMappings{ ourMapping: ourColMapping, theirMapping: theirColMapping, baseMapping: baseColMapping}, diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 12555e4eeb9..85e2681b03b 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -552,6 +552,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'feature-branch')", Expected: []sql.Row{{"test", uint64(1), uint64(0)}}, }, + { + Query: "SELECT COUNT(*) FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + Expected: []sql.Row{{1}}, + }, { Query: "SELECT is_merging, source, target, unmerged_tables FROM DOLT_MERGE_STATUS;", Expected: []sql.Row{{false, nil, nil, nil}}, @@ -584,6 +588,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_conflicts", Expected: []sql.Row{{"test", uint64(1)}}, }, + { + Query: "SELECT count(*) FROM dolt_conflicts_test", + Expected: []sql.Row{{1}}, + }, { Query: "DELETE FROM dolt_conflicts_test", Expected: []sql.Row{{types.NewOkResult(1)}}, From 735eb38147d136b96d65c403440875774c993299 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 29 May 2025 12:09:01 -0700 Subject: [PATCH 03/12] Fixes, tests --- .../dolt_preview_merge_conflicts.go | 88 ++++-- .../sqle/dtables/conflicts_tables_prolly.go | 10 +- .../sqle/enginetest/dolt_engine_test.go | 10 + .../sqle/enginetest/dolt_engine_tests.go | 22 ++ .../sqle/enginetest/dolt_queries_merge.go | 269 +++++++++++++++++- .../enginetest/dolt_queries_schema_merge.go | 4 + 6 files changed, 377 insertions(+), 26 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index ecc950260bf..e640a75f1ed 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -20,6 +20,7 @@ import ( "io" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/schema" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" @@ -446,10 +447,6 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. n: n, baseRootish: baseHash, theirRootish: rightHash, - baseHash: baseHash, - theirHash: rightHash, - baseRows: ancRows, - theirRows: rightRows, }, nil } @@ -576,9 +573,19 @@ func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql. c.h = itr.theirRootish // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. - b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() + // TODO: This is mutating the key, which is creating a schema with a different capacity than expected + // b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() + buf := make([]byte, len(ca.Key)+len(c.h)) + copy(buf, ca.Key) + copy(buf[len(ca.Key):], c.h[:]) + b := xxh3.Hash128(buf).Bytes() c.id = base64.RawStdEncoding.EncodeToString(b[:]) + err = itr.loadTableMaps(ctx, itr.baseRootish, itr.theirRootish) + if err != nil { + return conf{}, false, err + } + err = itr.baseRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { c.bV = v return nil @@ -604,15 +611,64 @@ func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql. return c, true, nil } -func getDiffType(base val.Tuple, other val.Tuple) string { - if base == nil { - return merge.ConflictDiffTypeAdded - } else if other == nil { - return merge.ConflictDiffTypeRemoved +// loadTableMaps loads the maps specified in the metadata if they are different from +// the currently loaded maps. |baseHash| and |theirHash| are table hashes. +func (itr *previewMergeConflictsTableFunctionRowIter) loadTableMaps(ctx *sql.Context, baseHash, theirHash hash.Hash) error { + if itr.baseHash.Compare(baseHash) != 0 { + rv, err := doltdb.LoadRootValueFromRootIshAddr(ctx, itr.vrw, itr.ns, baseHash) + if err != nil { + return err + } + baseTbl, ok, err := rv.GetTable(ctx, itr.tblName) + if err != nil { + return err + } + + var idx durable.Index + if !ok { + idx, err = durable.NewEmptyPrimaryIndex(ctx, itr.vrw, itr.ns, itr.ourSch) + } else { + idx, err = baseTbl.GetRowData(ctx) + } + + if err != nil { + return err + } + + itr.baseRows, err = durable.ProllyMapFromIndex(idx) + if err != nil { + return err + } + + itr.baseHash = baseHash + } + + if itr.theirHash.Compare(theirHash) != 0 { + rv, err := doltdb.LoadRootValueFromRootIshAddr(ctx, itr.vrw, itr.ns, theirHash) + if err != nil { + return err + } + + theirTbl, ok, err := rv.GetTable(ctx, itr.tblName) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("failed to find table %s in right root value", itr.tblName) + } + + idx, err := theirTbl.GetRowData(ctx) + if err != nil { + return err + } + itr.theirRows, err = durable.ProllyMapFromIndex(idx) + if err != nil { + return err + } + itr.theirHash = theirHash } - // There has to be some edit, otherwise it wouldn't be a conflict... - return merge.ConflictDiffTypeModified + return nil } func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sql.Context, c conf, r sql.Row) error { @@ -635,7 +691,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sq r[itr.o+itr.kd.Count()+i] = f } } - r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = getDiffType(c.bV, c.oV) + r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = dtables.GetConflictDiffType(c.bV, c.oV) if c.tV != nil { for i := 0; i < itr.theirsVD.Count(); i++ { @@ -646,7 +702,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sq r[itr.t+itr.kd.Count()+i] = f } } - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = getDiffType(c.bV, c.tV) + r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = dtables.GetConflictDiffType(c.bV, c.tV) r[itr.t+itr.kd.Count()+itr.theirsVD.Count()+1] = c.id return nil @@ -690,7 +746,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals( r[itr.n-2] = uint64(0) } - r[itr.o+itr.oursVD.Count()-1] = getDiffType(c.bV, c.oV) + r[itr.o+itr.oursVD.Count()-1] = dtables.GetConflictDiffType(c.bV, c.oV) if c.tV != nil { r[itr.n-1], err = tree.GetField(ctx, itr.theirsVD, 0, c.tV, ns) @@ -710,7 +766,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals( } o := itr.t + itr.theirsVD.Count() - 1 - r[o] = getDiffType(c.bV, c.tV) + r[o] = dtables.GetConflictDiffType(c.bV, c.tV) r[itr.n-4] = c.id return nil diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 49a9c4677db..064ae452330 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -262,7 +262,7 @@ func (itr *prollyConflictRowIter) putConflictRowVals(ctx *sql.Context, c conf, r r[itr.o+itr.kd.Count()+i] = f } } - r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = getDiffType(c.bV, c.oV) + r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = GetConflictDiffType(c.bV, c.oV) if c.tV != nil { for i := 0; i < itr.theirsVD.Count(); i++ { @@ -273,13 +273,13 @@ func (itr *prollyConflictRowIter) putConflictRowVals(ctx *sql.Context, c conf, r r[itr.t+itr.kd.Count()+i] = f } } - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = getDiffType(c.bV, c.tV) + r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = GetConflictDiffType(c.bV, c.tV) r[itr.t+itr.kd.Count()+itr.theirsVD.Count()+1] = c.id return nil } -func getDiffType(base val.Tuple, other val.Tuple) string { +func GetConflictDiffType(base val.Tuple, other val.Tuple) string { if base == nil { return merge.ConflictDiffTypeAdded } else if other == nil { @@ -328,7 +328,7 @@ func (itr *prollyConflictRowIter) putKeylessConflictRowVals(ctx *sql.Context, c r[itr.n-2] = uint64(0) } - r[itr.o+itr.oursVD.Count()-1] = getDiffType(c.bV, c.oV) + r[itr.o+itr.oursVD.Count()-1] = GetConflictDiffType(c.bV, c.oV) if c.tV != nil { r[itr.n-1], err = tree.GetField(ctx, itr.theirsVD, 0, c.tV, ns) @@ -348,7 +348,7 @@ func (itr *prollyConflictRowIter) putKeylessConflictRowVals(ctx *sql.Context, c } o := itr.t + itr.theirsVD.Count() - 1 - r[o] = getDiffType(c.bV, c.tV) + r[o] = GetConflictDiffType(c.bV, c.tV) r[itr.n-4] = c.id return nil diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 8d890d28f80..de51b7f5119 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -1342,6 +1342,16 @@ func TestDoltMergeArtifacts(t *testing.T) { RunDoltMergeArtifacts(t, h) } +func TestDoltPreviewMergeConflictsSummary(t *testing.T) { + h := newDoltEnginetestHarness(t) + RunDoltPreviewMergeConflictsSummaryTests(t, h) +} + +func TestDoltPreviewMergeConflictsSummaryPrepared(t *testing.T) { + h := newDoltEnginetestHarness(t) + RunDoltPreviewMergeConflictsSummaryPreparedTests(t, h) +} + func TestDoltPreviewMergeConflicts(t *testing.T) { h := newDoltEnginetestHarness(t) RunDoltPreviewMergeConflictsTests(t, h) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go index 782c8e552d4..36dd3e489ea 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go @@ -988,6 +988,28 @@ func RunDoltMergeArtifacts(t *testing.T, h DoltEnginetestHarness) { } } +func RunDoltPreviewMergeConflictsSummaryTests(t *testing.T, h DoltEnginetestHarness) { + for _, script := range PreviewMergeConflictsSummaryFunctionScripts { + // harness can't reset effectively. Use a new harness for each script + func() { + h := h.NewHarness(t) + defer h.Close() + enginetest.TestScript(t, h, script) + }() + } +} + +func RunDoltPreviewMergeConflictsSummaryPreparedTests(t *testing.T, h DoltEnginetestHarness) { + for _, script := range PreviewMergeConflictsSummaryFunctionScripts { + // harness can't reset effectively. Use a new harness for each script + func() { + h := h.NewHarness(t) + defer h.Close() + enginetest.TestScript(t, h, script) + }() + } +} + func RunDoltPreviewMergeConflictsTests(t *testing.T, h DoltEnginetestHarness) { for _, script := range PreviewMergeConflictsFunctionScripts { // harness can't reset effectively. Use a new harness for each script diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 85e2681b03b..ddbe0aad001 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -351,6 +351,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'feature-branch')", ExpectedErrStr: "this operation is not supported while in a detached head state", }, + { + Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + ExpectedErrStr: "this operation is not supported while in a detached head state", + }, { Query: "CALL DOLT_MERGE('feature-branch')", ExpectedErrStr: "this operation is not supported while in a detached head state", @@ -553,8 +557,8 @@ var MergeScripts = []queries.ScriptTest{ Expected: []sql.Row{{"test", uint64(1), uint64(0)}}, }, { - Query: "SELECT COUNT(*) FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", - Expected: []sql.Row{{1}}, + Query: "SELECT base_pk, base_val, our_pk, our_val, our_diff_type, their_pk, their_val, their_diff_type FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + Expected: []sql.Row{{0, 0, 0, 1001, "modified", 0, 1000, "modified"}}, }, { Query: "SELECT is_merging, source, target, unmerged_tables FROM DOLT_MERGE_STATUS;", @@ -589,8 +593,8 @@ var MergeScripts = []queries.ScriptTest{ Expected: []sql.Row{{"test", uint64(1)}}, }, { - Query: "SELECT count(*) FROM dolt_conflicts_test", - Expected: []sql.Row{{1}}, + Query: "SELECT base_pk, base_val, our_pk, our_val, our_diff_type, their_pk, their_val, their_diff_type FROM dolt_conflicts_test", + Expected: []sql.Row{{0, 0, 0, 1001, "modified", 0, 1000, "modified"}}, }, { Query: "DELETE FROM dolt_conflicts_test", @@ -1084,6 +1088,11 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('HEAD', 'HEAD~1')", Expected: []sql.Row{}, }, + { + Skip: true, // TODO: could not find tbl test in right root value + Query: "SELECT * FROM dolt_preview_merge_conflicts('HEAD', 'HEAD~1', 'test')", + Expected: []sql.Row{}, + }, { Query: "CALL DOLT_MERGE('HEAD~1')", Expected: []sql.Row{{"", 0, 0, "cannot fast forward from a to b. a is ahead of b already"}}, @@ -1092,6 +1101,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('HEAD', 'HEAD')", Expected: []sql.Row{}, }, + { + Query: "SELECT * FROM dolt_preview_merge_conflicts('HEAD', 'HEAD', 'test')", + Expected: []sql.Row{}, + }, { Query: "CALL DOLT_MERGE('HEAD')", Expected: []sql.Row{{"", 0, 0, "Everything up-to-date"}}, @@ -1119,6 +1132,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'feature-branch')", Expected: []sql.Row{{"test", uint64(1), uint64(0)}}, }, + { + Query: "SELECT COUNT(*) FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + Expected: []sql.Row{{1}}, + }, { Query: "CALL DOLT_MERGE('feature-branch')", Expected: []sql.Row{{"", 0, 1, "conflicts found"}}, @@ -1272,6 +1289,10 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'feature-branch')", Expected: []sql.Row{}, }, + { + Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'feature-branch', 'test')", + Expected: []sql.Row{}, + }, { Query: "CALL DOLT_MERGE('feature-branch', '-m', 'this is a merge')", ExpectedErrStr: "error: local changes would be stomped by merge:\n\ttest\n Please commit your changes before you merge.", @@ -1304,6 +1325,11 @@ var MergeScripts = []queries.ScriptTest{ Query: "SELECT * FROM dolt_preview_merge_conflicts_summary('main', 'b1')", Expected: []sql.Row{}, }, + { + Skip: true, // TODO: could not find tbl test in left root value + Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'b1', 'test')", + Expected: []sql.Row{}, + }, { Query: "call dolt_merge('b1')", Expected: []sql.Row{{doltCommit, 0, 0, "merge successful"}}, @@ -4643,7 +4669,7 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, } -var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ +var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ { Name: "invalid arguments", SetUpScript: []string{ @@ -5176,6 +5202,239 @@ var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ }, } +var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ + { + Name: "invalid arguments", + SetUpScript: []string{ + "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", + "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", + "call dolt_add('.')", + "call dolt_commit('-am', 'creating table t');", + + "call dolt_branch('branch1')", + "call dolt_branch('branch2')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT * from dolt_preview_merge_conflicts();", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('t');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't', 'extra');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(null, null, null);", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 123, 't');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(123, 'branch1', 't');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 123);", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('fake-branch', 'main', 't');", + ExpectedErrStr: "branch not found: fake-branch", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'fake-branch', 't');", + ExpectedErrStr: "branch not found: fake-branch", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main...branch1', 'branch2', 't');", + ExpectedErrStr: "string is not a valid branch or hash", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', concat('branch', '1'), 't');", + ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(hashof('main'), 'branch1', 't');", + ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 'nope');", + ExpectedErrStr: "could not find tbl nope in left root value", + }, + }, + }, + { + Name: "basic case with single table", + SetUpScript: []string{ + "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", + "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", + "call dolt_add('.')", + "set @Commit1 = '';", + "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", + + "call dolt_branch('branch1')", + "call dolt_checkout('-b', 'branch2')", + "update t set c1='one!' where pk=1", + "set @Commit2 = '';", + "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", + + "call dolt_checkout('branch1')", + "update t set c1='one?' where pk=1", + "set @Commit3 = '';", + "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", + + "call dolt_checkout('main')", + "call dolt_merge('branch1')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, + }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one!", "two", "modified", 1, "one?", "two", "modified"}}, + }, + }, + }, + { + Name: "basic case with keyless table", + SetUpScript: []string{ + "create table t (pk int, c1 varchar(20), c2 varchar(20));", + "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", + "call dolt_add('.')", + "set @Commit1 = '';", + "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", + + "call dolt_branch('branch1')", + "call dolt_checkout('-b', 'branch2')", + "update t set c1='one!' where pk=1", + "set @Commit2 = '';", + "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", + + "call dolt_checkout('branch1')", + "update t set c1='one?' where pk=1", + "set @Commit3 = '';", + "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", + + "call dolt_checkout('main')", + "call dolt_merge('branch1')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1}}, + }, + }, + }, + { + Name: "basic case with multiple tables", + SetUpScript: []string{ + "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", + "create table t2 (pk int primary key, c1 varchar(20));", + "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", + "insert into t2 values(100, 'hundred');", + "call dolt_add('.')", + "set @Commit1 = '';", + "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", + + "call dolt_branch('branch1')", + "call dolt_checkout('-b', 'branch2')", + "update t set c1='one!' where pk=1", + "update t2 set c1='hundred!' where pk=100", + "set @Commit2 = '';", + "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", + + "call dolt_checkout('branch1')", + "update t set c1='one?' where pk=1", + "update t2 set c1='hundred?' where pk=100", + "set @Commit3 = '';", + "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", + + "call dolt_checkout('main')", + "call dolt_merge('branch1')", + + "create table keyless (id int);", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't2')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't2')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't2')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't2')", + Expected: []sql.Row{{1}}, + }, + }, + }, +} + // convertMergeScriptTest converts a MergeScriptTest into a standard ScriptTest. If flipSides is true, then the // left and right setup is swapped (i.e. left setup is done on right branch and right setup is done on main branch). // This enables us to test merges in both directions, since the merge code is asymmetric and some code paths currently diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_schema_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_schema_merge.go index 0bff7e870bf..4628a552a42 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_schema_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_schema_merge.go @@ -1595,6 +1595,10 @@ var SchemaChangeTestsTypeChanges = []MergeScriptTest{ Query: "select * from dolt_preview_merge_conflicts_summary('main', 'right');", Expected: []sql.Row{{"t", nil, uint64(1)}}, }, + { + Query: "select * from dolt_preview_merge_conflicts('main', 'right', 't');", + ExpectedErrStr: "schema conflicts found: 1", + }, { Query: "call dolt_merge('right');", Expected: []sql.Row{{"", 0, 1, "conflicts found"}}, From 7507de16cbb68b7eca96e6228ba36284dcba5e91 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 29 May 2025 16:49:02 -0700 Subject: [PATCH 04/12] Fixes --- .../sqle/dtablefunctions/dolt_diff.go | 2 +- .../dolt_preview_merge_conflicts.go | 111 +++++++++--------- .../dolt_preview_merge_conflicts_summary.go | 34 ++++-- .../sqle/dtables/conflicts_tables_prolly.go | 10 +- 4 files changed, 82 insertions(+), 75 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go index a3f0e477cf9..adda8d38948 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_diff.go @@ -315,7 +315,7 @@ func loadCommitStrings(ctx *sql.Context, fromRef, toRef, dotRef interface{}, db func interfaceToString(r interface{}) (string, error) { str, ok := r.(string) if !ok { - return "", fmt.Errorf("received '%v' when expecting commit hash string", str) + return "", fmt.Errorf("received '%v' when expecting commit hash string", r) } return str, nil } diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index e640a75f1ed..6d0b39d28ad 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -41,6 +41,7 @@ var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil) var _ sql.ExecSourceRel = (*PreviewMergeConflictsTableFunction)(nil) var _ sql.AuthorizationCheckerNode = (*PreviewMergeConflictsTableFunction)(nil) + type PreviewMergeConflictsTableFunction struct { ctx *sql.Context leftBranchExpr sql.Expression @@ -48,15 +49,14 @@ type PreviewMergeConflictsTableFunction struct { tableNameExpr sql.Expression database sql.Database - root, leftRoot, rightRoot, baseRoot doltdb.RootValue - tblName doltdb.TableName - sqlSch sql.PrimaryKeySchema - baseSch, ourSch, theirSch schema.Schema - rightSrc, ancestorSrc doltdb.Rootish + rootInfo rootInfo + tblName doltdb.TableName + sqlSch sql.PrimaryKeySchema + baseSch, ourSch, theirSch schema.Schema } // NewInstance creates a new instance of TableFunction interface -func (ds *PreviewMergeConflictsTableFunction) NewInstance(ctx *sql.Context, db sql.Database, expressions []sql.Expression) (sql.Node, error) { +func (pm *PreviewMergeConflictsTableFunction) NewInstance(ctx *sql.Context, db sql.Database, expressions []sql.Expression) (sql.Node, error) { newInstance := &PreviewMergeConflictsTableFunction{ ctx: ctx, database: db, @@ -70,74 +70,74 @@ func (ds *PreviewMergeConflictsTableFunction) NewInstance(ctx *sql.Context, db s return node, nil } -func (ds *PreviewMergeConflictsTableFunction) DataLength(ctx *sql.Context) (uint64, error) { - numBytesPerRow := schema.SchemaAvgLength(ds.Schema()) - numRows, _, err := ds.RowCount(ctx) +func (pm *PreviewMergeConflictsTableFunction) DataLength(ctx *sql.Context) (uint64, error) { + numBytesPerRow := schema.SchemaAvgLength(pm.Schema()) + numRows, _, err := pm.RowCount(ctx) if err != nil { return 0, err } return numBytesPerRow * numRows, nil } -func (ds *PreviewMergeConflictsTableFunction) RowCount(_ *sql.Context) (uint64, bool, error) { +func (pm *PreviewMergeConflictsTableFunction) RowCount(_ *sql.Context) (uint64, bool, error) { return previewMergeConflictsDefaultRowCount, false, nil } // Database implements the sql.Databaser interface -func (ds *PreviewMergeConflictsTableFunction) Database() sql.Database { - return ds.database +func (pm *PreviewMergeConflictsTableFunction) Database() sql.Database { + return pm.database } // WithDatabase implements the sql.Databaser interface -func (ds *PreviewMergeConflictsTableFunction) WithDatabase(database sql.Database) (sql.Node, error) { - nds := *ds - nds.database = database - return &nds, nil +func (pm *PreviewMergeConflictsTableFunction) WithDatabase(database sql.Database) (sql.Node, error) { + npm := *pm + npm.database = database + return &npm, nil } // Name implements the sql.TableFunction interface -func (ds *PreviewMergeConflictsTableFunction) Name() string { +func (pm *PreviewMergeConflictsTableFunction) Name() string { return "dolt_preview_merge_conflicts" } // Resolved implements the sql.Resolvable interface -func (ds *PreviewMergeConflictsTableFunction) Resolved() bool { - return ds.leftBranchExpr.Resolved() && ds.rightBranchExpr.Resolved() && ds.tableNameExpr.Resolved() +func (pm *PreviewMergeConflictsTableFunction) Resolved() bool { + return pm.leftBranchExpr.Resolved() && pm.rightBranchExpr.Resolved() && pm.tableNameExpr.Resolved() } -func (ds *PreviewMergeConflictsTableFunction) IsReadOnly() bool { +func (pm *PreviewMergeConflictsTableFunction) IsReadOnly() bool { return true } // String implements the Stringer interface -func (ds *PreviewMergeConflictsTableFunction) String() string { - return fmt.Sprintf("DOLT_PREVIEW_MERGE_CONFLICTS(%s, %s, %s)", ds.leftBranchExpr.String(), ds.rightBranchExpr.String(), ds.tableNameExpr.String()) +func (pm *PreviewMergeConflictsTableFunction) String() string { + return fmt.Sprintf("DOLT_PREVIEW_MERGE_CONFLICTS(%s, %s, %s)", pm.leftBranchExpr.String(), pm.rightBranchExpr.String(), pm.tableNameExpr.String()) } // Schema implements the sql.Node interface. -func (ds *PreviewMergeConflictsTableFunction) Schema() sql.Schema { - if !ds.Resolved() { +func (pm *PreviewMergeConflictsTableFunction) Schema() sql.Schema { + if !pm.Resolved() { return nil } - if ds.sqlSch.Schema == nil { + if pm.sqlSch.Schema == nil { panic("schema hasn't been generated yet") } - return ds.sqlSch.Schema + return pm.sqlSch.Schema } // Children implements the sql.Node interface. -func (ds *PreviewMergeConflictsTableFunction) Children() []sql.Node { +func (pm *PreviewMergeConflictsTableFunction) Children() []sql.Node { return nil } // WithChildren implements the sql.Node interface. -func (ds *PreviewMergeConflictsTableFunction) WithChildren(children ...sql.Node) (sql.Node, error) { +func (pm *PreviewMergeConflictsTableFunction) WithChildren(children ...sql.Node) (sql.Node, error) { if len(children) != 0 { return nil, fmt.Errorf("unexpected children") } - return ds, nil + return pm, nil } // CheckAuth implements the interface sql.AuthorizationCheckerNode. @@ -232,18 +232,13 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l return err } - leftRoot, rightRoot, baseRoot, rightSrc, ancestorSrc, err := resolveBranchesToRoots(ctx, sqledb, leftBranch, rightBranch) - if err != nil { - return err - } - - root, err := sqledb.GetRoot(ctx) + ri, err := resolveBranchesToRoots(ctx, sqledb, leftBranch, rightBranch) if err != nil { return err } tblName := doltdb.TableName{Name: tableName, Schema: doltdb.DefaultSchemaName} - baseSch, ourSch, theirSch, err := getConflictSchemasFromRoots(ctx, tblName, leftRoot, rightRoot, baseRoot) + baseSch, ourSch, theirSch, err := getConflictSchemasFromRoots(ctx, tblName, ri.leftRoot, ri.rightRoot, ri.baseRoot) if err != nil { return err } @@ -259,12 +254,7 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l } pm.sqlSch = sqlSch - pm.root = root - pm.leftRoot = leftRoot - pm.rightRoot = rightRoot - pm.baseRoot = baseRoot - pm.rightSrc = rightSrc - pm.ancestorSrc = ancestorSrc + pm.rootInfo = ri pm.tblName = tblName pm.baseSch = baseSch pm.ourSch = ourSch @@ -278,10 +268,7 @@ func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, lef if err != nil { return nil, nil, nil, err } - if !ourOk { - return nil, nil, nil, fmt.Errorf("could not find tbl %s in left root value", tblName) - } - + baseTbl, baseOk, err := baseRoot.GetTable(ctx, tblName) if err != nil { return nil, nil, nil, err @@ -290,6 +277,10 @@ func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, lef if err != nil { return nil, nil, nil, err } + + if !ourOk { + return nil, nil, nil, fmt.Errorf("could not find tbl %s in left root value", tblName) + } if !theirOK { return nil, nil, nil, fmt.Errorf("could not find tbl %s in right root value", tblName) } @@ -327,8 +318,9 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. if pm.sqlSch.Schema == nil { panic("schema hasn't been generated yet") } + - merger, err := merge.NewMerger(pm.leftRoot, pm.rightRoot, pm.baseRoot, pm.rightSrc, pm.ancestorSrc, pm.leftRoot.VRW(), pm.leftRoot.NodeStore()) + merger, err := merge.NewMerger(pm.rootInfo.leftRoot, pm.rootInfo.rightRoot, pm.rootInfo.baseRoot, pm.rootInfo.rightCm, pm.rootInfo.ancCm, pm.rootInfo.leftRoot.VRW(), pm.rootInfo.leftRoot.NodeStore()) if err != nil { return nil, err } @@ -363,7 +355,7 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. } if !tm.InvolvesRootObjects() { - if !dtypes.IsFormat_DOLT(pm.leftRoot.VRW().Format()) { + if !dtypes.IsFormat_DOLT(pm.rootInfo.leftRoot.VRW().Format()) { return nil, fmt.Errorf("preview_merge_conflicts table function only supports dolt format") } } else { @@ -385,20 +377,20 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. return nil, err } - rightHash, err := pm.rightSrc.HashOf() + rightHash, err := pm.rootInfo.rightCm.HashOf() if err != nil { return nil, err } - baseHash, err := pm.ancestorSrc.HashOf() + baseHash, err := pm.rootInfo.ancCm.HashOf() if err != nil { return nil, err } - kd := pm.baseSch.GetKeyDescriptor(pm.root.NodeStore()) - baseVD := pm.baseSch.GetValueDescriptor(pm.root.NodeStore()) - oursVD := pm.ourSch.GetValueDescriptor(pm.root.NodeStore()) - theirsVD := pm.theirSch.GetValueDescriptor(pm.root.NodeStore()) + kd := pm.baseSch.GetKeyDescriptor(pm.rootInfo.baseRoot.NodeStore()) + baseVD := pm.baseSch.GetValueDescriptor(pm.rootInfo.baseRoot.NodeStore()) + oursVD := pm.ourSch.GetValueDescriptor(pm.rootInfo.leftRoot.NodeStore()) + theirsVD := pm.theirSch.GetValueDescriptor(pm.rootInfo.rightRoot.NodeStore()) b := 1 var o, t, n int @@ -432,7 +424,7 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. return &previewMergeConflictsTableFunctionRowIter{ itr: differ, tblName: pm.tblName, - vrw: pm.leftRoot.VRW(), + vrw: pm.rootInfo.leftRoot.VRW(), ns: leftRows.NodeStore(), ourRows: leftRows, keyless: keyless, @@ -474,6 +466,10 @@ func (pm *PreviewMergeConflictsTableFunction) evaluateArguments() (interface{}, return nil, nil, "", ErrInvalidTableName.New(pm.tableNameExpr.String()) } + if tableName == "" { + return nil, nil, "", fmt.Errorf("table name cannot be empty") + } + return leftBranchVal, rightBranchVal, tableName, nil } @@ -572,9 +568,12 @@ func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql. c.k = ca.Key c.h = itr.theirRootish - // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. + if len(ca.Key) == 0 { + return conf{}, false, fmt.Errorf("empty key found in conflict") + } // TODO: This is mutating the key, which is creating a schema with a different capacity than expected // b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() + // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. buf := make([]byte, len(ca.Key)+len(c.h)) copy(buf, ca.Key) copy(buf[len(ca.Key):], c.h[:]) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go index 9e373d84600..ee9018b3904 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go @@ -288,50 +288,58 @@ func getRowFromConflict(conflict tableConflict) sql.Row { return row } +type rootInfo struct { + leftRoot doltdb.RootValue + rightRoot doltdb.RootValue + baseRoot doltdb.RootValue + rightCm doltdb.Rootish + ancCm doltdb.Rootish +} + // resolveBranchesToRoots resolves branch names to their corresponding root values // and finds the common merge base. -func resolveBranchesToRoots(ctx *sql.Context, db dsess.SqlDatabase, leftBranch, rightBranch string) (doltdb.RootValue, doltdb.RootValue, doltdb.RootValue, doltdb.Rootish, doltdb.Rootish, error) { +func resolveBranchesToRoots(ctx *sql.Context, db dsess.SqlDatabase, leftBranch, rightBranch string) (rootInfo, error) { sess := dsess.DSessFromSess(ctx.Session) headRef, err := sess.CWBHeadRef(ctx, db.Name()) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } leftCm, err := resolveCommit(ctx, db.DbData().Ddb, headRef, leftBranch) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } rightCm, err := resolveCommit(ctx, db.DbData().Ddb, headRef, rightBranch) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } optCm, err := doltdb.GetCommitAncestor(ctx, leftCm, rightCm) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } ancCm, ok := optCm.ToCommit() if !ok { - return nil, nil, nil, nil, nil, doltdb.ErrGhostCommitEncountered + return rootInfo{}, doltdb.ErrGhostCommitEncountered } rightRoot, err := rightCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } leftRoot, err := leftCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } baseRoot, err := ancCm.GetRootValue(ctx) if err != nil { - return nil, nil, nil, nil, nil, err + return rootInfo{}, err } - return leftRoot, rightRoot, baseRoot, rightCm, ancCm, nil + return rootInfo{leftRoot, rightRoot, baseRoot, rightCm, ancCm}, nil } type tableConflict struct { @@ -344,17 +352,17 @@ type tableConflict struct { // a list of tables that would have conflicts. It performs a dry-run merge // to identify both schema and data conflicts without modifying the database. func getTablesWithConflicts(ctx *sql.Context, db dsess.SqlDatabase, baseBranch, mergeBranch string) ([]tableConflict, error) { - leftRoot, rightRoot, baseRoot, _, _, err := resolveBranchesToRoots(ctx, db, baseBranch, mergeBranch) + ri, err := resolveBranchesToRoots(ctx, db, baseBranch, mergeBranch) if err != nil { return nil, err } - merger, err := merge.NewMerger(leftRoot, rightRoot, baseRoot, rightRoot, baseRoot, leftRoot.VRW(), leftRoot.NodeStore()) + merger, err := merge.NewMerger(ri.leftRoot, ri.rightRoot, ri.baseRoot, ri.rightRoot, ri.baseRoot, ri.leftRoot.VRW(), ri.leftRoot.NodeStore()) if err != nil { return nil, err } - tblNames, err := doltdb.UnionTableNames(ctx, leftRoot, rightRoot) + tblNames, err := doltdb.UnionTableNames(ctx, ri.leftRoot, ri.rightRoot) if err != nil { return nil, err } diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 064ae452330..6c1e5d4c81e 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -88,7 +88,7 @@ type ProllyConflictsTable struct { rs RootSetter artM prolly.ArtifactMap sqlTable sql.UpdatableTable - versionMappings *VersionMappings + versionMappings *versionMappings } var _ sql.UpdatableTable = ProllyConflictsTable{} @@ -472,7 +472,7 @@ func (itr *prollyConflictRowIter) Close(ctx *sql.Context) error { type prollyConflictOurTableUpdater struct { baseSch, ourSch, theirSch schema.Schema srcUpdater sql.RowUpdater - versionMappings *VersionMappings + versionMappings *versionMappings pkOrdinals []int schemaOK bool } @@ -683,12 +683,12 @@ func (cd *prollyConflictDeleter) Close(ctx *sql.Context) error { return cd.ct.rs.SetRoot(ctx, updatedRoot) } -type VersionMappings struct { +type versionMappings struct { ourMapping, theirMapping, baseMapping val.OrdinalMapping } // returns the schema of the rows returned by the conflicts table and a mappings between each version and the source table. -func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *VersionMappings, error) { +func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, *versionMappings, error) { keyless := schema.IsKeyless(ours) n := 4 + ours.GetAllCols().Size() + theirs.GetAllCols().Size() + base.GetAllCols().Size() if keyless { @@ -773,7 +773,7 @@ func CalculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * } return sch, - &VersionMappings{ + &versionMappings{ ourMapping: ourColMapping, theirMapping: theirColMapping, baseMapping: baseColMapping}, From 7a5378e1e3dc3a693982a296a4355f3ed9561d68 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Fri, 30 May 2025 14:39:05 -0700 Subject: [PATCH 05/12] More fixes --- .../dolt_preview_merge_conflicts.go | 204 +++++++++++------- .../sqle/enginetest/dolt_queries_merge.go | 41 +++- 2 files changed, 166 insertions(+), 79 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 6d0b39d28ad..4e21669a0d1 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -41,7 +41,6 @@ var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil) var _ sql.ExecSourceRel = (*PreviewMergeConflictsTableFunction)(nil) var _ sql.AuthorizationCheckerNode = (*PreviewMergeConflictsTableFunction)(nil) - type PreviewMergeConflictsTableFunction struct { ctx *sql.Context leftBranchExpr sql.Expression @@ -120,8 +119,14 @@ func (pm *PreviewMergeConflictsTableFunction) Schema() sql.Schema { return nil } + // Lazy schema generation - generate schema on first access if pm.sqlSch.Schema == nil { - panic("schema hasn't been generated yet") + err := pm.generateSchema(pm.ctx) + if err != nil { + // Schema generation failed, but we can't return an error from Schema() + // This will surface the error when RowIter() is called + return nil + } } return pm.sqlSch.Schema @@ -200,22 +205,18 @@ func (pm *PreviewMergeConflictsTableFunction) WithExpressions(exprs ...sql.Expre return nil, sql.ErrInvalidArgumentDetails.New(newPmcs.Name(), newPmcs.tableNameExpr.String()) } - leftBranchVal, rightBranchVal, tableName, err := newPmcs.evaluateArguments() - if err != nil { - return nil, err - } - - err = newPmcs.generateSchema(pm.ctx, leftBranchVal, rightBranchVal, tableName) - if err != nil { - return nil, err - } - return &newPmcs, nil } -func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, leftBranchVal, rightBranchVal interface{}, tableName string) error { +// generateSchema generates the schema if it hasn't been generated yet +// This method is called lazily when the schema is first needed +func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context) error { + if pm.sqlSch.Schema != nil { + return nil // already generated + } + if !pm.Resolved() { - return nil + return fmt.Errorf("table function not resolved") } sqledb, ok := pm.database.(dsess.SqlDatabase) @@ -223,6 +224,11 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l return fmt.Errorf("unexpected database type: %T", pm.database) } + leftBranchVal, rightBranchVal, tableName, err := pm.evaluateArguments() + if err != nil { + return err + } + leftBranch, err := interfaceToString(leftBranchVal) if err != nil { return err @@ -264,61 +270,109 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context, l } func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, leftRoot, rightRoot, baseRoot doltdb.RootValue) (base, sch, mergeSch schema.Schema, err error) { + // Get table references from all three roots ourTbl, ourOk, err := leftRoot.GetTable(ctx, tblName) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, fmt.Errorf("failed to get table from left root: %w", err) } - + baseTbl, baseOk, err := baseRoot.GetTable(ctx, tblName) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, fmt.Errorf("failed to get table from base root: %w", err) } - theirTbl, theirOK, err := rightRoot.GetTable(ctx, tblName) + + theirTbl, theirOk, err := rightRoot.GetTable(ctx, tblName) if err != nil { - return nil, nil, nil, err - } - - if !ourOk { - return nil, nil, nil, fmt.Errorf("could not find tbl %s in left root value", tblName) + return nil, nil, nil, fmt.Errorf("failed to get table from right root: %w", err) } - if !theirOK { - return nil, nil, nil, fmt.Errorf("could not find tbl %s in right root value", tblName) + + // Check if table exists in at least one root + if !ourOk && !theirOk && !baseOk { + return nil, nil, nil, sql.ErrTableNotFound.New(tblName.String()) } - ourSch, err := ourTbl.GetSchema(ctx) + // Extract schemas from existing tables + schemas, err := extractSchemas(ctx, ourTbl, ourOk, theirTbl, theirOk, baseTbl, baseOk) if err != nil { return nil, nil, nil, err } - theirSch, err := theirTbl.GetSchema(ctx) - if err != nil { - return nil, nil, nil, err + // Apply fallback logic for missing schemas + return applySchemaFallbacks(schemas.our, schemas.their, schemas.base, ourOk, theirOk, baseOk) +} + +// extractedSchemas holds the schemas extracted from tables +type extractedSchemas struct { + our, their, base schema.Schema +} + +// extractSchemas retrieves schemas from the provided tables +func extractSchemas(ctx *sql.Context, ourTbl *doltdb.Table, ourOk bool, theirTbl *doltdb.Table, theirOk bool, baseTbl *doltdb.Table, baseOk bool) (*extractedSchemas, error) { + schemas := &extractedSchemas{} + var err error + + if ourOk { + schemas.our, err = ourTbl.GetSchema(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get schema from our table: %w", err) + } } - // If the table does not exist in the ancestor, pretend it existed and that - // it was completely empty. - if !baseOk { - if schema.SchemasAreEqual(ourSch, theirSch) { - return ourSch, ourSch, theirSch, nil + if theirOk { + schemas.their, err = theirTbl.GetSchema(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get schema from their table: %w", err) + } + } + + if baseOk { + schemas.base, err = baseTbl.GetSchema(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get schema from base table: %w", err) + } + } + + return schemas, nil +} + +// applySchemaFallbacks applies fallback logic when schemas are missing +func applySchemaFallbacks(ourSch, theirSch, baseSch schema.Schema, ourOk, theirOk, baseOk bool) (schema.Schema, schema.Schema, schema.Schema, error) { + // Apply fallback for missing "their" schema + if !theirOk { + if ourOk { + theirSch = ourSch } else { - return nil, nil, nil, fmt.Errorf("expected our schema to equal their schema since the table did not exist in the ancestor") + theirSch = baseSch } } - baseSch, err := baseTbl.GetSchema(ctx) - if err != nil { - return nil, nil, nil, err + // Apply fallback for missing "our" schema + if !ourOk { + if theirOk { + ourSch = theirSch + } else { + ourSch = baseSch + } } - return baseSch, ourSch, theirSch, nil + // Handle case where table doesn't exist in ancestor + if !baseOk { + if schema.SchemasAreEqual(ourSch, theirSch) { + return ourSch, ourSch, theirSch, nil + } + return nil, nil, nil, fmt.Errorf("expected our schema to equal their schema since the table did not exist in the ancestor") + } + + return ourSch, theirSch, baseSch, nil } // RowIter implements the sql.Node interface func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) { - if pm.sqlSch.Schema == nil { - panic("schema hasn't been generated yet") + // Ensure schema is generated before creating row iterator + err := pm.generateSchema(ctx) + if err != nil { + return nil, err } - merger, err := merge.NewMerger(pm.rootInfo.leftRoot, pm.rootInfo.rightRoot, pm.rootInfo.baseRoot, pm.rootInfo.rightCm, pm.rootInfo.ancCm, pm.rootInfo.leftRoot.VRW(), pm.rootInfo.leftRoot.NodeStore()) if err != nil { @@ -504,47 +558,49 @@ func (itr *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sq return nil, io.EOF } - r := make(sql.Row, itr.n) - c, exists, err := itr.nextConflictVals(ctx) - if err != nil { - return nil, err - } - if !exists { - // Move on to next conflict - return itr.Next(ctx) - } + for { + r := make(sql.Row, itr.n) + c, exists, err := itr.nextConflictVals(ctx) + if err != nil { + return nil, err + } + if !exists { + // Continue to next iteration if conflict does not exist + continue + } - r[0] = c.h.String() + r[0] = c.h.String() + + if !itr.keyless { + for i := 0; i < itr.kd.Count(); i++ { + f, err := tree.GetField(ctx, itr.kd, i, c.k, itr.baseRows.NodeStore()) + if err != nil { + return nil, err + } + if c.bV != nil { + r[itr.b+i] = f + } + if c.oV != nil { + r[itr.o+i] = f + } + if c.tV != nil { + r[itr.t+i] = f + } + } - if !itr.keyless { - for i := 0; i < itr.kd.Count(); i++ { - f, err := tree.GetField(ctx, itr.kd, i, c.k, itr.baseRows.NodeStore()) + err = itr.putConflictRowVals(ctx, c, r) if err != nil { return nil, err } - if c.bV != nil { - r[itr.b+i] = f - } - if c.oV != nil { - r[itr.o+i] = f - } - if c.tV != nil { - r[itr.t+i] = f + } else { + err = itr.putKeylessConflictRowVals(ctx, c, r) + if err != nil { + return nil, err } } - err = itr.putConflictRowVals(ctx, c, r) - if err != nil { - return nil, err - } - } else { - err = itr.putKeylessConflictRowVals(ctx, c, r) - if err != nil { - return nil, err - } + return r, nil } - - return r, nil } type conf struct { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index ddbe0aad001..3683fdf8cd6 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -1089,7 +1089,6 @@ var MergeScripts = []queries.ScriptTest{ Expected: []sql.Row{}, }, { - Skip: true, // TODO: could not find tbl test in right root value Query: "SELECT * FROM dolt_preview_merge_conflicts('HEAD', 'HEAD~1', 'test')", Expected: []sql.Row{}, }, @@ -1326,8 +1325,7 @@ var MergeScripts = []queries.ScriptTest{ Expected: []sql.Row{}, }, { - Skip: true, // TODO: could not find tbl test in left root value - Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'b1', 'test')", + Query: "SELECT * FROM dolt_preview_merge_conflicts('main', 'b1', 't1')", Expected: []sql.Row{}, }, { @@ -5268,8 +5266,8 @@ var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, }, { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 'nope');", - ExpectedErrStr: "could not find tbl nope in left root value", + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 'nope');", + ExpectedErr: sql.ErrTableNotFound, }, }, }, @@ -5433,6 +5431,39 @@ var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ }, }, }, + { + Name: "additional conflicts testing with multiple columns", + SetUpScript: []string{ + "create table test_table (pk int primary key, col1 varchar(20), col2 int);", + "insert into test_table values (1, 'original', 100), (2, 'second', 200);", + "call dolt_add('.')", + "call dolt_commit('-am', 'initial commit');", + + "call dolt_branch('branch1')", + "call dolt_checkout('-b', 'branch2')", + + "update test_table set col1 = 'branch2_val', col2 = 300 where pk = 1;", + "call dolt_add('.')", + "call dolt_commit('-am', 'modify on branch2');", + + "call dolt_checkout('branch1')", + "update test_table set col1 = 'branch1_val', col2 = 400 where pk = 1;", + "call dolt_add('.')", + "call dolt_commit('-am', 'modify on branch1');", + + "call dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT base_pk, our_col1, their_col1 from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", + Expected: []sql.Row{{1, "branch1_val", "branch2_val"}}, + }, + }, + }, } // convertMergeScriptTest converts a MergeScriptTest into a standard ScriptTest. If flipSides is true, then the From 333cd9d8f40c119e378101da28cfa04e1d818c38 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Tue, 3 Jun 2025 13:57:01 -0700 Subject: [PATCH 06/12] Clean up --- .../dolt_preview_merge_conflicts.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 4e21669a0d1..4a52bcc0f04 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -118,7 +118,6 @@ func (pm *PreviewMergeConflictsTableFunction) Schema() sql.Schema { if !pm.Resolved() { return nil } - // Lazy schema generation - generate schema on first access if pm.sqlSch.Schema == nil { err := pm.generateSchema(pm.ctx) @@ -209,10 +208,9 @@ func (pm *PreviewMergeConflictsTableFunction) WithExpressions(exprs ...sql.Expre } // generateSchema generates the schema if it hasn't been generated yet -// This method is called lazily when the schema is first needed func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context) error { if pm.sqlSch.Schema != nil { - return nil // already generated + return nil } if !pm.Resolved() { @@ -270,7 +268,6 @@ func (pm *PreviewMergeConflictsTableFunction) generateSchema(ctx *sql.Context) e } func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, leftRoot, rightRoot, baseRoot doltdb.RootValue) (base, sch, mergeSch schema.Schema, err error) { - // Get table references from all three roots ourTbl, ourOk, err := leftRoot.GetTable(ctx, tblName) if err != nil { return nil, nil, nil, fmt.Errorf("failed to get table from left root: %w", err) @@ -286,27 +283,22 @@ func getConflictSchemasFromRoots(ctx *sql.Context, tblName doltdb.TableName, lef return nil, nil, nil, fmt.Errorf("failed to get table from right root: %w", err) } - // Check if table exists in at least one root if !ourOk && !theirOk && !baseOk { return nil, nil, nil, sql.ErrTableNotFound.New(tblName.String()) } - // Extract schemas from existing tables schemas, err := extractSchemas(ctx, ourTbl, ourOk, theirTbl, theirOk, baseTbl, baseOk) if err != nil { return nil, nil, nil, err } - // Apply fallback logic for missing schemas return applySchemaFallbacks(schemas.our, schemas.their, schemas.base, ourOk, theirOk, baseOk) } -// extractedSchemas holds the schemas extracted from tables type extractedSchemas struct { our, their, base schema.Schema } -// extractSchemas retrieves schemas from the provided tables func extractSchemas(ctx *sql.Context, ourTbl *doltdb.Table, ourOk bool, theirTbl *doltdb.Table, theirOk bool, baseTbl *doltdb.Table, baseOk bool) (*extractedSchemas, error) { schemas := &extractedSchemas{} var err error @@ -335,9 +327,7 @@ func extractSchemas(ctx *sql.Context, ourTbl *doltdb.Table, ourOk bool, theirTbl return schemas, nil } -// applySchemaFallbacks applies fallback logic when schemas are missing func applySchemaFallbacks(ourSch, theirSch, baseSch schema.Schema, ourOk, theirOk, baseOk bool) (schema.Schema, schema.Schema, schema.Schema, error) { - // Apply fallback for missing "their" schema if !theirOk { if ourOk { theirSch = ourSch @@ -346,7 +336,6 @@ func applySchemaFallbacks(ourSch, theirSch, baseSch schema.Schema, ourOk, theirO } } - // Apply fallback for missing "our" schema if !ourOk { if theirOk { ourSch = theirSch @@ -355,7 +344,6 @@ func applySchemaFallbacks(ourSch, theirSch, baseSch schema.Schema, ourOk, theirO } } - // Handle case where table doesn't exist in ancestor if !baseOk { if schema.SchemasAreEqual(ourSch, theirSch) { return ourSch, ourSch, theirSch, nil @@ -368,7 +356,6 @@ func applySchemaFallbacks(ourSch, theirSch, baseSch schema.Schema, ourOk, theirO // RowIter implements the sql.Node interface func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) { - // Ensure schema is generated before creating row iterator err := pm.generateSchema(ctx) if err != nil { return nil, err @@ -627,9 +614,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql. if len(ca.Key) == 0 { return conf{}, false, fmt.Errorf("empty key found in conflict") } - // TODO: This is mutating the key, which is creating a schema with a different capacity than expected - // b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() - // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. + // To ensure that the conflict id is unique, we hash both theirRootish and the key of the table. buf := make([]byte, len(ca.Key)+len(c.h)) copy(buf, ca.Key) copy(buf[len(ca.Key):], c.h[:]) From eec403b4f162a1cbace96a7cfa9d0cf568e04c7d Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 5 Jun 2025 15:14:47 -0700 Subject: [PATCH 07/12] Merge tests, better naming, share some conflict logic --- .../dolt_preview_merge_conflicts.go | 285 +++------ .../sqle/dtables/conflicts_tables_prolly.go | 302 ++++++---- .../sqle/enginetest/dolt_engine_test.go | 10 - .../sqle/enginetest/dolt_engine_tests.go | 22 - .../sqle/enginetest/dolt_queries_merge.go | 558 +++++++++--------- 5 files changed, 525 insertions(+), 652 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 4a52bcc0f04..405c61695ef 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -15,7 +15,6 @@ package dtablefunctions import ( - "encoding/base64" "fmt" "io" @@ -34,7 +33,6 @@ import ( "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" "github.com/dolthub/go-mysql-server/sql/types" - "github.com/zeebo/xxh3" ) var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil) @@ -428,22 +426,8 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. return nil, err } - kd := pm.baseSch.GetKeyDescriptor(pm.rootInfo.baseRoot.NodeStore()) - baseVD := pm.baseSch.GetValueDescriptor(pm.rootInfo.baseRoot.NodeStore()) - oursVD := pm.ourSch.GetValueDescriptor(pm.rootInfo.leftRoot.NodeStore()) - theirsVD := pm.theirSch.GetValueDescriptor(pm.rootInfo.rightRoot.NodeStore()) - - b := 1 - var o, t, n int - if !keyless { - o = b + kd.Count() + baseVD.Count() - t = o + kd.Count() + oursVD.Count() + 1 - n = t + kd.Count() + theirsVD.Count() + 2 - } else { - o = b + baseVD.Count() - 1 - t = o + oursVD.Count() - n = t + theirsVD.Count() + 4 - } + vds := dtables.GetConflictValueDescriptors(pm.baseSch, pm.ourSch, pm.theirSch, pm.rootInfo.baseRoot.NodeStore()) + offsets := dtables.GetConflictOffsets(keyless, vds) valueMerger := tm.GetNewValueMerger(mergeSch, leftRows) @@ -470,14 +454,8 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. ourRows: leftRows, keyless: keyless, ourSch: pm.ourSch, - kd: kd, - baseVD: baseVD, - oursVD: oursVD, - theirsVD: theirsVD, - b: b, - o: o, - t: t, - n: n, + offsets: offsets, + vds: vds, baseRootish: baseHash, theirRootish: rightHash, }, nil @@ -529,11 +507,8 @@ type previewMergeConflictsTableFunctionRowIter struct { keyless bool ourSch schema.Schema - kd val.TupleDesc - baseVD, oursVD, theirsVD val.TupleDesc - // offsets for each version - b, o, t int - n int + vds dtables.ConflictValueDescriptors + offsets dtables.ConflictOffsets baseHash, theirHash hash.Hash baseRows, theirRows prolly.Map @@ -545,110 +520,75 @@ func (itr *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sq return nil, io.EOF } - for { - r := make(sql.Row, itr.n) - c, exists, err := itr.nextConflictVals(ctx) + row := make(sql.Row, itr.offsets.N) + confVal, err := itr.nextConflictVals(ctx) + if err != nil { + return nil, err + } + + row[0] = confVal.Hash.String() + + if !itr.keyless { + err = itr.putConflictRowVals(ctx, confVal, row) if err != nil { return nil, err } - if !exists { - // Continue to next iteration if conflict does not exist - continue - } - - r[0] = c.h.String() - - if !itr.keyless { - for i := 0; i < itr.kd.Count(); i++ { - f, err := tree.GetField(ctx, itr.kd, i, c.k, itr.baseRows.NodeStore()) - if err != nil { - return nil, err - } - if c.bV != nil { - r[itr.b+i] = f - } - if c.oV != nil { - r[itr.o+i] = f - } - if c.tV != nil { - r[itr.t+i] = f - } - } - - err = itr.putConflictRowVals(ctx, c, r) - if err != nil { - return nil, err - } - } else { - err = itr.putKeylessConflictRowVals(ctx, c, r) - if err != nil { - return nil, err - } + } else { + err = itr.putKeylessConflictRowVals(ctx, confVal, row) + if err != nil { + return nil, err } - - return r, nil } -} -type conf struct { - k, bV, oV, tV val.Tuple - h hash.Hash - id string + return row, nil } -func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql.Context) (c conf, exists bool, err error) { - ca, err := itr.itr.Next(ctx) - if err != nil { - return conf{}, false, err - } - isConflict := ca.Op == tree.DiffOpDivergentModifyConflict || ca.Op == tree.DiffOpDivergentDeleteConflict - isKeylessConflict := itr.keyless && (ca.Op == tree.DiffOpConvergentAdd || ca.Op == tree.DiffOpConvergentModify || ca.Op == tree.DiffOpConvergentDelete) - if !isConflict && !isKeylessConflict { - // If this is not a conflict, then we don't need to return anything. - return conf{}, false, nil - } +func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql.Context) (confVal dtables.ConflictVal, err error) { + for { + ca, err := itr.itr.Next(ctx) + if err != nil { + return dtables.ConflictVal{}, err + } + isConflict := ca.Op == tree.DiffOpDivergentModifyConflict || ca.Op == tree.DiffOpDivergentDeleteConflict + isKeylessConflict := itr.keyless && (ca.Op == tree.DiffOpConvergentAdd || ca.Op == tree.DiffOpConvergentModify || ca.Op == tree.DiffOpConvergentDelete) + if !isConflict && !isKeylessConflict { + // If this is not a conflict, continue to next iteration + continue + } - c.k = ca.Key - c.h = itr.theirRootish + confVal.Key = ca.Key + confVal.Hash = itr.theirRootish + confVal.Id = dtables.GetConflictId(ca.Key, confVal.Hash) - if len(ca.Key) == 0 { - return conf{}, false, fmt.Errorf("empty key found in conflict") - } - // To ensure that the conflict id is unique, we hash both theirRootish and the key of the table. - buf := make([]byte, len(ca.Key)+len(c.h)) - copy(buf, ca.Key) - copy(buf[len(ca.Key):], c.h[:]) - b := xxh3.Hash128(buf).Bytes() - c.id = base64.RawStdEncoding.EncodeToString(b[:]) + err = itr.loadTableMaps(ctx, itr.baseRootish, itr.theirRootish) + if err != nil { + return dtables.ConflictVal{}, err + } - err = itr.loadTableMaps(ctx, itr.baseRootish, itr.theirRootish) - if err != nil { - return conf{}, false, err - } + err = itr.baseRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + confVal.Base = v + return nil + }) + if err != nil { + return dtables.ConflictVal{}, err + } + err = itr.ourRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + confVal.Ours = v + return nil + }) + if err != nil { + return dtables.ConflictVal{}, err + } + err = itr.theirRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { + confVal.Theirs = v + return nil + }) + if err != nil { + return dtables.ConflictVal{}, err + } - err = itr.baseRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.bV = v - return nil - }) - if err != nil { - return conf{}, false, err - } - err = itr.ourRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.oV = v - return nil - }) - if err != nil { - return conf{}, false, err - } - err = itr.theirRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.tV = v - return nil - }) - if err != nil { - return conf{}, false, err + return confVal, nil } - - return c, true, nil } // loadTableMaps loads the maps specified in the metadata if they are different from @@ -711,105 +651,14 @@ func (itr *previewMergeConflictsTableFunctionRowIter) loadTableMaps(ctx *sql.Con return nil } -func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sql.Context, c conf, r sql.Row) error { - if c.bV != nil { - for i := 0; i < itr.baseVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.baseVD, i, c.bV, itr.baseRows.NodeStore()) - if err != nil { - return err - } - r[itr.b+itr.kd.Count()+i] = f - } - } - - if c.oV != nil { - for i := 0; i < itr.oursVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.oursVD, i, c.oV, itr.baseRows.NodeStore()) - if err != nil { - return err - } - r[itr.o+itr.kd.Count()+i] = f - } - } - r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = dtables.GetConflictDiffType(c.bV, c.oV) - - if c.tV != nil { - for i := 0; i < itr.theirsVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.theirsVD, i, c.tV, itr.baseRows.NodeStore()) - if err != nil { - return err - } - r[itr.t+itr.kd.Count()+i] = f - } - } - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = dtables.GetConflictDiffType(c.bV, c.tV) - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()+1] = c.id - - return nil +func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sql.Context, confVal dtables.ConflictVal, row sql.Row) error { + ns := itr.baseRows.NodeStore() + return dtables.PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) } -func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals(ctx *sql.Context, c conf, r sql.Row) (err error) { +func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals(ctx *sql.Context, confVal dtables.ConflictVal, row sql.Row) (err error) { ns := itr.baseRows.NodeStore() - - if c.bV != nil { - // Cardinality - r[itr.n-3], err = tree.GetField(ctx, itr.baseVD, 0, c.bV, ns) - if err != nil { - return err - } - - for i := 0; i < itr.baseVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.baseVD, i+1, c.bV, ns) - if err != nil { - return err - } - r[itr.b+i] = f - } - } else { - r[itr.n-3] = uint64(0) - } - - if c.oV != nil { - r[itr.n-2], err = tree.GetField(ctx, itr.oursVD, 0, c.oV, ns) - if err != nil { - return err - } - - for i := 0; i < itr.oursVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.oursVD, i+1, c.oV, ns) - if err != nil { - return err - } - r[itr.o+i] = f - } - } else { - r[itr.n-2] = uint64(0) - } - - r[itr.o+itr.oursVD.Count()-1] = dtables.GetConflictDiffType(c.bV, c.oV) - - if c.tV != nil { - r[itr.n-1], err = tree.GetField(ctx, itr.theirsVD, 0, c.tV, ns) - if err != nil { - return err - } - - for i := 0; i < itr.theirsVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.theirsVD, i+1, c.tV, ns) - if err != nil { - return err - } - r[itr.t+i] = f - } - } else { - r[itr.n-1] = uint64(0) - } - - o := itr.t + itr.theirsVD.Count() - 1 - r[o] = dtables.GetConflictDiffType(c.bV, c.tV) - r[itr.n-4] = c.id - - return nil + return dtables.PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) } func (d *previewMergeConflictsTableFunctionRowIter) Close(context *sql.Context) error { diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 6c1e5d4c81e..c31a135d808 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -135,17 +135,64 @@ type prollyConflictRowIter struct { keyless bool ourSch schema.Schema - kd val.TupleDesc - baseVD, oursVD, theirsVD val.TupleDesc - // offsets for each version - b, o, t int - n int + vds ConflictValueDescriptors + offsets ConflictOffsets baseHash, theirHash hash.Hash baseRows prolly.Map theirRows prolly.Map } +// ConflictOffsets holds the offsets of the columns in a conflict row. The +// offsets are used to put the values in the correct place in the row. +type ConflictOffsets struct { + Base, Ours, Theirs int + N int +} + +// ConflictValueDescriptors holds the value descriptors for the base, ours, and theirs rows. +type ConflictValueDescriptors struct { + Base, Ours, Theirs val.TupleDesc + Key val.TupleDesc +} + +// GetConflictOffsets returns the offsets of the columns in a conflict row. +func GetConflictOffsets(keyless bool, vds ConflictValueDescriptors) ConflictOffsets { + baseOffset := 1 + var ourOffset, theirOffset, n int + if !keyless { + ourOffset = baseOffset + vds.Key.Count() + vds.Base.Count() + theirOffset = ourOffset + vds.Key.Count() + vds.Ours.Count() + 1 + n = theirOffset + vds.Key.Count() + vds.Theirs.Count() + 2 + } else { + ourOffset = baseOffset + vds.Base.Count() - 1 + theirOffset = ourOffset + vds.Ours.Count() + n = theirOffset + vds.Theirs.Count() + 4 + } + + return ConflictOffsets{ + Base: baseOffset, + Ours: ourOffset, + Theirs: theirOffset, + N: n, + } +} + +// GetConflictValueDescriptors returns the value descriptors for the base, ours, and theirs +// tables. +func GetConflictValueDescriptors(baseSch, ourSch, theirSch schema.Schema, ns tree.NodeStore) ConflictValueDescriptors { + keyVD := baseSch.GetKeyDescriptor(ns) + baseVD := baseSch.GetValueDescriptor(ns) + oursVD := ourSch.GetValueDescriptor(ns) + theirsVD := theirSch.GetValueDescriptor(ns) + return ConflictValueDescriptors{ + Base: baseVD, + Ours: oursVD, + Theirs: theirsVD, + Key: keyVD, + } +} + var _ sql.RowIter = (*prollyConflictRowIter)(nil) // base_cols, our_cols, our_diff_type, their_cols, their_diff_type @@ -165,121 +212,116 @@ func newProllyConflictRowIter(ctx *sql.Context, ct ProllyConflictsTable) (*proll } keyless := schema.IsKeyless(ct.ourSch) - - kd := ct.baseSch.GetKeyDescriptor(ct.root.NodeStore()) - baseVD := ct.baseSch.GetValueDescriptor(ct.root.NodeStore()) - oursVD := ct.ourSch.GetValueDescriptor(ct.root.NodeStore()) - theirsVD := ct.theirSch.GetValueDescriptor(ct.root.NodeStore()) - - b := 1 - var o, t, n int - if !keyless { - o = b + kd.Count() + baseVD.Count() - t = o + kd.Count() + oursVD.Count() + 1 - n = t + kd.Count() + theirsVD.Count() + 2 - } else { - o = b + baseVD.Count() - 1 - t = o + oursVD.Count() - n = t + theirsVD.Count() + 4 - } + vds := GetConflictValueDescriptors(ct.baseSch, ct.ourSch, ct.theirSch, ct.root.NodeStore()) + offsets := GetConflictOffsets(keyless, vds) return &prollyConflictRowIter{ - itr: itr, - tblName: ct.tblName, - vrw: ct.tbl.ValueReadWriter(), - ns: ct.tbl.NodeStore(), - ourRows: ourRows, - keyless: keyless, - ourSch: ct.ourSch, - kd: kd, - baseVD: baseVD, - oursVD: oursVD, - theirsVD: theirsVD, - b: b, - o: o, - t: t, - n: n, + itr: itr, + tblName: ct.tblName, + vrw: ct.tbl.ValueReadWriter(), + ns: ct.tbl.NodeStore(), + ourRows: ourRows, + keyless: keyless, + ourSch: ct.ourSch, + vds: vds, + offsets: offsets, }, nil } func (itr *prollyConflictRowIter) Next(ctx *sql.Context) (sql.Row, error) { - c, err := itr.nextConflictVals(ctx) + confVal, err := itr.nextConflictVals(ctx) if err != nil { return nil, err } - r := make(sql.Row, itr.n) - r[0] = c.h.String() + row := make(sql.Row, itr.offsets.N) + row[0] = confVal.Hash.String() // from_root_ish if !itr.keyless { - for i := 0; i < itr.kd.Count(); i++ { - f, err := tree.GetField(ctx, itr.kd, i, c.k, itr.baseRows.NodeStore()) - if err != nil { - return nil, err - } - if c.bV != nil { - r[itr.b+i] = f - } - if c.oV != nil { - r[itr.o+i] = f - } - if c.tV != nil { - r[itr.t+i] = f - } - } - - err = itr.putConflictRowVals(ctx, c, r) + err = itr.putConflictRowVals(ctx, confVal, row) if err != nil { return nil, err } } else { - err = itr.putKeylessConflictRowVals(ctx, c, r) + err = itr.putKeylessConflictRowVals(ctx, confVal, row) if err != nil { return nil, err } } - return r, nil + return row, nil } -func (itr *prollyConflictRowIter) putConflictRowVals(ctx *sql.Context, c conf, r sql.Row) error { - if c.bV != nil { - for i := 0; i < itr.baseVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.baseVD, i, c.bV, itr.baseRows.NodeStore()) +// PutConflictRowVals puts the values of the conflict row into the given row. +func PutConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, vds ConflictValueDescriptors, ns tree.NodeStore) error { + // Sets key columns for the conflict row. + for i := 0; i < vds.Key.Count(); i++ { + f, err := tree.GetField(ctx, vds.Key, i, confVal.Key, ns) + if err != nil { + return err + } + if confVal.Base != nil { + row[offsets.Base+i] = f + } + if confVal.Ours != nil { + row[offsets.Ours+i] = f + } + if confVal.Theirs != nil { + row[offsets.Theirs+i] = f + } + } + + if confVal.Base != nil { + for i := 0; i < vds.Base.Count(); i++ { + f, err := tree.GetField(ctx, vds.Base, i, confVal.Base, ns) if err != nil { return err } - r[itr.b+itr.kd.Count()+i] = f + baseColOffset := offsets.Base + vds.Key.Count() + i + row[baseColOffset] = f // base_[col] } } - if c.oV != nil { - for i := 0; i < itr.oursVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.oursVD, i, c.oV, itr.baseRows.NodeStore()) + if confVal.Ours != nil { + for i := 0; i < vds.Ours.Count(); i++ { + f, err := tree.GetField(ctx, vds.Ours, i, confVal.Ours, ns) if err != nil { return err } - r[itr.o+itr.kd.Count()+i] = f + ourColOffset := offsets.Ours + vds.Key.Count() + i + row[ourColOffset] = f // our_[col] } } - r[itr.o+itr.kd.Count()+itr.oursVD.Count()] = GetConflictDiffType(c.bV, c.oV) - if c.tV != nil { - for i := 0; i < itr.theirsVD.Count(); i++ { - f, err := tree.GetField(ctx, itr.theirsVD, i, c.tV, itr.baseRows.NodeStore()) + ourDiffTypeOffset := offsets.Ours + vds.Key.Count() + vds.Ours.Count() + row[ourDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Ours) // our_diff_type + + if confVal.Theirs != nil { + for i := 0; i < vds.Theirs.Count(); i++ { + f, err := tree.GetField(ctx, vds.Theirs, i, confVal.Theirs, ns) if err != nil { return err } - r[itr.t+itr.kd.Count()+i] = f + theirColOffset := offsets.Theirs + vds.Key.Count() + i + row[theirColOffset] = f // their_[col] } } - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()] = GetConflictDiffType(c.bV, c.tV) - r[itr.t+itr.kd.Count()+itr.theirsVD.Count()+1] = c.id + + theirDiffTypeOffset := offsets.Theirs + vds.Key.Count() + vds.Theirs.Count() + row[theirDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Theirs) // their_diff_type + + conflictIdOffset := theirDiffTypeOffset + 1 + row[conflictIdOffset] = confVal.Id // dolt_conflict_id return nil } -func GetConflictDiffType(base val.Tuple, other val.Tuple) string { +func (itr *prollyConflictRowIter) putConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row) error { + ns := itr.baseRows.NodeStore() + return PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) +} + +func getConflictDiffType(base val.Tuple, other val.Tuple) string { if base == nil { return merge.ConflictDiffTypeAdded } else if other == nil { @@ -290,116 +332,134 @@ func GetConflictDiffType(base val.Tuple, other val.Tuple) string { return merge.ConflictDiffTypeModified } -func (itr *prollyConflictRowIter) putKeylessConflictRowVals(ctx *sql.Context, c conf, r sql.Row) (err error) { - ns := itr.baseRows.NodeStore() - - if c.bV != nil { - // Cardinality - r[itr.n-3], err = tree.GetField(ctx, itr.baseVD, 0, c.bV, ns) +// PutKeylessConflictRowVals puts the values of the keyless conflict row into the given row. +func PutKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, vds ConflictValueDescriptors, ns tree.NodeStore) (err error) { + if confVal.Base != nil { + f, err := tree.GetField(ctx, vds.Base, 0, confVal.Base, ns) if err != nil { return err } + row[offsets.N-3] = f // base_cardinality - for i := 0; i < itr.baseVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.baseVD, i+1, c.bV, ns) + for i := 0; i < vds.Base.Count()-1; i++ { + f, err := tree.GetField(ctx, vds.Base, i+1, confVal.Base, ns) if err != nil { return err } - r[itr.b+i] = f + baseColOffset := offsets.Base + i + row[baseColOffset] = f // base_[col] } } else { - r[itr.n-3] = uint64(0) + row[offsets.N-3] = uint64(0) // base_cardinality } - if c.oV != nil { - r[itr.n-2], err = tree.GetField(ctx, itr.oursVD, 0, c.oV, ns) + if confVal.Ours != nil { + f, err := tree.GetField(ctx, vds.Ours, 0, confVal.Ours, ns) if err != nil { return err } + row[offsets.N-2] = f // our_cardinality - for i := 0; i < itr.oursVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.oursVD, i+1, c.oV, ns) + for i := 0; i < vds.Ours.Count()-1; i++ { + f, err := tree.GetField(ctx, vds.Ours, i+1, confVal.Ours, ns) if err != nil { return err } - r[itr.o+i] = f + row[offsets.Ours+i] = f // our_[col] } } else { - r[itr.n-2] = uint64(0) + row[offsets.N-2] = uint64(0) // our_cardinality } - r[itr.o+itr.oursVD.Count()-1] = GetConflictDiffType(c.bV, c.oV) + ourDiffTypeOffset := offsets.Ours + vds.Ours.Count() - 1 + row[ourDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Ours) // our_diff_type - if c.tV != nil { - r[itr.n-1], err = tree.GetField(ctx, itr.theirsVD, 0, c.tV, ns) + if confVal.Theirs != nil { + f, err := tree.GetField(ctx, vds.Theirs, 0, confVal.Theirs, ns) if err != nil { return err } + row[offsets.N-1] = f // their_cardinality - for i := 0; i < itr.theirsVD.Count()-1; i++ { - f, err := tree.GetField(ctx, itr.theirsVD, i+1, c.tV, ns) + for i := 0; i < vds.Theirs.Count()-1; i++ { + f, err := tree.GetField(ctx, vds.Theirs, i+1, confVal.Theirs, ns) if err != nil { return err } - r[itr.t+i] = f + row[offsets.Theirs+i] = f // their_[col] } } else { - r[itr.n-1] = uint64(0) + row[offsets.N-1] = uint64(0) // their_cardinality } - o := itr.t + itr.theirsVD.Count() - 1 - r[o] = GetConflictDiffType(c.bV, c.tV) - r[itr.n-4] = c.id + theirDiffTypeOffset := offsets.Theirs + vds.Theirs.Count() - 1 + row[theirDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Theirs) // their_diff_type + + row[offsets.N-4] = confVal.Id // dolt_conflict_id return nil } -type conf struct { - k, bV, oV, tV val.Tuple - h hash.Hash - id string +func (itr *prollyConflictRowIter) putKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row) (err error) { + ns := itr.baseRows.NodeStore() + return PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) +} + +type ConflictVal struct { + Key, Base, Ours, Theirs val.Tuple + Hash hash.Hash + Id string +} + +// GetConflictId gets the conflict ID, ensuring that it is unique by hashing both theirRootish and the key of the table. +func GetConflictId(key val.Tuple, confHash hash.Hash) string { + // TODO: We were using this but it makes the dolt_preview_merge_conflicts table function panic with "slice bounds out of range" in flatbuffer table.SchemaBytes + // b := xxh3.Hash128(append(key, confHash[:]...)).Bytes() + buf := make([]byte, len(key)+len(confHash)) + copy(buf, key) + copy(buf[len(key):], confHash[:]) + b := xxh3.Hash128(buf).Bytes() + return base64.RawStdEncoding.EncodeToString(b[:]) } -func (itr *prollyConflictRowIter) nextConflictVals(ctx *sql.Context) (c conf, err error) { +func (itr *prollyConflictRowIter) nextConflictVals(ctx *sql.Context) (confVal ConflictVal, err error) { ca, err := itr.itr.Next(ctx) if err != nil { - return conf{}, err + return ConflictVal{}, err } - c.k = ca.Key - c.h = ca.TheirRootIsh - // To ensure that the conflict id is unique, we hash both TheirRootIsh and the key of the table. - b := xxh3.Hash128(append(ca.Key, c.h[:]...)).Bytes() - c.id = base64.RawStdEncoding.EncodeToString(b[:]) + confVal.Key = ca.Key + confVal.Hash = ca.TheirRootIsh + confVal.Id = GetConflictId(ca.Key, confVal.Hash) err = itr.loadTableMaps(ctx, ca.Metadata.BaseRootIsh, ca.TheirRootIsh) if err != nil { - return conf{}, err + return ConflictVal{}, err } err = itr.baseRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.bV = v + confVal.Base = v return nil }) if err != nil { - return conf{}, err + return ConflictVal{}, err } err = itr.ourRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.oV = v + confVal.Ours = v return nil }) if err != nil { - return conf{}, err + return ConflictVal{}, err } err = itr.theirRows.Get(ctx, ca.Key, func(_, v val.Tuple) error { - c.tV = v + confVal.Theirs = v return nil }) if err != nil { - return conf{}, err + return ConflictVal{}, err } - return c, nil + return confVal, nil } // loadTableMaps loads the maps specified in the metadata if they are different from diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 3ca69686695..d8e69854f6c 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -1342,16 +1342,6 @@ func TestDoltMergeArtifacts(t *testing.T) { RunDoltMergeArtifacts(t, h) } -func TestDoltPreviewMergeConflictsSummary(t *testing.T) { - h := newDoltEnginetestHarness(t) - RunDoltPreviewMergeConflictsSummaryTests(t, h) -} - -func TestDoltPreviewMergeConflictsSummaryPrepared(t *testing.T) { - h := newDoltEnginetestHarness(t) - RunDoltPreviewMergeConflictsSummaryPreparedTests(t, h) -} - func TestDoltPreviewMergeConflicts(t *testing.T) { h := newDoltEnginetestHarness(t) RunDoltPreviewMergeConflictsTests(t, h) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go index a0a34cad57d..38865eed432 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go @@ -988,28 +988,6 @@ func RunDoltMergeArtifacts(t *testing.T, h DoltEnginetestHarness) { } } -func RunDoltPreviewMergeConflictsSummaryTests(t *testing.T, h DoltEnginetestHarness) { - for _, script := range PreviewMergeConflictsSummaryFunctionScripts { - // harness can't reset effectively. Use a new harness for each script - func() { - h := h.NewHarness(t) - defer h.Close() - enginetest.TestScript(t, h, script) - }() - } -} - -func RunDoltPreviewMergeConflictsSummaryPreparedTests(t *testing.T, h DoltEnginetestHarness) { - for _, script := range PreviewMergeConflictsSummaryFunctionScripts { - // harness can't reset effectively. Use a new harness for each script - func() { - h := h.NewHarness(t) - defer h.Close() - enginetest.TestScript(t, h, script) - }() - } -} - func RunDoltPreviewMergeConflictsTests(t *testing.T, h DoltEnginetestHarness) { for _, script := range PreviewMergeConflictsFunctionScripts { // harness can't reset effectively. Use a new harness for each script diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 93d3913c17f..74e2cbd0278 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4682,7 +4682,7 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, } -var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ +var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ { Name: "invalid arguments", SetUpScript: []string{ @@ -4695,6 +4695,7 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ "call dolt_branch('branch2')", }, Assertions: []queries.ScriptTestAssertion{ + // dolt_preview_merge_conflicts_summary { Query: "SELECT * from dolt_preview_merge_conflicts_summary();", ExpectedErr: sql.ErrInvalidArgumentNumber, @@ -4739,6 +4740,63 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary(hashof('main'), 'branch1');", ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, }, + // dolt_preview_merge_conflicts + { + Query: "SELECT * from dolt_preview_merge_conflicts();", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('t');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't', 'extra');", + ExpectedErr: sql.ErrInvalidArgumentNumber, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(null, null, null);", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 123, 't');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(123, 'branch1', 't');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 123);", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('fake-branch', 'main', 't');", + ExpectedErrStr: "branch not found: fake-branch", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'fake-branch', 't');", + ExpectedErrStr: "branch not found: fake-branch", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main...branch1', 'branch2', 't');", + ExpectedErrStr: "string is not a valid branch or hash", + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', concat('branch', '1'), 't');", + ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(hashof('main'), 'branch1', 't');", + ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 'nope');", + ExpectedErr: sql.ErrTableNotFound, + }, }, }, { @@ -4769,22 +4827,42 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary(@Commit1, @Commit2)", // not branches Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch2', 'main')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1, "one", "two", 1, "one!", "two", "modified", 1, "one?", "two", "modified"}}, + }, }, }, { @@ -4815,26 +4893,134 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary(@Commit1, @Commit2)", // not branches Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch2', 'main')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1}}, + }, + }, + }, + { + Name: "basic case with multiple tables, data conflicts", + SetUpScript: []string{ + "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", + "create table t2 (pk int primary key, c1 varchar(20));", + "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", + "insert into t2 values(100, 'hundred');", + "call dolt_add('.')", + "set @Commit1 = '';", + "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", + + "call dolt_branch('branch1')", + "call dolt_checkout('-b', 'branch2')", + "update t set c1='one!' where pk=1", + "update t2 set c1='hundred!' where pk=100", + "set @Commit2 = '';", + "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", + + "call dolt_checkout('branch1')", + "update t set c1='one?' where pk=1", + "update t2 set c1='hundred?' where pk=100", + "set @Commit3 = '';", + "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", + + "call dolt_checkout('main')", + "call dolt_merge('branch1')", + + "create table keyless (id int);", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't2')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", + Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", uint64(1), uint64(0)}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't2')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", + Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", uint64(1), uint64(0)}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't2')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts_summary(@Commit1, @Commit2)", // not branches + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch2', 'main')", + Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", uint64(1), uint64(0)}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't2')", + Expected: []sql.Row{{1}}, + }, }, }, { - Name: "basic case with multiple tables", + Name: "basic case with multiple tables, schema conflict", SetUpScript: []string{ "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", "create table t2 (pk int primary key, c1 varchar(20));", @@ -4867,22 +5053,58 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't2')", + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", nil, uint64(1)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't2')", + ExpectedErrStr: "schema conflicts found: 1", + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", nil, uint64(1)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't2')", + ExpectedErrStr: "schema conflicts found: 1", + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary(@Commit1, @Commit2)", // not branches Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch2', 'main')", Expected: []sql.Row{{"t", uint64(1), uint64(0)}, {"t2", nil, uint64(1)}}, }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", + Expected: []sql.Row{{1}}, + }, + { + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't2')", + ExpectedErrStr: "schema conflicts found: 1", + }, }, }, { @@ -4910,14 +5132,26 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", nil, uint64(1)}}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch2', 't')", + ExpectedErrStr: "schema conflicts found: 1", + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", nil, uint64(1)}}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + ExpectedErrStr: "schema conflicts found: 1", + }, }, }, { @@ -4947,14 +5181,26 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", nil, uint64(1)}}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch2', 't')", + ExpectedErrStr: "schema conflicts found: 1", + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", nil, uint64(1)}}, }, + { + Query: "SELECT * from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + ExpectedErrStr: "schema conflicts found: 1", + }, }, }, { @@ -5127,54 +5373,58 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", Expected: []sql.Row{}, }, + { + Query: "SELECT COUNT(*) from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Expected: []sql.Row{{0}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2')", Expected: []sql.Row{{"t", uint64(4), uint64(0)}}, // 4 rows have conflicts (1,2,3,4) }, + { + Query: "SELECT COUNT(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", + Expected: []sql.Row{{4}}, + }, { Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{{"t", uint64(4), uint64(0)}}, }, + { + Query: "SELECT COUNT(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", + Expected: []sql.Row{{4}}, + }, }, }, { - Name: "multiple tables with different conflict types", + Name: "additional conflicts testing with multiple columns", SetUpScript: []string{ - "create table data_conflicts (pk int primary key, value varchar(20));", - "create table schema_conflicts (pk int primary key, name varchar(20) default 'default');", - "create table no_conflicts (pk int primary key, info varchar(20));", - "insert into data_conflicts values (1, 'original'), (2, 'data');", - "insert into schema_conflicts values (1, 'schema');", - "insert into no_conflicts values (1, 'unchanged');", + "create table test_table (pk int primary key, col1 varchar(20), col2 int);", + "insert into test_table values (1, 'original', 100), (2, 'second', 200);", "call dolt_add('.')", - "call dolt_commit('-am', 'initial setup');", + "call dolt_commit('-am', 'initial commit');", "call dolt_branch('branch1')", "call dolt_checkout('-b', 'branch2')", - "update data_conflicts set value='branch2_value' where pk=1;", - "alter table schema_conflicts alter column name set default 'branch2_default';", - "call dolt_commit('-am', 'changes on branch2');", + + "update test_table set col1 = 'branch2_val', col2 = 300 where pk = 1;", + "call dolt_add('.')", + "call dolt_commit('-am', 'modify on branch2');", "call dolt_checkout('branch1')", - "update data_conflicts set value='branch1_value' where pk=1;", - "alter table schema_conflicts alter column name set default 'branch1_default';", - "call dolt_commit('-am', 'changes on branch1');", + "update test_table set col1 = 'branch1_val', col2 = 400 where pk = 1;", + "call dolt_add('.')", + "call dolt_commit('-am', 'modify on branch1');", "call dolt_checkout('main')", - "call dolt_merge('branch1')", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch1')", - Expected: []sql.Row{}, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts_summary('main', 'branch2') order by `table`", - Expected: []sql.Row{{"data_conflicts", uint64(1), uint64(0)}, {"schema_conflicts", nil, uint64(1)}}, + Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", + Expected: []sql.Row{{1}}, }, { - Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2') order by `table`", - Expected: []sql.Row{{"data_conflicts", uint64(1), uint64(0)}, {"schema_conflicts", nil, uint64(1)}}, + Query: "SELECT base_pk, our_col1, their_col1 from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", + Expected: []sql.Row{{1, "branch1_val", "branch2_val"}}, }, }, }, @@ -5211,272 +5461,18 @@ var PreviewMergeConflictsSummaryFunctionScripts = []queries.ScriptTest{ Query: "SELECT * from dolt_preview_merge_conflicts_summary('branch1', 'branch2')", Expected: []sql.Row{}, }, - }, - }, -} - -var PreviewMergeConflictsFunctionScripts = []queries.ScriptTest{ - { - Name: "invalid arguments", - SetUpScript: []string{ - "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", - "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", - "call dolt_add('.')", - "call dolt_commit('-am', 'creating table t');", - - "call dolt_branch('branch1')", - "call dolt_branch('branch2')", - }, - Assertions: []queries.ScriptTestAssertion{ - { - Query: "SELECT * from dolt_preview_merge_conflicts();", - ExpectedErr: sql.ErrInvalidArgumentNumber, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('t');", - ExpectedErr: sql.ErrInvalidArgumentNumber, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1');", - ExpectedErr: sql.ErrInvalidArgumentNumber, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't', 'extra');", - ExpectedErr: sql.ErrInvalidArgumentNumber, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(null, null, null);", - ExpectedErr: sql.ErrInvalidArgumentDetails, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 123, 't');", - ExpectedErr: sql.ErrInvalidArgumentDetails, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(123, 'branch1', 't');", - ExpectedErr: sql.ErrInvalidArgumentDetails, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 123);", - ExpectedErr: sql.ErrInvalidArgumentDetails, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('fake-branch', 'main', 't');", - ExpectedErrStr: "branch not found: fake-branch", - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'fake-branch', 't');", - ExpectedErrStr: "branch not found: fake-branch", - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main...branch1', 'branch2', 't');", - ExpectedErrStr: "string is not a valid branch or hash", - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', concat('branch', '1'), 't');", - ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(hashof('main'), 'branch1', 't');", - ExpectedErr: dtablefunctions.ErrInvalidNonLiteralArgument, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 'nope');", - ExpectedErr: sql.ErrTableNotFound, - }, - }, - }, - { - Name: "basic case with single table", - SetUpScript: []string{ - "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", - "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", - "call dolt_add('.')", - "set @Commit1 = '';", - "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", - - "call dolt_branch('branch1')", - "call dolt_checkout('-b', 'branch2')", - "update t set c1='one!' where pk=1", - "set @Commit2 = '';", - "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", - - "call dolt_checkout('branch1')", - "update t set c1='one?' where pk=1", - "set @Commit3 = '';", - "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", - - "call dolt_checkout('main')", - "call dolt_merge('branch1')", - }, - Assertions: []queries.ScriptTestAssertion{ { Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", Expected: []sql.Row{}, }, { - Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('main', 'branch2', 't')", - Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, - }, - { - Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", - Expected: []sql.Row{{1, "one", "two", 1, "one?", "two", "modified", 1, "one!", "two", "modified"}}, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches + Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch2', 't')", Expected: []sql.Row{}, }, { - Query: "SELECT base_pk, base_c1, base_c2, our_pk, our_c1, our_c2, our_diff_type, their_pk, their_c1, their_c2, their_diff_type from dolt_preview_merge_conflicts('branch2', 'main', 't')", - Expected: []sql.Row{{1, "one", "two", 1, "one!", "two", "modified", 1, "one?", "two", "modified"}}, - }, - }, - }, - { - Name: "basic case with keyless table", - SetUpScript: []string{ - "create table t (pk int, c1 varchar(20), c2 varchar(20));", - "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", - "call dolt_add('.')", - "set @Commit1 = '';", - "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", - - "call dolt_branch('branch1')", - "call dolt_checkout('-b', 'branch2')", - "update t set c1='one!' where pk=1", - "set @Commit2 = '';", - "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", - - "call dolt_checkout('branch1')", - "update t set c1='one?' where pk=1", - "set @Commit3 = '';", - "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", - - "call dolt_checkout('main')", - "call dolt_merge('branch1')", - }, - Assertions: []queries.ScriptTestAssertion{ - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", - Expected: []sql.Row{}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches - Expected: []sql.Row{}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", - Expected: []sql.Row{{1}}, - }, - }, - }, - { - Name: "basic case with multiple tables", - SetUpScript: []string{ - "create table t (pk int primary key, c1 varchar(20), c2 varchar(20));", - "create table t2 (pk int primary key, c1 varchar(20));", - "insert into t values (1, 'one', 'two'), (2, 'two', 'three');", - "insert into t2 values(100, 'hundred');", - "call dolt_add('.')", - "set @Commit1 = '';", - "call dolt_commit_hash_out(@Commit1, '-am', 'creating table t');", - - "call dolt_branch('branch1')", - "call dolt_checkout('-b', 'branch2')", - "update t set c1='one!' where pk=1", - "update t2 set c1='hundred!' where pk=100", - "set @Commit2 = '';", - "call dolt_commit_hash_out(@Commit2, '-am', 'update row 1 on branch2');", - - "call dolt_checkout('branch1')", - "update t set c1='one?' where pk=1", - "update t2 set c1='hundred?' where pk=100", - "set @Commit3 = '';", - "call dolt_commit_hash_out(@Commit3, '-am', 'update row 1 on branch1');", - - "call dolt_checkout('main')", - "call dolt_merge('branch1')", - - "create table keyless (id int);", - }, - Assertions: []queries.ScriptTestAssertion{ - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't')", + Query: "SELECT * from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", Expected: []sql.Row{}, }, - { - Query: "SELECT * from dolt_preview_merge_conflicts('main', 'branch1', 't2')", - Expected: []sql.Row{}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('main', 'branch2', 't2')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 't2')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT * from dolt_preview_merge_conflicts(@Commit1, @Commit2, 't')", // not branches - Expected: []sql.Row{}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch2', 'main', 't2')", - Expected: []sql.Row{{1}}, - }, - }, - }, - { - Name: "additional conflicts testing with multiple columns", - SetUpScript: []string{ - "create table test_table (pk int primary key, col1 varchar(20), col2 int);", - "insert into test_table values (1, 'original', 100), (2, 'second', 200);", - "call dolt_add('.')", - "call dolt_commit('-am', 'initial commit');", - - "call dolt_branch('branch1')", - "call dolt_checkout('-b', 'branch2')", - - "update test_table set col1 = 'branch2_val', col2 = 300 where pk = 1;", - "call dolt_add('.')", - "call dolt_commit('-am', 'modify on branch2');", - - "call dolt_checkout('branch1')", - "update test_table set col1 = 'branch1_val', col2 = 400 where pk = 1;", - "call dolt_add('.')", - "call dolt_commit('-am', 'modify on branch1');", - - "call dolt_checkout('main')", - }, - Assertions: []queries.ScriptTestAssertion{ - { - Query: "SELECT count(*) from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", - Expected: []sql.Row{{1}}, - }, - { - Query: "SELECT base_pk, our_col1, their_col1 from dolt_preview_merge_conflicts('branch1', 'branch2', 'test_table')", - Expected: []sql.Row{{1, "branch1_val", "branch2_val"}}, - }, }, }, } From 3ba5443a179cc33c3c0c5375b1c09654896dc79c Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 5 Jun 2025 16:17:47 -0700 Subject: [PATCH 08/12] Fix conflict id calculation for table function --- .../dtablefunctions/dolt_preview_merge_conflicts.go | 5 ++++- .../doltcore/sqle/dtables/conflicts_tables_prolly.go | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 405c61695ef..30f07fbaaff 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -558,7 +558,10 @@ func (itr *previewMergeConflictsTableFunctionRowIter) nextConflictVals(ctx *sql. confVal.Key = ca.Key confVal.Hash = itr.theirRootish - confVal.Id = dtables.GetConflictId(ca.Key, confVal.Hash) + + buf := make([]byte, 0, len(ca.Key)+len(confVal.Hash)) + buf = append(buf, ca.Key...) + confVal.Id = dtables.GetConflictId(buf, confVal.Hash) err = itr.loadTableMaps(ctx, itr.baseRootish, itr.theirRootish) if err != nil { diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index c31a135d808..b5702c7b6c2 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -414,11 +414,11 @@ type ConflictVal struct { // GetConflictId gets the conflict ID, ensuring that it is unique by hashing both theirRootish and the key of the table. func GetConflictId(key val.Tuple, confHash hash.Hash) string { // TODO: We were using this but it makes the dolt_preview_merge_conflicts table function panic with "slice bounds out of range" in flatbuffer table.SchemaBytes - // b := xxh3.Hash128(append(key, confHash[:]...)).Bytes() - buf := make([]byte, len(key)+len(confHash)) - copy(buf, key) - copy(buf[len(key):], confHash[:]) - b := xxh3.Hash128(buf).Bytes() + b := xxh3.Hash128(append(key, confHash[:]...)).Bytes() + // buf := make([]byte, len(key)+len(confHash)) + // copy(buf, key) + // copy(buf[len(key):], confHash[:]) + // b := xxh3.Hash128(buf).Bytes() return base64.RawStdEncoding.EncodeToString(b[:]) } From 09e334f34235a3fec08e2fdf5c1f16807db00eee Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Fri, 6 Jun 2025 11:34:24 -0700 Subject: [PATCH 09/12] Better naming, Schema doc string --- .../dolt_preview_merge_conflicts.go | 20 ++- .../sqle/dtables/conflicts_tables_prolly.go | 138 +++++++++--------- 2 files changed, 79 insertions(+), 79 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 30f07fbaaff..2cd939ff35a 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -112,6 +112,12 @@ func (pm *PreviewMergeConflictsTableFunction) String() string { } // Schema implements the sql.Node interface. +// Returns the schema for the preview merge conflicts table function, which includes: +// - from_root_ish: Hash of the right-side commit/working set during merge +// - base_[col], our_[col], their_[col]: Each column has these three columns representing the values of each column in the base, our, and their branches respectively +// - our_diff_type, their_diff_type: Indicates the type of change for our and their columns respectively ("added", "modified", "removed") +// - base_cardinality, our_cardinality, their_cardinality: Additional columns for keyless tables only, indicating the number of rows in the base, our, and their branches +// - dolt_conflict_id: Unique identifier for the conflict, derived from the key and the right-side commit hash func (pm *PreviewMergeConflictsTableFunction) Schema() sql.Schema { if !pm.Resolved() { return nil @@ -426,8 +432,8 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. return nil, err } - vds := dtables.GetConflictValueDescriptors(pm.baseSch, pm.ourSch, pm.theirSch, pm.rootInfo.baseRoot.NodeStore()) - offsets := dtables.GetConflictOffsets(keyless, vds) + cds := dtables.GetConflictDescriptors(pm.baseSch, pm.ourSch, pm.theirSch, pm.rootInfo.baseRoot.NodeStore()) + offsets := dtables.GetConflictOffsets(keyless, cds) valueMerger := tm.GetNewValueMerger(mergeSch, leftRows) @@ -455,7 +461,7 @@ func (pm *PreviewMergeConflictsTableFunction) RowIter(ctx *sql.Context, row sql. keyless: keyless, ourSch: pm.ourSch, offsets: offsets, - vds: vds, + cds: cds, baseRootish: baseHash, theirRootish: rightHash, }, nil @@ -507,7 +513,7 @@ type previewMergeConflictsTableFunctionRowIter struct { keyless bool ourSch schema.Schema - vds dtables.ConflictValueDescriptors + cds dtables.ConflictDescriptors offsets dtables.ConflictOffsets baseHash, theirHash hash.Hash @@ -520,7 +526,7 @@ func (itr *previewMergeConflictsTableFunctionRowIter) Next(ctx *sql.Context) (sq return nil, io.EOF } - row := make(sql.Row, itr.offsets.N) + row := make(sql.Row, itr.offsets.ColCount) confVal, err := itr.nextConflictVals(ctx) if err != nil { return nil, err @@ -656,12 +662,12 @@ func (itr *previewMergeConflictsTableFunctionRowIter) loadTableMaps(ctx *sql.Con func (itr *previewMergeConflictsTableFunctionRowIter) putConflictRowVals(ctx *sql.Context, confVal dtables.ConflictVal, row sql.Row) error { ns := itr.baseRows.NodeStore() - return dtables.PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) + return dtables.PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.cds, ns) } func (itr *previewMergeConflictsTableFunctionRowIter) putKeylessConflictRowVals(ctx *sql.Context, confVal dtables.ConflictVal, row sql.Row) (err error) { ns := itr.baseRows.NodeStore() - return dtables.PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) + return dtables.PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.cds, ns) } func (d *previewMergeConflictsTableFunctionRowIter) Close(context *sql.Context) error { diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index b5702c7b6c2..0fd6ca707ec 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -135,7 +135,7 @@ type prollyConflictRowIter struct { keyless bool ourSch schema.Schema - vds ConflictValueDescriptors + cds ConflictDescriptors offsets ConflictOffsets baseHash, theirHash hash.Hash @@ -147,55 +147,54 @@ type prollyConflictRowIter struct { // offsets are used to put the values in the correct place in the row. type ConflictOffsets struct { Base, Ours, Theirs int - N int + ColCount int } -// ConflictValueDescriptors holds the value descriptors for the base, ours, and theirs rows. -type ConflictValueDescriptors struct { - Base, Ours, Theirs val.TupleDesc - Key val.TupleDesc +// ConflictDescriptors holds the descriptors for the key and base, ours, and theirs values for a row. +type ConflictDescriptors struct { + BaseVal, OurVal, TheirVal val.TupleDesc + Key val.TupleDesc } // GetConflictOffsets returns the offsets of the columns in a conflict row. -func GetConflictOffsets(keyless bool, vds ConflictValueDescriptors) ConflictOffsets { +func GetConflictOffsets(keyless bool, cds ConflictDescriptors) ConflictOffsets { baseOffset := 1 - var ourOffset, theirOffset, n int + var ourOffset, theirOffset, colCount int if !keyless { - ourOffset = baseOffset + vds.Key.Count() + vds.Base.Count() - theirOffset = ourOffset + vds.Key.Count() + vds.Ours.Count() + 1 - n = theirOffset + vds.Key.Count() + vds.Theirs.Count() + 2 + ourOffset = baseOffset + cds.Key.Count() + cds.BaseVal.Count() + theirOffset = ourOffset + cds.Key.Count() + cds.OurVal.Count() + 1 + colCount = theirOffset + cds.Key.Count() + cds.TheirVal.Count() + 2 } else { - ourOffset = baseOffset + vds.Base.Count() - 1 - theirOffset = ourOffset + vds.Ours.Count() - n = theirOffset + vds.Theirs.Count() + 4 + ourOffset = baseOffset + cds.BaseVal.Count() - 1 + theirOffset = ourOffset + cds.OurVal.Count() + colCount = theirOffset + cds.TheirVal.Count() + 4 } return ConflictOffsets{ - Base: baseOffset, - Ours: ourOffset, - Theirs: theirOffset, - N: n, + Base: baseOffset, + Ours: ourOffset, + Theirs: theirOffset, + ColCount: colCount, } } -// GetConflictValueDescriptors returns the value descriptors for the base, ours, and theirs -// tables. -func GetConflictValueDescriptors(baseSch, ourSch, theirSch schema.Schema, ns tree.NodeStore) ConflictValueDescriptors { - keyVD := baseSch.GetKeyDescriptor(ns) +// GetConflictDescriptors returns the descriptors for the key and base, ours, and theirs +// values. +func GetConflictDescriptors(baseSch, ourSch, theirSch schema.Schema, ns tree.NodeStore) ConflictDescriptors { + key := baseSch.GetKeyDescriptor(ns) baseVD := baseSch.GetValueDescriptor(ns) oursVD := ourSch.GetValueDescriptor(ns) theirsVD := theirSch.GetValueDescriptor(ns) - return ConflictValueDescriptors{ - Base: baseVD, - Ours: oursVD, - Theirs: theirsVD, - Key: keyVD, + return ConflictDescriptors{ + BaseVal: baseVD, + OurVal: oursVD, + TheirVal: theirsVD, + Key: key, } } var _ sql.RowIter = (*prollyConflictRowIter)(nil) -// base_cols, our_cols, our_diff_type, their_cols, their_diff_type func newProllyConflictRowIter(ctx *sql.Context, ct ProllyConflictsTable) (*prollyConflictRowIter, error) { idx, err := ct.tbl.GetRowData(ctx) if err != nil { @@ -212,8 +211,8 @@ func newProllyConflictRowIter(ctx *sql.Context, ct ProllyConflictsTable) (*proll } keyless := schema.IsKeyless(ct.ourSch) - vds := GetConflictValueDescriptors(ct.baseSch, ct.ourSch, ct.theirSch, ct.root.NodeStore()) - offsets := GetConflictOffsets(keyless, vds) + cds := GetConflictDescriptors(ct.baseSch, ct.ourSch, ct.theirSch, ct.root.NodeStore()) + offsets := GetConflictOffsets(keyless, cds) return &prollyConflictRowIter{ itr: itr, @@ -223,7 +222,7 @@ func newProllyConflictRowIter(ctx *sql.Context, ct ProllyConflictsTable) (*proll ourRows: ourRows, keyless: keyless, ourSch: ct.ourSch, - vds: vds, + cds: cds, offsets: offsets, }, nil } @@ -234,7 +233,7 @@ func (itr *prollyConflictRowIter) Next(ctx *sql.Context) (sql.Row, error) { return nil, err } - row := make(sql.Row, itr.offsets.N) + row := make(sql.Row, itr.offsets.ColCount) row[0] = confVal.Hash.String() // from_root_ish if !itr.keyless { @@ -253,10 +252,10 @@ func (itr *prollyConflictRowIter) Next(ctx *sql.Context) (sql.Row, error) { } // PutConflictRowVals puts the values of the conflict row into the given row. -func PutConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, vds ConflictValueDescriptors, ns tree.NodeStore) error { +func PutConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, cds ConflictDescriptors, ns tree.NodeStore) error { // Sets key columns for the conflict row. - for i := 0; i < vds.Key.Count(); i++ { - f, err := tree.GetField(ctx, vds.Key, i, confVal.Key, ns) + for i := 0; i < cds.Key.Count(); i++ { + f, err := tree.GetField(ctx, cds.Key, i, confVal.Key, ns) if err != nil { return err } @@ -272,42 +271,42 @@ func PutConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offs } if confVal.Base != nil { - for i := 0; i < vds.Base.Count(); i++ { - f, err := tree.GetField(ctx, vds.Base, i, confVal.Base, ns) + for i := 0; i < cds.BaseVal.Count(); i++ { + f, err := tree.GetField(ctx, cds.BaseVal, i, confVal.Base, ns) if err != nil { return err } - baseColOffset := offsets.Base + vds.Key.Count() + i + baseColOffset := offsets.Base + cds.Key.Count() + i row[baseColOffset] = f // base_[col] } } if confVal.Ours != nil { - for i := 0; i < vds.Ours.Count(); i++ { - f, err := tree.GetField(ctx, vds.Ours, i, confVal.Ours, ns) + for i := 0; i < cds.OurVal.Count(); i++ { + f, err := tree.GetField(ctx, cds.OurVal, i, confVal.Ours, ns) if err != nil { return err } - ourColOffset := offsets.Ours + vds.Key.Count() + i + ourColOffset := offsets.Ours + cds.Key.Count() + i row[ourColOffset] = f // our_[col] } } - ourDiffTypeOffset := offsets.Ours + vds.Key.Count() + vds.Ours.Count() + ourDiffTypeOffset := offsets.Ours + cds.Key.Count() + cds.OurVal.Count() row[ourDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Ours) // our_diff_type if confVal.Theirs != nil { - for i := 0; i < vds.Theirs.Count(); i++ { - f, err := tree.GetField(ctx, vds.Theirs, i, confVal.Theirs, ns) + for i := 0; i < cds.TheirVal.Count(); i++ { + f, err := tree.GetField(ctx, cds.TheirVal, i, confVal.Theirs, ns) if err != nil { return err } - theirColOffset := offsets.Theirs + vds.Key.Count() + i + theirColOffset := offsets.Theirs + cds.Key.Count() + i row[theirColOffset] = f // their_[col] } } - theirDiffTypeOffset := offsets.Theirs + vds.Key.Count() + vds.Theirs.Count() + theirDiffTypeOffset := offsets.Theirs + cds.Key.Count() + cds.TheirVal.Count() row[theirDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Theirs) // their_diff_type conflictIdOffset := theirDiffTypeOffset + 1 @@ -318,7 +317,7 @@ func PutConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offs func (itr *prollyConflictRowIter) putConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row) error { ns := itr.baseRows.NodeStore() - return PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) + return PutConflictRowVals(ctx, confVal, row, itr.offsets, itr.cds, ns) } func getConflictDiffType(base val.Tuple, other val.Tuple) string { @@ -333,16 +332,16 @@ func getConflictDiffType(base val.Tuple, other val.Tuple) string { } // PutKeylessConflictRowVals puts the values of the keyless conflict row into the given row. -func PutKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, vds ConflictValueDescriptors, ns tree.NodeStore) (err error) { +func PutKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row, offsets ConflictOffsets, cds ConflictDescriptors, ns tree.NodeStore) (err error) { if confVal.Base != nil { - f, err := tree.GetField(ctx, vds.Base, 0, confVal.Base, ns) + f, err := tree.GetField(ctx, cds.BaseVal, 0, confVal.Base, ns) if err != nil { return err } - row[offsets.N-3] = f // base_cardinality + row[offsets.ColCount-3] = f // base_cardinality - for i := 0; i < vds.Base.Count()-1; i++ { - f, err := tree.GetField(ctx, vds.Base, i+1, confVal.Base, ns) + for i := 0; i < cds.BaseVal.Count()-1; i++ { + f, err := tree.GetField(ctx, cds.BaseVal, i+1, confVal.Base, ns) if err != nil { return err } @@ -350,59 +349,59 @@ func PutKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Ro row[baseColOffset] = f // base_[col] } } else { - row[offsets.N-3] = uint64(0) // base_cardinality + row[offsets.ColCount-3] = uint64(0) // base_cardinality } if confVal.Ours != nil { - f, err := tree.GetField(ctx, vds.Ours, 0, confVal.Ours, ns) + f, err := tree.GetField(ctx, cds.OurVal, 0, confVal.Ours, ns) if err != nil { return err } - row[offsets.N-2] = f // our_cardinality + row[offsets.ColCount-2] = f // our_cardinality - for i := 0; i < vds.Ours.Count()-1; i++ { - f, err := tree.GetField(ctx, vds.Ours, i+1, confVal.Ours, ns) + for i := 0; i < cds.OurVal.Count()-1; i++ { + f, err := tree.GetField(ctx, cds.OurVal, i+1, confVal.Ours, ns) if err != nil { return err } row[offsets.Ours+i] = f // our_[col] } } else { - row[offsets.N-2] = uint64(0) // our_cardinality + row[offsets.ColCount-2] = uint64(0) // our_cardinality } - ourDiffTypeOffset := offsets.Ours + vds.Ours.Count() - 1 + ourDiffTypeOffset := offsets.Ours + cds.OurVal.Count() - 1 row[ourDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Ours) // our_diff_type if confVal.Theirs != nil { - f, err := tree.GetField(ctx, vds.Theirs, 0, confVal.Theirs, ns) + f, err := tree.GetField(ctx, cds.TheirVal, 0, confVal.Theirs, ns) if err != nil { return err } - row[offsets.N-1] = f // their_cardinality + row[offsets.ColCount-1] = f // their_cardinality - for i := 0; i < vds.Theirs.Count()-1; i++ { - f, err := tree.GetField(ctx, vds.Theirs, i+1, confVal.Theirs, ns) + for i := 0; i < cds.TheirVal.Count()-1; i++ { + f, err := tree.GetField(ctx, cds.TheirVal, i+1, confVal.Theirs, ns) if err != nil { return err } row[offsets.Theirs+i] = f // their_[col] } } else { - row[offsets.N-1] = uint64(0) // their_cardinality + row[offsets.ColCount-1] = uint64(0) // their_cardinality } - theirDiffTypeOffset := offsets.Theirs + vds.Theirs.Count() - 1 + theirDiffTypeOffset := offsets.Theirs + cds.TheirVal.Count() - 1 row[theirDiffTypeOffset] = getConflictDiffType(confVal.Base, confVal.Theirs) // their_diff_type - row[offsets.N-4] = confVal.Id // dolt_conflict_id + row[offsets.ColCount-4] = confVal.Id // dolt_conflict_id return nil } func (itr *prollyConflictRowIter) putKeylessConflictRowVals(ctx *sql.Context, confVal ConflictVal, row sql.Row) (err error) { ns := itr.baseRows.NodeStore() - return PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.vds, ns) + return PutKeylessConflictRowVals(ctx, confVal, row, itr.offsets, itr.cds, ns) } type ConflictVal struct { @@ -413,12 +412,7 @@ type ConflictVal struct { // GetConflictId gets the conflict ID, ensuring that it is unique by hashing both theirRootish and the key of the table. func GetConflictId(key val.Tuple, confHash hash.Hash) string { - // TODO: We were using this but it makes the dolt_preview_merge_conflicts table function panic with "slice bounds out of range" in flatbuffer table.SchemaBytes b := xxh3.Hash128(append(key, confHash[:]...)).Bytes() - // buf := make([]byte, len(key)+len(confHash)) - // copy(buf, key) - // copy(buf[len(key):], confHash[:]) - // b := xxh3.Hash128(buf).Bytes() return base64.RawStdEncoding.EncodeToString(b[:]) } From e5ee96448b7df4b09fa2aa64acb0a9387c87f56d Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 9 Jun 2025 11:57:10 -0700 Subject: [PATCH 10/12] More feedback --- .../dolt_preview_merge_conflicts_summary.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go index 4458e1d0aa6..e8c5666dea3 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts_summary.go @@ -243,14 +243,6 @@ type previewMergeConflictsSummaryTableFunctionRowIter struct { conIdx int } -func (iter *previewMergeConflictsSummaryTableFunctionRowIter) incrementIndexes() { - iter.conIdx++ - if iter.conIdx >= len(iter.conflicts) { - iter.conIdx = 0 - iter.conflicts = nil - } -} - func NewPreviewMergeConflictsSummaryTableFunctionRowIter(pm []tableConflict) sql.RowIter { return &previewMergeConflictsSummaryTableFunctionRowIter{ conflicts: pm, @@ -258,12 +250,12 @@ func NewPreviewMergeConflictsSummaryTableFunctionRowIter(pm []tableConflict) sql } func (iter *previewMergeConflictsSummaryTableFunctionRowIter) Next(ctx *sql.Context) (sql.Row, error) { - defer iter.incrementIndexes() - if iter.conIdx >= len(iter.conflicts) || iter.conflicts == nil { + if iter.conIdx >= len(iter.conflicts) { return nil, io.EOF } conflict := iter.conflicts[iter.conIdx] + iter.conIdx++ return getRowFromConflict(conflict), nil } @@ -342,7 +334,7 @@ func resolveBranchesToRoots(ctx *sql.Context, db dsess.SqlDatabase, leftBranch, type tableConflict struct { tableName doltdb.TableName - numDataConflicts uint64 // nil if schema conflicts exist + numDataConflicts uint64 // ignored if schema conflicts exist numSchemaConflicts uint64 } From e0225f4b0a595d27fb5ee152fbac2c74cb7eab77 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 9 Jun 2025 12:57:14 -0700 Subject: [PATCH 11/12] Add comments --- .../dolt_preview_merge_conflicts.go | 11 ++++++----- .../sqle/dtables/conflicts_tables_prolly.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index 2cd939ff35a..c0b34b58468 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -113,11 +113,12 @@ func (pm *PreviewMergeConflictsTableFunction) String() string { // Schema implements the sql.Node interface. // Returns the schema for the preview merge conflicts table function, which includes: -// - from_root_ish: Hash of the right-side commit/working set during merge -// - base_[col], our_[col], their_[col]: Each column has these three columns representing the values of each column in the base, our, and their branches respectively -// - our_diff_type, their_diff_type: Indicates the type of change for our and their columns respectively ("added", "modified", "removed") -// - base_cardinality, our_cardinality, their_cardinality: Additional columns for keyless tables only, indicating the number of rows in the base, our, and their branches -// - dolt_conflict_id: Unique identifier for the conflict, derived from the key and the right-side commit hash +// - from_root_ish: Hash of the right-side commit/working set during merge. +// - For each column on the named table, base_[col], our_[col], and their_[col] represent the value of that column on the base, our, and their branches respectively. +// All base columns come first, followed by all our columns, and then their columns. +// - our_diff_type, their_diff_type: Indicates the type of change for our and their columns respectively ("added", "modified", "removed"). +// - base_cardinality, our_cardinality, their_cardinality: Additional columns for keyless tables only, indicating the number of occurrences of the conflicting row in the base, our, and their branches. +// - dolt_conflict_id: Unique identifier for the conflict, derived from the key and the right-side commit hash. func (pm *PreviewMergeConflictsTableFunction) Schema() sql.Schema { if !pm.Resolved() { return nil diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 0fd6ca707ec..29406b5291d 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -145,6 +145,8 @@ type prollyConflictRowIter struct { // ConflictOffsets holds the offsets of the columns in a conflict row. The // offsets are used to put the values in the correct place in the row. +// Base is the offset of the first base column, Ours is the offset of the first +// ours column, and Theirs is the offset of the first theirs column. type ConflictOffsets struct { Base, Ours, Theirs int ColCount int @@ -157,16 +159,29 @@ type ConflictDescriptors struct { } // GetConflictOffsets returns the offsets of the columns in a conflict row. +// +// For keyed tables, the conflict row structure is: +// [from_root_ish] [base_key...] [base_vals...] [our_key...] [our_vals...] [our_diff_type] [their_key...] [their_vals...] [their_diff_type] [dolt_conflict_id] +// +// For keyless tables, the conflict row structure is: +// [from_root_ish] [base_vals...] [our_vals...] [our_diff_type] [their_vals...] [their_diff_type] [dolt_conflict_id] [base_cardinality] [our_cardinality] [their_cardinality] func GetConflictOffsets(keyless bool, cds ConflictDescriptors) ConflictOffsets { + // Skip index 0 which is always from_root_ish baseOffset := 1 var ourOffset, theirOffset, colCount int if !keyless { + // Base section: base key columns + base value columns ourOffset = baseOffset + cds.Key.Count() + cds.BaseVal.Count() + // +1 for our_diff_type column that follows the ours section theirOffset = ourOffset + cds.Key.Count() + cds.OurVal.Count() + 1 + // +2 for their_diff_type and dolt_conflict_id columns at the end colCount = theirOffset + cds.Key.Count() + cds.TheirVal.Count() + 2 } else { + // For keyless: base values (excluding cardinality which comes at the end) ourOffset = baseOffset + cds.BaseVal.Count() - 1 + // Ours section: our value columns (excluding cardinality) theirOffset = ourOffset + cds.OurVal.Count() + // +4 for our_diff_type, their_diff_type, dolt_conflict_id, and 3 cardinality columns colCount = theirOffset + cds.TheirVal.Count() + 4 } From f1f9decdca371b5101fd52646d9b33517659a5f6 Mon Sep 17 00:00:00 2001 From: tbantle22 Date: Mon, 9 Jun 2025 22:10:59 +0000 Subject: [PATCH 12/12] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- .../sqle/dtablefunctions/dolt_preview_merge_conflicts.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go index c0b34b58468..3d850bafe06 100644 --- a/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go +++ b/go/libraries/doltcore/sqle/dtablefunctions/dolt_preview_merge_conflicts.go @@ -18,6 +18,10 @@ import ( "fmt" "io" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" + "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/merge" @@ -30,9 +34,6 @@ import ( "github.com/dolthub/dolt/go/store/prolly/tree" dtypes "github.com/dolthub/dolt/go/store/types" "github.com/dolthub/dolt/go/store/val" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/expression" - "github.com/dolthub/go-mysql-server/sql/types" ) var _ sql.TableFunction = (*PreviewMergeConflictsTableFunction)(nil)