Skip to content

Commit

Permalink
Merge pull request #8402 from dolthub/zachmu/merge-2
Browse files Browse the repository at this point in the history
More TableName fixes for system tables / related methods
  • Loading branch information
zachmu authored Sep 30, 2024
2 parents 8aca26c + 0e3fac9 commit 471d129
Show file tree
Hide file tree
Showing 13 changed files with 70 additions and 77 deletions.
2 changes: 1 addition & 1 deletion go/cmd/dolt/commands/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 28 additions & 4 deletions go/libraries/doltcore/doltdb/root_val.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -790,6 +790,30 @@ 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()
}

// 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 = ""
Expand Down
15 changes: 1 addition & 14 deletions go/libraries/doltcore/doltdb/system_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,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.
Expand Down Expand Up @@ -200,7 +187,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"
)

Expand Down
4 changes: 2 additions & 2 deletions go/libraries/doltcore/env/actions/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}
Expand Down
29 changes: 20 additions & 9 deletions go/libraries/doltcore/env/actions/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 1 addition & 14 deletions go/libraries/doltcore/env/actions/staged.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package actions

import (
"context"
"fmt"
"strings"

"github.com/dolthub/dolt/go/libraries/doltcore/diff"
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion go/libraries/doltcore/env/actions/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions go/libraries/doltcore/merge/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion go/libraries/doltcore/sqle/dprocedures/dolt_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions go/libraries/doltcore/sqle/dtables/merge_status_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ package dtables
import (
"context"
"io"
"strings"

"github.com/dolthub/go-mysql-server/sql"
"github.com/dolthub/go-mysql-server/sql/types"

"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
Expand Down Expand Up @@ -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
Expand All @@ -131,8 +130,10 @@ func newMergeStatusItr(ctx context.Context, ws *doltdb.WorkingSet) (*MergeStatus
s3 := curr.String()
target = &s3

s4 := strings.Join(unmergedTblNames.AsSlice(), ", ")
unmergedTables = &s4
// 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
}

return &MergeStatusIter{
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/dtables/status_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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})
}
}

Expand Down

0 comments on commit 471d129

Please sign in to comment.