-
-
Notifications
You must be signed in to change notification settings - Fork 559
Support the dolt_conflicts_resolve
workflow when a table is dropped on one side of a merge.
#9203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…e other, record this as a schema conflict.
@nicktobey DOLT
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should focus on making dolt_conflicts_resolve
work before worrying about the dolt conflicts
CLI command. Honestly we should migrate or deprecate the cli command.
At present This is the merge error I get:
db/dev*> CALL DOLT_CONFLICTS_RESOLVE("--ours", "TABLEFOO");
Unable to automatically resolve schema conflicts since data changes may not have been fully merged yet. To continue, abort this merge (dolt merge --abort) then apply ALTER TABLE statements to one side of this merge to get the two schemas in sync with the desired schema, then rerun the merge. To track resolution of this limitation, follow https://github.com/dolthub/dolt/issues/6616
@@ -751,7 +751,19 @@ SQL | |||
run dolt merge feature-branch | |||
|
|||
log_status_eq 1 | |||
[[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false | |||
[[ "$output" =~ "CONFLICT (schema): Merge conflict in test" ]] || false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matching all the lines like this seems pretty frail. Would it make more sense to build these into the go engine tests?
Alright, I modified Unfortunately, the conflict resolution workflow seems to be broken for this case: Assuming that the branch containing the data edit is named When checking out
|
…nflicts because one side of the merge was deleted.
f271d38
to
21094c5
Compare
dolt_conflicts_resolve
workflow when a table is dropped on one side of a merge.
@nicktobey DOLT
|
The original behavior (reporting dropped tables as schema conflicts) was moved to its own PR: #9208
This PR contains the follow-up work to allow
dolt conflicts resolve
to function in the presence of table drops. See below for examples where this doesn't currently work as expected.Currently, attempting to merge when a table has been drop on one branch and had its data modified on the other produces an immediate error that aborts the merge. This error does not necessarily have enough context to determine the affected table, which makes resolving the merge difficult.Given that we already handle a simultaneous drop + schema modification (by treating it as a schema conflict), we should be able to handle the simpler case of data modifications as well.Since dropping a table is technically a schema change, this should now get reported as a schema conflict, and pause the merge, allowing the user to querydolt_schema_conflicts
for more information.I'm not thrilled about the behavioral change observed in the BATS test "merge: ourRoot renames, theirRoot modifies the schema".What's happening there is that a renamed table is seen as a drop + a create. So the merge code sees a create with no conflicts, and attempts to add it to the same root as the original table (which is now in a conflict state.) Since both tables contain duplicate tags, this causes an error. This probably isn't what we want to happen, and we should figure out the intended behavior before merging this PR.