Autocommit blocks access to conflict resolution table

[VinaiRachakonda]

Autocommit (on by default) prevents transactions from executing when conflicts exists. This causes problems from select * from dolt_conflicts query which is needed to resolve said queries.

Repro below

create database test_db;
use test_db;

select dolt_checkout('main');
create table state_populations (state varchar(14) not null,population int,rate double,primary key (state));
insert into state_populations (state, population, rate) values ('Delaware', 59096, 0.98),('New Hampshire', 141885, 1.007);
select dolt_commit('-a', '-m', 'initial table for merge conflict path');
select dolt_checkout('-b', 'mybranch_2', 'main');
select dolt_checkout('-b', 'mybranch_1', 'main');

update state_populations set population = 200000 where state like 'New%';
select dolt_commit('-a', '-m', 'commit data to mybranch_1');
select dolt_checkout('main');
select dolt_merge('mybranch_1');
select dolt_commit('-m', 'dolt_commit merge mybranch_1');

select dolt_checkout('mybranch_2');

update state_populations set population = 300000 where state like 'New%';
select dolt_commit('-a', '-m', 'commit data to mybranch_2');
select dolt_checkout('main');
select dolt_merge('mybranch_2');
select * from dolt_conflicts;

[timsehn]

We need end to end tests like:

https://github.com/dolthub/dolt/blob/main/integration-tests/bats/merge.bats https://github.com/dolthub/dolt/blob/main/integration-tests/bats/conflict-detection.bats https://github.com/dolthub/dolt/blob/main/integration-tests/bats/conflict-detection-2.bats

But on the SQL path.

I'll start on that.

[timsehn]

Here's a more minimal repro:

create table state_populations (state varchar(14) not null,population int,rate \
double,primary key (state));
insert into state_populations (state, population, rate) values ('Delaware', 590\
96, 0.98),('New Hampshire', 141885, 1.007);
select dolt_commit('-a', '-m', 'initial table for merge conflict path');
select dolt_branch('mybranch');

update state_populations set population = 200000 where state like 'New%';
select dolt_commit('-a', '-m', 'commit data to main');

select dolt_checkout('mybranch');

update state_populations set population = 300000 where state like 'New%';
select dolt_commit('-a', '-m', 'commit data to mybranch_2');
select dolt_checkout('main');

Then run:

test-conflicts $ dolt sql -q "select dolt_merge('mybranch')" 
+------------------------+
| dolt_merge('mybranch') |
+------------------------+
| 0                      |
+------------------------+
merge has unresolved conflicts. please use the dolt_conflicts table to resolve
test-conflicts $ dolt sql -q "select * from dolt_conflicts" 
+-------+---------------+
| table | num_conflicts |
+-------+---------------+
+-------+---------------+
test-conflicts $ dolt sql -q "select * from dolt_conflicts_state_populations" 
+------------+-----------------+-----------+-----------+----------------+----------+-------------+------------------+------------+
| base_state | base_population | base_rate | our_state | our_population | our_rate | their_state | their_population | their_rate |
+------------+-----------------+-----------+-----------+----------------+----------+-------------+------------------+------------+
+------------+-----------------+-----------+-----------+----------------+----------+-------------+------------------+------------+
test-conflicts $ dolt status
On branch main
nothing to commit, working tree clean

So basically you can't dolt_merge() with autocommit on.

[timsehn]

Our bats tests are actually pretty good here. They just assume you shut autocommit off to do all the stuff.

[VinaiRachakonda]

When those tests were written they were when the Dolt SQL functions were just introduced. Then we didn't have a transactional model so we wrote them assuming autocommit needed to be off.

We definitely need to add tests that use similar functionality with autocommit on.

[timsehn]

There's a ton of complexity with the following:

1. autocommit on/off a. if off, open or closed transaction
2. batch mode on/off (ie. disable batch)

[VinaiRachakonda]

Noted!

[zachmu]

Isn't this fixed by new default behavior that lets you commit conflicts?

[dphil]

I am currently encountering what appears to be the same issue using dolt 0.40.0. I am running a nodejs dev app that connects to the dolt sql-server using knex with the mysql2 connector.

Running a merge as follows:

  await knex.raw(
    `Call dolt_merge('${fromBranch}', '-m', 'Merge branch ${fromBranch} into ${toBranch}')`
  );

For merges that don't produce conflicts, this works fine. But a merge that produces a conflict will throw an exception here with the error: Call dolt_merge('main', '-m', 'Merge branch main into feature') - merge has unresolved conflicts. please use the dolt_conflicts table to resolve

I'm assuming that's intended behaviour when there's a conflict. However, there is nothing in the conflicts tables (neither dolt_conflicts nor dolt_conflicts_$table), and so the conflict cannot be resolved. The dolt status doesn't appear to indicate anything outstanding either.

So currently I cannot process merges that produce conflicts.

[dphil]

From the above discussion, it sounds like this might be resolved by disabling autocommit. This presumably would bring some implications in how our code is structured - ie needing to convert everything to knex transactions or something like that.

Isn't this fixed by new default behavior that lets you commit conflicts?

Is there some new approach that involves committing conflicts after a merge? There doesn't seem to be anything I can commit after the merge.

[dphil]

I have tried disabling autocommit on the connection session and then wrapping the merge action in a knex transaction. The merge is occurring and producing the same message about a conflict, but again nothing in the conflict tables.

[timsehn]

Dave,

We've had a ton of discussion about this internally.

As far as I understand, the idea is that you need to create and resolve conflicts in the same transaction. So, what you're seeing is a bug.

@zachmu @druvv How is this supposed to work?

[timsehn]

PS C:\Users\timse> dolt sql-client
# Welcome to the Dolt MySQL client.
# Statements must be terminated with ';'.
# "exit" or "quit" (or Ctrl-D) to exit.
mysql> show databases;
+----------------------+
| Database             |
+----------------------+
| information_schema   |
| test_merge_conflicts |
+----------------------+
mysql> use test_merge_conflicts
    -> ;
mysql> create table test (pk int, c1 int, primary key (pk));
mysql> call dolt_commit('-a', '-m', 'created table');
mysql> show variables like 'autocommit';
+---------------+-------+
| Variable_name | Value |
+---------------+-------+
| autocommit    | 1     |
+---------------+-------+
mysql> call dolt_checkout('-b', 'other');
+--------+
| status |
+--------+
| 0      |
+--------+
mysql> insert into test values (1,1);
mysql> call dolt_commit('-a', '-m', 'insert (1,1)');
+----------------------------------+
| hash                             |
+----------------------------------+
| 078r2g3otvh5hd1asi2fude9s5hsoe4s |
+----------------------------------+
mysql> call dolt_checkout('main');
+--------+
| status |
+--------+
| 0      |
+--------+
mysql> insert into test values (1,0);
mysql> call dolt_commit('-a', '-m', 'insert (1,0)');
+----------------------------------+
| hash                             |
+----------------------------------+
| e5s3qh4ebkksq4ukq5onsn9ne61stpt9 |
+----------------------------------+
mysql> select * from dolt_log;
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
| commit_hash                      | committer | email           | date                    | message                    |
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
| e5s3qh4ebkksq4ukq5onsn9ne61stpt9 | Tim Sehn  | [email protected] | 2022-05-14 15:21:31.584 | insert (1,0)               |
| 1j063idbl8o4cnkqd7apka2fkotk6ppe | Tim Sehn  | [email protected] | 2022-05-14 15:17:22.2   | created table              |
| k5citlk3p4hjihcnbto3gd6qceqt1tog | Tim Sehn  | [email protected] | 2022-05-14 15:15:08.108 | Initialize data repository |
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
mysql> call dolt_merge('other');
Error 1105: merge has unresolved conflicts. please use the dolt_conflicts table to resolve
mysql> select * from dolt_conflcits;
Error 1146: table not found: dolt_conflcits
mysql> set autocommit=0;
mysql> call dolt_merge('other');
+--------------+
| no_conflicts |
+--------------+
| 0            |
+--------------+
mysql> select * from test;
+----+----+
| pk | c1 |
+----+----+
| 1  | 0  |
+----+----+
mysql> select * from dolt_log;
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
| commit_hash                      | committer | email           | date                    | message                    |
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
| e5s3qh4ebkksq4ukq5onsn9ne61stpt9 | Tim Sehn  | [email protected] | 2022-05-14 15:21:31.584 | insert (1,0)               |
| 1j063idbl8o4cnkqd7apka2fkotk6ppe | Tim Sehn  | [email protected] | 2022-05-14 15:17:22.2   | created table              |
| k5citlk3p4hjihcnbto3gd6qceqt1tog | Tim Sehn  | [email protected] | 2022-05-14 15:15:08.108 | Initialize data repository |
+----------------------------------+-----------+-----------------+-------------------------+----------------------------+
mysql> call dolt_merge('other');
Error 1105: merge has unresolved conflicts. please use the dolt_conflicts table to resolve
mysql> show variables like 'autocommit';
+---------------+-------+
| Variable_name | Value |
+---------------+-------+
| autocommit    | 0     |
+---------------+-------+
mysql> select * from dolt_conflicts;
+-------+---------------+
| table | num_conflicts |
+-------+---------------+
| test  | 1             |
+-------+---------------+
mysql> select * from dolt_conflicts_test;
+---------+---------+--------+--------+----------+----------+
| base_pk | base_c1 | our_pk | our_c1 | their_pk | their_c1 |
+---------+---------+--------+--------+----------+----------+
| NULL    | NULL    | 1      | 0      | 1        | 1        |
+---------+---------+--------+--------+----------+----------+
mysql> delete * from dolt_conflcits;
Error 1105: syntax error at position 9 near 'delete'
mysql> delete from dolt_conflicts;
Error 1105: table doesn't support DELETE FROM
mysql> delete from dolt_conflicts_test;
mysql> COMMIT;
mysql> call dolt_commit('-a', '-m', 'merged and resolved conflicts');
+----------------------------------+
| hash                             |
+----------------------------------+
| t9dicg9aa3h2h96tvfi2sag6j7jeg0ui |
+----------------------------------+
mysql> select * from dolt_log
    -> ;
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+
| commit_hash                      | committer | email           | date                    | message                       |
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+
| t9dicg9aa3h2h96tvfi2sag6j7jeg0ui | Tim Sehn  | [email protected] | 2022-05-14 15:26:38.257 | merged and resolved conflicts |
| e5s3qh4ebkksq4ukq5onsn9ne61stpt9 | Tim Sehn  | [email protected] | 2022-05-14 15:21:31.584 | insert (1,0)                  |
| 078r2g3otvh5hd1asi2fude9s5hsoe4s | Tim Sehn  | [email protected] | 2022-05-14 15:21:01.458 | insert (1,1)                  |
| 1j063idbl8o4cnkqd7apka2fkotk6ppe | Tim Sehn  | [email protected] | 2022-05-14 15:17:22.2   | created table                 |
| k5citlk3p4hjihcnbto3gd6qceqt1tog | Tim Sehn  | [email protected] | 2022-05-14 15:15:08.108 | Initialize data repository    |
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+
mysql>

[timsehn]

That's me doing this and seeing how it worked using autocommit off.

So, one bug is that the merge returns:

mysql> call dolt_merge('other');
+--------------+
| no_conflicts |
+--------------+
| 0            |
+--------------+

which is a confusing double negative.

But the flow works. I'll see what happens with the autocommit on case.

[zachmu]

If you want to use auto commit for merges, you need to set a session variable that allows you to commit transactions with merge conflicts. By default this behavior is disabled, but you can enable it with a session variable. Docs here:

https://docs.dolthub.com/reference/sql/branches#commiting-with-merge-conflicts

[timsehn]

mysql> set autocommit=1;
mysql> show variables like 'autocommit';
+---------------+-------+
| Variable_name | Value |
+---------------+-------+
| autocommit    | 1     |
+---------------+-------+
mysql> call dolt_checkout('other');
+--------+
| status |
+--------+
| 0      |
+--------+
mysql> insert into test values (2,1);
mysql> call dolt_commit('-a', '-m', 'inserted (2,1)');
+----------------------------------+
| hash                             |
+----------------------------------+
| pjfrafd1cmc078k76bheih7c003a3cmc |
+----------------------------------+
mysql> call dolt_checkout('main');
+--------+
| status |
+--------+
| 0      |
+--------+
mysql> insert into test values (2,0);
mysql> call dolt_commit('-a', '-m', 'inserted (2,0)');
+----------------------------------+
| hash                             |
+----------------------------------+
| g3i5l58scasaomn6u2i3enod0hpmoc4m |
+----------------------------------+
mysql> call dolt_merge('other');
Error 1105: merge has unresolved conflicts. please use the dolt_conflicts table to resolve
mysql> select * from dolt_conflicts;
mysql> select * from dolt_conflicts_test;
mysql> select * from dolt_log;
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+
| commit_hash                      | committer | email           | date                    | message                       |
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+
| g3i5l58scasaomn6u2i3enod0hpmoc4m | Tim Sehn  | [email protected] | 2022-05-14 15:33:53.038 | inserted (2,0)                |
| t9dicg9aa3h2h96tvfi2sag6j7jeg0ui | Tim Sehn  | [email protected] | 2022-05-14 15:26:38.257 | merged and resolved conflicts |
| e5s3qh4ebkksq4ukq5onsn9ne61stpt9 | Tim Sehn  | [email protected] | 2022-05-14 15:21:31.584 | insert (1,0)                  |
| 078r2g3otvh5hd1asi2fude9s5hsoe4s | Tim Sehn  | [email protected] | 2022-05-14 15:21:01.458 | insert (1,1)                  |
| 1j063idbl8o4cnkqd7apka2fkotk6ppe | Tim Sehn  | [email protected] | 2022-05-14 15:17:22.2   | created table                 |
| k5citlk3p4hjihcnbto3gd6qceqt1tog | Tim Sehn  | [email protected] | 2022-05-14 15:15:08.108 | Initialize data repository    |
+----------------------------------+-----------+-----------------+-------------------------+-------------------------------+

We have to at least change the error message to something like:

Merge conflict with autocommit on. Aborting merge. To resolve conflicts with dolt_merge, do so in a single transaction. To SQL commit conflicts set @@dolt_allow_commit_conflicts.

[zachmu]

The fact that there are no rows in the dolt_conflicts table after the merge makes me think that the session variable was not actually set, and that the transaction actually got rolled back when it attempted to commit and discovered merge conflicts. I'm not familiar with the ORM you are using, but that's my guess as to what's happening. To be extra sure of this, you can set the system variable globally using the global keyword, and then every new session will get that value when it begins.

[dphil]

Wrapping this up, with insights provided above I have been able to resolve this by:

1. disabling autocommit prior to doing the merge
2. start a transaction
3. perform the merge
4. check dolt_conflicts table for conflicts
5. resolve conflicts per guidance here: https://docs.dolthub.com/reference/sql/branches#resolving-conflicts
6. commit changes (Call dolt_commit(...))
7. close transaction
8. re-enable autocommit

All done by issuing SQL via knex from a node application, in case anyone is interested.

Thanks @zachmu / @timsehn .

[timsehn]

We're still going to fix the error message. Here's the PR:

https://github.com/dolthub/dolt/pull/3413