From aa062cfcd01745ff9ce01806754b0cef526f11aa Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 26 Sep 2024 17:36:55 -0700 Subject: [PATCH 1/2] Plumb table name more places --- go/cmd/dolt/commands/reset.go | 2 +- go/libraries/doltcore/doltdb/root_val.go | 20 ++++++++++--- go/libraries/doltcore/doltdb/system_table.go | 15 +--------- go/libraries/doltcore/env/actions/commit.go | 4 +-- go/libraries/doltcore/env/actions/errors.go | 29 +++++++++++++------ go/libraries/doltcore/env/actions/staged.go | 15 +--------- go/libraries/doltcore/env/actions/table.go | 2 +- go/libraries/doltcore/merge/merge.go | 19 ------------ .../doltcore/sqle/dprocedures/dolt_add.go | 3 +- .../doltcore/sqle/dprocedures/dolt_rebase.go | 2 +- .../sqle/dtables/merge_status_table.go | 13 ++++----- .../doltcore/sqle/dtables/status_table.go | 2 +- .../dtables/table_of_tables_in_conflict.go | 7 +++-- 13 files changed, 56 insertions(+), 77 deletions(-) diff --git a/go/cmd/dolt/commands/reset.go b/go/cmd/dolt/commands/reset.go index 398731b3075..d27a01a55e8 100644 --- a/go/cmd/dolt/commands/reset.go +++ b/go/cmd/dolt/commands/reset.go @@ -257,7 +257,7 @@ func handleResetError(err error, usage cli.UsagePrinter) int { } for _, tbl := range tbls { - bdr.AddDetails("\t" + tbl) + bdr.AddDetails("\t" + tbl.Name) } return HandleVErrAndExitCode(bdr.Build(), usage) diff --git a/go/libraries/doltcore/doltdb/root_val.go b/go/libraries/doltcore/doltdb/root_val.go index 502f89d14e8..a91fcfb6cb3 100644 --- a/go/libraries/doltcore/doltdb/root_val.go +++ b/go/libraries/doltcore/doltdb/root_val.go @@ -628,15 +628,15 @@ func (root *rootValue) getTableMap(ctx context.Context, schemaName string) (tabl return root.st.GetTablesMap(ctx, root.vrw, root.ns, schemaName) } -func TablesWithDataConflicts(ctx context.Context, root RootValue) ([]string, error) { - names, err := root.GetTableNames(ctx, DefaultSchemaName) +func TablesWithDataConflicts(ctx context.Context, root RootValue) ([]TableName, error) { + names, err := UnionTableNames(ctx, root) if err != nil { return nil, err } - conflicted := make([]string, 0, len(names)) + conflicted := make([]TableName, 0, len(names)) for _, name := range names { - tbl, _, err := root.GetTable(ctx, TableName{Name: name}) + tbl, _, err := root.GetTable(ctx, name) if err != nil { return nil, err } @@ -790,6 +790,18 @@ func FlattenTableNames(names []TableName) []string { return tbls } +// TableNamesAsString returns a comma-separated string of the table names given +func TableNamesAsString(names []TableName) string { + sb := strings.Builder{} + for i, name := range names { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(name.String()) + } + return sb.String() +} + // DefaultSchemaName is the name of the default schema. Tables with this schema name will be stored in the // primary (unnamed) table store in a root. var DefaultSchemaName = "" diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index 7d3198f981a..8fcb3d67c13 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -140,19 +140,6 @@ func GetGeneratedSystemTables(ctx context.Context, root RootValue) ([]string, er return s.AsSlice(), nil } -// GetAllTableNames returns table names for all persisted and generated tables. -func GetAllTableNames(ctx context.Context, root RootValue) ([]string, error) { - n, err := GetNonSystemTableNames(ctx, root) - if err != nil { - return nil, err - } - s, err := GetSystemTableNames(ctx, root) - if err != nil { - return nil, err - } - return append(n, s...), nil -} - // The set of reserved dolt_ tables that should be considered part of user space, like any other user-created table, // for the purposes of the dolt command line. These tables cannot be created or altered explicitly, but can be updated // like normal SQL tables. @@ -206,7 +193,7 @@ const ( DocTableName = "dolt_docs" // DocPkColumnName is the name of the pk column in the docs table DocPkColumnName = "doc_name" - //DocTextColumnName is the name of the column containing the document contents in the docs table + // DocTextColumnName is the name of the column containing the document contents in the docs table DocTextColumnName = "doc_text" ) diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index 6063be3e46a..9e33b6a4fec 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -89,13 +89,13 @@ func GetCommitStaged( return nil, err } if len(violatesConstraints) > 0 { - return nil, NewTblHasConstraintViolations(doltdb.FlattenTableNames(violatesConstraints)) + return nil, NewTblHasConstraintViolations(violatesConstraints) } if ws.MergeActive() { schConflicts := ws.MergeState().TablesWithSchemaConflicts() if len(schConflicts) > 0 { - return nil, NewTblSchemaConflictError(schConflicts) + return nil, NewTblSchemaConflictError(doltdb.ToTableNames(schConflicts, doltdb.DefaultSchemaName)) } } } diff --git a/go/libraries/doltcore/env/actions/errors.go b/go/libraries/doltcore/env/actions/errors.go index 585bde82541..6210a8bbcdf 100644 --- a/go/libraries/doltcore/env/actions/errors.go +++ b/go/libraries/doltcore/env/actions/errors.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/dolthub/dolt/go/libraries/doltcore/diff" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" ) type tblErrorType string @@ -32,28 +33,38 @@ const ( ) type TblError struct { - tables []string + tables []doltdb.TableName tblErrType tblErrorType } -func NewTblNotExistError(tbls []string) TblError { +func NewTblNotExistError(tbls []doltdb.TableName) TblError { return TblError{tbls, tblErrTypeNotExist} } -func NewTblInConflictError(tbls []string) TblError { - return TblError{tbls, tblErrTypeInConflict} +func NewTblInConflictError(tbls []doltdb.TableName) TblError { + return TblError{tables: tbls, tblErrType: tblErrTypeInConflict} } -func NewTblSchemaConflictError(tbls []string) TblError { - return TblError{tbls, tblErrTypeSchConflict} +func NewTblSchemaConflictError(tbls []doltdb.TableName) TblError { + return TblError{tables: tbls, tblErrType: tblErrTypeSchConflict} } -func NewTblHasConstraintViolations(tbls []string) TblError { +func NewTblHasConstraintViolations(tbls []doltdb.TableName) TblError { return TblError{tbls, tblErrTypeConstViols} } func (te TblError) Error() string { - return "error: the table(s) " + strings.Join(te.tables, ", ") + " " + string(te.tblErrType) + sb := strings.Builder{} + sb.WriteString("error: the table(s) ") + for i, tbl := range te.tables { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(tbl.String()) + } + sb.WriteString(" ") + sb.WriteString(string(te.tblErrType)) + return sb.String() } func getTblErrType(err error) tblErrorType { @@ -82,7 +93,7 @@ func IsTblViolatesConstraints(err error) bool { return getTblErrType(err) == tblErrTypeConstViols } -func GetTablesForError(err error) []string { +func GetTablesForError(err error) []doltdb.TableName { te, ok := err.(TblError) if !ok { diff --git a/go/libraries/doltcore/env/actions/staged.go b/go/libraries/doltcore/env/actions/staged.go index 3bffa690eaf..a753728f05e 100644 --- a/go/libraries/doltcore/env/actions/staged.go +++ b/go/libraries/doltcore/env/actions/staged.go @@ -16,7 +16,6 @@ package actions import ( "context" - "fmt" "strings" "github.com/dolthub/dolt/go/libraries/doltcore/diff" @@ -219,17 +218,5 @@ func ValidateTables(ctx context.Context, tbls []doltdb.TableName, roots ...doltd return nil } - return NewTblNotExistError(summarizeTableNames(missing)) -} - -func summarizeTableNames(names []doltdb.TableName) []string { - namesStrs := make([]string, len(names)) - for i, name := range names { - if name.Schema != "" { - namesStrs[i] = fmt.Sprintf("%s.%s", name.Schema, name.Name) - } else { - namesStrs[i] = fmt.Sprintf("%s", name.Name) - } - } - return namesStrs + return NewTblNotExistError(missing) } diff --git a/go/libraries/doltcore/env/actions/table.go b/go/libraries/doltcore/env/actions/table.go index c9c6fefda3d..4615d4984bd 100644 --- a/go/libraries/doltcore/env/actions/table.go +++ b/go/libraries/doltcore/env/actions/table.go @@ -167,7 +167,7 @@ func validateTablesExist(ctx context.Context, currRoot doltdb.RootValue, unknown } if len(notExist) > 0 { - return NewTblNotExistError(summarizeTableNames(notExist)) + return NewTblNotExistError(notExist) } return nil diff --git a/go/libraries/doltcore/merge/merge.go b/go/libraries/doltcore/merge/merge.go index 41f14c3c05f..af414ce126a 100644 --- a/go/libraries/doltcore/merge/merge.go +++ b/go/libraries/doltcore/merge/merge.go @@ -528,25 +528,6 @@ func (as ArtifactStatus) HasConstraintViolations() bool { return len(as.ConstraintViolationsTables) > 0 } -func GetMergeArtifactStatus(ctx context.Context, working *doltdb.WorkingSet) (as ArtifactStatus, err error) { - if working.MergeActive() { - as.SchemaConflictsTables = working.MergeState().TablesWithSchemaConflicts() - } - - as.DataConflictTables, err = doltdb.TablesWithDataConflicts(ctx, working.WorkingRoot()) - if err != nil { - return as, err - } - - violations, err := doltdb.TablesWithConstraintViolations(ctx, working.WorkingRoot()) - if err != nil { - return ArtifactStatus{}, err - } - - as.ConstraintViolationsTables = doltdb.FlattenTableNames(violations) - return -} - // MergeWouldStompChanges returns list of table names that are stomped and the diffs map between head and working set. func MergeWouldStompChanges(ctx context.Context, roots doltdb.Roots, mergeCommit *doltdb.Commit) ([]string, map[string]hash.Hash, error) { mergeRoot, err := mergeCommit.GetRootValue(ctx) diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_add.go b/go/libraries/doltcore/sqle/dprocedures/dolt_add.go index 288949bb111..eb73133734e 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_add.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_add.go @@ -115,7 +115,8 @@ func doDoltAdd(ctx *sql.Context, args []string) (int, error) { // This mirrors the logic in actions.StageTables if len(missingTables) > 0 { - return 1, actions.NewTblNotExistError(missingTables) + // TODO: schema names + return 1, actions.NewTblNotExistError(doltdb.ToTableNames(missingTables, doltdb.DefaultSchemaName)) } } else { tableNames = doltdb.ToTableNames(unqualifiedTableNames, doltdb.DefaultSchemaName) diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go b/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go index 1aa38f4ee6b..d89a5a8b05e 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go @@ -467,7 +467,7 @@ func validateNoConflicts(ctx *sql.Context) error { } if len(tablesWithDataConflicts) > 0 { return ErrRebaseUnresolvedConflicts.New( - strings.Join(tablesWithDataConflicts, ", ")) + doltdb.TableNamesAsString(tablesWithDataConflicts)) } return nil diff --git a/go/libraries/doltcore/sqle/dtables/merge_status_table.go b/go/libraries/doltcore/sqle/dtables/merge_status_table.go index fc6f7e2cedf..496e4ba49de 100644 --- a/go/libraries/doltcore/sqle/dtables/merge_status_table.go +++ b/go/libraries/doltcore/sqle/dtables/merge_status_table.go @@ -17,7 +17,6 @@ package dtables import ( "context" "io" - "strings" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/types" @@ -25,7 +24,6 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/index" - "github.com/dolthub/dolt/go/libraries/utils/set" ) // MergeStatusTable is a sql.Table implementation that implements a system table @@ -103,9 +101,10 @@ func newMergeStatusItr(ctx context.Context, ws *doltdb.WorkingSet) (*MergeStatus schConflicts = ws.MergeState().TablesWithSchemaConflicts() } - unmergedTblNames := set.NewStrSet(inConflict) - unmergedTblNames.Add(doltdb.FlattenTableNames(tblsWithViolations)...) - unmergedTblNames.Add(schConflicts...) + unmergedTblNames := doltdb.NewTableNameSet(inConflict) + unmergedTblNames.Add(tblsWithViolations...) + // TODO: schema name + unmergedTblNames.Add(doltdb.ToTableNames(schConflicts, doltdb.DefaultSchemaName)...) var sourceCommitSpecStr *string var sourceCommitHash *string @@ -131,8 +130,8 @@ func newMergeStatusItr(ctx context.Context, ws *doltdb.WorkingSet) (*MergeStatus s3 := curr.String() target = &s3 - s4 := strings.Join(unmergedTblNames.AsSlice(), ", ") - unmergedTables = &s4 + tableNamesAsString := doltdb.TableNamesAsString(unmergedTblNames.AsSlice()) + unmergedTables = &tableNamesAsString } return &MergeStatusIter{ diff --git a/go/libraries/doltcore/sqle/dtables/status_table.go b/go/libraries/doltcore/sqle/dtables/status_table.go index fc885c535a9..398a719b688 100644 --- a/go/libraries/doltcore/sqle/dtables/status_table.go +++ b/go/libraries/doltcore/sqle/dtables/status_table.go @@ -166,7 +166,7 @@ func newStatusItr(ctx *sql.Context, st *StatusTable) (*StatusItr, error) { } for _, tbl := range cnfTables { rows = append(rows, statusTableRow{ - tableName: tbl, + tableName: tbl.Name, status: mergeConflictStatus, }) } diff --git a/go/libraries/doltcore/sqle/dtables/table_of_tables_in_conflict.go b/go/libraries/doltcore/sqle/dtables/table_of_tables_in_conflict.go index c3d3ca8d52f..7489ca277ad 100644 --- a/go/libraries/doltcore/sqle/dtables/table_of_tables_in_conflict.go +++ b/go/libraries/doltcore/sqle/dtables/table_of_tables_in_conflict.go @@ -129,12 +129,13 @@ func (dt *TableOfTablesInConflict) Partitions(ctx *sql.Context) (sql.PartitionIt if ws.MergeActive() { schConflicts := ws.MergeState().TablesWithSchemaConflicts() - tblNames = append(tblNames, schConflicts...) + // TODO: schema name + tblNames = append(tblNames, doltdb.ToTableNames(schConflicts, doltdb.DefaultSchemaName)...) } var partitions []*tableInConflict for _, tblName := range tblNames { - tbl, ok, err := root.GetTable(ctx, doltdb.TableName{Name: tblName}) + tbl, ok, err := root.GetTable(ctx, tblName) if err != nil { return nil, err @@ -143,7 +144,7 @@ func (dt *TableOfTablesInConflict) Partitions(ctx *sql.Context) (sql.PartitionIt if err != nil { return nil, err } - partitions = append(partitions, &tableInConflict{tblName, n, false}) + partitions = append(partitions, &tableInConflict{name: tblName.Name, size: n}) } } From 0e3fac91f8445e2fd97273842f90a9cfdb84e250 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 27 Sep 2024 13:48:57 -0700 Subject: [PATCH 2/2] Bug fix for merge status table --- go/libraries/doltcore/doltdb/root_val.go | 12 ++++++++++++ .../doltcore/sqle/dtables/merge_status_table.go | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/root_val.go b/go/libraries/doltcore/doltdb/root_val.go index a91fcfb6cb3..261256d147e 100644 --- a/go/libraries/doltcore/doltdb/root_val.go +++ b/go/libraries/doltcore/doltdb/root_val.go @@ -802,6 +802,18 @@ func TableNamesAsString(names []TableName) string { return sb.String() } +// UnqualifiedTableNamesAsString returns a comma-separated string of the table names given +func UnqualifiedTableNamesAsString(names []TableName) string { + sb := strings.Builder{} + for i, name := range names { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(name.Name) + } + return sb.String() +} + // DefaultSchemaName is the name of the default schema. Tables with this schema name will be stored in the // primary (unnamed) table store in a root. var DefaultSchemaName = "" diff --git a/go/libraries/doltcore/sqle/dtables/merge_status_table.go b/go/libraries/doltcore/sqle/dtables/merge_status_table.go index 496e4ba49de..7ac092fb831 100644 --- a/go/libraries/doltcore/sqle/dtables/merge_status_table.go +++ b/go/libraries/doltcore/sqle/dtables/merge_status_table.go @@ -130,7 +130,9 @@ func newMergeStatusItr(ctx context.Context, ws *doltdb.WorkingSet) (*MergeStatus s3 := curr.String() target = &s3 - tableNamesAsString := doltdb.TableNamesAsString(unmergedTblNames.AsSlice()) + // TODO: it might be nice to include schema name in this output, not sure yet + // It makes testing more challenging to have the behavior diverge between Dolt and Doltgres though + tableNamesAsString := doltdb.UnqualifiedTableNamesAsString(unmergedTblNames.AsSlice()) unmergedTables = &tableNamesAsString }