Inconsistent detection of merge conflict, depending on direction of merge.
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 873
- Avg merge
- 1d 5h
- Merged PRs (30d)
- 108
Description
Potentially related to https://github.com/dolthub/dolt/issues/7762
Steps to reproduce:
Add the following test to `SchemaChangeTestsBasicCases` in `dolt_queries_schema_merge.go`
```
{
Name: "ambiguous choice of ancestor column",
AncSetUpScript: []string{
"CREATE table t (pk int primary key, col1 int, col2 int);",
"INSERT into t values (1, 10, 100), (2, 20, 200);",
},
RightSetUpScript: []string{
"alter table t drop column col1;",
"insert into t values (3, 30), (4, 40);",
},
LeftSetUpScript: []string{
"alter table t drop column col2;",
"alter table t rename column col1 to col2;",
"insert into t values (5, 50), (6, 60);",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_merge('right');",
ExpectedErrStr: "Merge conflict detected, @autocommit transaction rolled back. @autocommit must be disabled so that merge conflicts can be resolved using the dolt_conflicts and dolt_schema_conflicts tables before manually committing the transaction. Alternatively, to commit transactions with merge conflicts, set @@dolt_allow_commit_conflicts = 1",
},
},
},
```
In this test, both branches have a `col2` column, but these columns correspond to different columns in the ancestor schema. (In the implementation, we track this by giving each column a tag.) It's ambiguous which column in the ancestor should be used for the merge.
I don't know if we explicitly handle this case, and it's not obvious what the correct behavior should be. I think there's two reasonable arguments here:
- Since the column is named "col2" in both branches, the merge should use the column "col2" in the ancestor.
- This is ambiguous and should result in a merge conflict.
Regardless of the "correct" behavior, we're currently bugged. Dolt will do one or the other of these, depending on the merge direction.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.