Skip to content
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

Inconsistent detection of merge conflict, depending on direction of merge. #7767

Open
nicktobey opened this issue Apr 21, 2024 · 0 comments
Open
Labels
bug Something isn't working merge Issues relating to merge

Comments

@nicktobey
Copy link
Contributor

Potentially related to #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working merge Issues relating to merge
Projects
None yet
Development

No branches or pull requests

2 participants