Messaging on a merge where a table was altered and deleted on each side of the merge not helpful

[timsehn]

Repro:

$ mkdir test_schema_merge
$ cd test_schema_merge 
$ dolt init
Successfully initialized dolt data repository.
$ dolt sql -q "create table a (pk int)"
$ dolt sql -q "create table b (pk int)"
$ dolt add .
$ dolt commit -am "Created tables"
commit 8l8pc9p9dieqh4vmaa57g7geagt8omba (HEAD -> main) 
Author: timsehn <[email protected]>
Date:  Mon Jan 22 09:01:18 -0800 2024

        Created tables

$ dolt branch br
$ dolt sql -q "alter table a add column c1 int" 
$ dolt commit -am "alter table a"
commit vv0upq531ii5mcbnpb5drjcmstevtqtn (HEAD -> main) 
Author: timsehn <[email protected]>
Date:  Mon Jan 22 09:02:53 -0800 2024

        alter table a

$ dolt checkout br               
Switched to branch 'br'
$ dolt sql -q "drop table a"
$ dolt commit -am "drop table a" 
commit bcnus5grga0caqmiug85c8l3o2grtsko (HEAD -> br) 
Author: timsehn <[email protected]>
Date:  Mon Jan 22 09:03:17 -0800 2024

        drop table a

$ dolt checkout main 
Switched to branch 'main'
$ dolt merge br                 
conflict: table with same name deleted and modified 
$ dolt sql -q "select * from dolt_schema_conflicts"

$

We should add the conflicting table names(s) to the conflict: table with same name deleted and modified and make sure the conflicting tables show up in the dolt_schema_conflicts table. In the dolt_schema_conflicts table have the side of the branch where it's deleted say NULL and have the schema of the altered one on the other side.

[nicktobey]

It looks like in this specific case (simultaneous modification and deletion of a table), the code doesn't consider it a conflict, it just considers it an error and aborts the merge immediately. There's no reason why we can't surface this as a conflict instead, but there are some implementation details to consider (conflict data gets stored on the table, which maintains the schema from ours... but how do we represent that if ours is the one being deleted?)

One thing to note is that if we want to be consistent with how we treat other types of conflicts, this should be a data conflict, not a schema conflict. Schema conflicts are for situations where both sides have schema changes that can't be resolved. Schema conflicts do not depend on data changes at all.

However, given that the customer assumed that this should have been a schema conflict (a not unreasonable assumption), we might want to either reconsider how we draw that distinction, or at least how we communicate about it.

[timsehn]

At least change the error message to list the tables that generated the error. Something like:

Error. Merge aborted. Table a was deleted and modified on either side of the merge.

I do think this makes most sense as a schema conflict even if technically the data is actually in conflict. The resolution is you either delete the table or keep the table with the new schema. You should not have to resolve every row of a data conflict.

[nicktobey]

You're right, this is absolutely a schema conflict. I misread the reproduction case and assumed this was a conflict between table being dropped with table rows being modified. My mistake.

I created https://github.com/dolthub/dolt/pull/7390. The output now looks like this:

Auto-merging a
CONFLICT (schema): Merge conflict in a
Automatic merge failed; 1 table(s) are unmerged.
Use 'dolt conflicts' to investigate and resolve conflicts.
nick@Nicks-MBP conflict % dolt conflicts cat .
+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+--------------------------------------------------------+
| our_schema                                                        | their_schema | base_schema                                                       | description                                            |
+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+--------------------------------------------------------+
| CREATE TABLE `a` (                                                | <deleted>    | CREATE TABLE `a` (                                                | cannot merge a table deletion with schema modification |
|   `pk` int,                                                       |              |   `pk` int                                                        |                                                        |
|   `c1` int                                                        |              | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; |                                                        |
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; |              |                                                                   |                                                        |
+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+--------------------------------------------------------+

[timsehn]

This is resolved by #7390