Neil changes + many edge cases

This commit is contained in:
Nathan Gabrielson
2025-06-26 15:29:36 -07:00
parent ac820ba945
commit 97b6a84092
5 changed files with 158 additions and 29 deletions
+2 -2
View File
@@ -330,9 +330,9 @@ func CreateReflogArgParser() *argparser.ArgParser {
}
func CreateRmArgParser() *argparser.ArgParser {
ap := argparser.NewArgParserWithVariableArgs("add")
ap := argparser.NewArgParserWithVariableArgs("rm")
ap.ArgListHelp = append(ap.ArgListHelp, [2]string{"table", "table(s) to remove from the list of tables staged to be committed."})
ap.SupportsFlag(CachedFlag, "", "description here") //TODO
ap.SupportsFlag(CachedFlag, "", "Use this option to unstage and remove tables only from the index. Working tree tables, whether modified or not, will be left alone.") //TODO
return ap
}
+6 -1
View File
@@ -30,7 +30,8 @@ const (
tblErrTypeInConflict tblErrorType = "are in conflict"
tblErrTypeSchConflict tblErrorType = "have schema conflicts"
tblErrTypeConstViols tblErrorType = "have constraint violations"
tblErrStagedChanges tblErrorType = "have changes saved in the index."
tblErrStagedChanges tblErrorType = "have changes saved in the index. Use --cached or commit."
tblErrUnstagedChanges tblErrorType = "have unstaged changes."
)
type TblError struct {
@@ -46,6 +47,10 @@ func NewTblStagedError(tbls []doltdb.TableName) TblError {
return TblError{tbls, tblErrStagedChanges}
}
func NewTblUnstagedError(tbls []doltdb.TableName) TblError {
return TblError{tbls, tblErrUnstagedChanges}
}
func NewTblInConflictError(tbls []doltdb.TableName) TblError {
return TblError{tables: tbls, tblErrType: tblErrTypeInConflict}
}
@@ -21,6 +21,7 @@ import (
"github.com/dolthub/dolt/go/cmd/dolt/cli"
"github.com/dolthub/dolt/go/libraries/doltcore/branch_control"
"github.com/dolthub/dolt/go/libraries/doltcore/diff"
"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
@@ -71,7 +72,7 @@ func doDoltRm(ctx *sql.Context, args []string) (int, error) {
roots, ok := dSess.GetRoots(ctx, dbName)
if !ok {
return 1, fmt.Errorf("Could not load database %s", dbName)
return 1, fmt.Errorf("Could not load roots for database %s", dbName)
}
checkStaged := apr.Contains(cli.CachedFlag)
@@ -105,10 +106,12 @@ func doDoltRm(ctx *sql.Context, args []string) (int, error) {
func verifyTables(ctx *sql.Context, unqualifiedTables []string, checkStaged bool, roots doltdb.Roots) ([]doltdb.TableName, error) {
var missingTables []string
var missingStaged []string
var missingStagedTables []string
var unstagedTables []string
var TableNames []doltdb.TableName
for _, name := range unqualifiedTables {
_, okHead, err := resolve.TableName(ctx, roots.Head, name)
if err != nil {
return nil, err
@@ -118,28 +121,86 @@ func verifyTables(ctx *sql.Context, unqualifiedTables []string, checkStaged bool
return nil, err
}
// If tables exist in both staged and head roots, we can remove them
// If it is not in the head, we either error if --cached was not used, or add it otherwise
// If neither is true it does not exist in staged, or does not exist in both.
// In this case there's nothing to remove, so we error.
if okHead && okStage {
TableNames = append(TableNames, tblName)
} else if okStage {
if checkStaged {
// Does the table have unstaged changes? If so, error out
hasChanges, err := hasUnstagedChanges(ctx, roots, name, okStage, okHead)
if err != nil {
return nil, err
}
if hasChanges {
unstagedTables = append(unstagedTables, name)
continue
}
// If the table exists in staged:
// If we use --cached, or it exists in HEAD, we can remove it safely
// Otherwise we error out.
// If the table does not exist in staged:
// If it is in HEAD we can remove it safely
// If it isn't in HEAD it doesn't exist, and so we error
if okStage {
if okHead || checkStaged {
TableNames = append(TableNames, tblName)
} else {
missingStaged = append(missingStaged, name)
missingStagedTables = append(missingStagedTables, name)
}
} else {
missingTables = append(missingTables, name)
if okHead {
TableNames = append(TableNames, tblName)
} else {
missingTables = append(missingTables, name)
}
}
}
if len(missingTables) > 0 {
return nil, actions.NewTblNotExistError(doltdb.ToTableNames(missingTables, doltdb.DefaultSchemaName))
} else if len(missingStaged) > 0 {
return nil, actions.NewTblStagedError(doltdb.ToTableNames(missingStaged, doltdb.DefaultSchemaName))
} else if len(unstagedTables) > 0 {
return nil, actions.NewTblUnstagedError(doltdb.ToTableNames(unstagedTables, doltdb.DefaultSchemaName))
} else if len(missingStagedTables) > 0 {
return nil, actions.NewTblStagedError(doltdb.ToTableNames(missingStagedTables, doltdb.DefaultSchemaName))
}
return TableNames, nil
}
func hasUnstagedChanges(ctx *sql.Context, roots doltdb.Roots, name string, okStaged bool, okHead bool) (bool, error) {
// Check diff between working and staged.
// We'll check this if the table exists in the staged root or if it doesn't exist in the HEAD root
if okStaged || !okHead {
tableDiff, err := diff.GetTableDeltas(ctx, roots.Staged, roots.Working)
if err != nil {
return false, err
}
for _, tbl := range tableDiff {
hasChanges, err := tbl.HasChanges()
if err != nil {
return false, err
}
if tbl.ToName.String() == name && hasChanges {
return true, nil
}
}
}
// Now check diff between working and HEAD
// We'll check this if the table exists in the HEAD root or if it doesn't exist in the staged root
if okHead || !okStaged {
tableDiff, err := diff.GetTableDeltas(ctx, roots.Head, roots.Working)
if err != nil {
return false, err
}
for _, tbl := range tableDiff {
hasChanges, err := tbl.HasChanges()
if err != nil {
return false, err
}
if tbl.ToName.String() == name && hasChanges {
return true, nil
}
}
}
return false, nil
}
@@ -2042,4 +2042,15 @@ func RunDoltRmTests(t *testing.T, h DoltEnginetestHarness) {
enginetest.TestScript(t, h, script)
}()
}
h = h.NewHarness(t)
defer h.Close()
engine, err := h.NewEngine(t)
require.NoError(t, err)
readOnlyEngine, err := h.NewReadOnlyEngine(engine.EngineAnalyzer().Catalog.DbProvider)
require.NoError(t, err)
for _, script := range DoltRmReadOnlyTests {
enginetest.TestScriptWithEngine(t, readOnlyEngine, h, script)
}
}
@@ -19,6 +19,18 @@ import (
"github.com/dolthub/go-mysql-server/sql"
)
var DoltRmReadOnlyTests = []queries.ScriptTest{
{
Name: "dolt Rm returns error for read-only database",
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('t')",
ExpectedErrStr: "unable to rm in read-only databases",
},
},
},
}
var DoltRmTests = []queries.ScriptTest{
{
Name: "dolt Rm without tables",
@@ -50,12 +62,12 @@ var DoltRmTests = []queries.ScriptTest{
Name: "dolt Rm staged table",
SetUpScript: []string{
"CREATE TABLE test (i int)",
"CALL DOLT_ADD('.')",
"CALL DOLT_ADD('test')",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('test');",
ExpectedErrStr: "error: the table(s) test have changes saved in the index.",
ExpectedErrStr: "error: the table(s) test have changes saved in the index. Use --cached or commit.",
},
},
},
@@ -67,7 +79,35 @@ var DoltRmTests = []queries.ScriptTest{
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('test');",
ExpectedErrStr: "error: the table(s) test do not exist",
ExpectedErrStr: "error: the table(s) test have unstaged changes.",
},
},
},
{
Name: "dolt Rm staged table with unstaged changes",
SetUpScript: []string{
"CREATE TABLE test (i int)",
"CALL DOLT_ADD('test')",
"INSERT INTO test VALUES (1)",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('test');",
ExpectedErrStr: "error: the table(s) test have unstaged changes.",
},
},
},
{
Name: "dolt Rm committed table with unstaged changes",
SetUpScript: []string{
"CREATE TABLE test (i int)",
"CALL DOLT_COMMIT('-A', '-m', 'created table')",
"INSERT INTO test VALUES (1);",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('test');",
ExpectedErrStr: "error: the table(s) test have unstaged changes.",
},
},
},
@@ -75,14 +115,16 @@ var DoltRmTests = []queries.ScriptTest{
Name: "dolt Rm with cached option",
SetUpScript: []string{
"CREATE TABLE test (i int)",
"INSERT INTO test VALUES (1)",
"CALL DOLT_COMMIT('-A', '-m', 'created table')",
"call dolt_rm('test', '--cached');",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "select * from test;",
Expected: []sql.Row{{1}},
Query: "select * from dolt_status;",
Expected: []sql.Row{
{"test", true, "deleted"},
{"test", false, "new table"},
},
},
},
},
@@ -90,7 +132,7 @@ var DoltRmTests = []queries.ScriptTest{
Name: "dolt Rm staged table with cached option",
SetUpScript: []string{
"CREATE TABLE test (i int)",
"CALL DOLT_ADD('.')",
"CALL DOLT_ADD('test')",
},
Assertions: []queries.ScriptTestAssertion{
{
@@ -111,7 +153,7 @@ var DoltRmTests = []queries.ScriptTest{
"CREATE TABLE committed (i int)",
"CALL DOLT_COMMIT('-A', '-m', 'created table')",
"CREATE TABLE staged (i int)",
"CALL DOLT_ADD('.')",
"CALL DOLT_ADD('staged')",
},
Assertions: []queries.ScriptTestAssertion{
{
@@ -128,8 +170,18 @@ var DoltRmTests = []queries.ScriptTest{
},
},
},
/*TODO:
ADD TESTS RELATED TO STUFF WHEN YOU SHOULDN'T BE ABLE TO REMOVE
FOR EXAMPLE, FOREIGN KEY CONSTRAINTS.
*/
{
Name: "dolt Rm errors on foreign key constrained table",
SetUpScript: []string{
"CREATE TABLE parent (pk int primary key, p1 int)",
"CREATE TABLE child (pk int primary key, c1 int, FOREIGN KEY (c1) REFERENCES parent (pk))",
"CALL DOLT_COMMIT('-A', '-m', 'created tables')",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_rm('parent');",
ExpectedErrStr: "unable to remove `parent` since it is referenced from table `child`",
},
},
},
}