dolt icon indicating copy to clipboard operation
dolt copied to clipboard

Branch may have incorrect root hash after merge if no changes are made to left side.

Open nicktobey opened this issue 8 months ago • 0 comments

This is a bit tricky to reproduce, unlikely to affect any real-world use case, and does not cause correctness issues. But it can influence table hashes, which means it can break tests which compare tables by hash. It's minimal priority, and this issue is mostly here to document a specific design decision in https://github.com/dolthub/dolt/pull/7191.

There seems to be a bug that occurs after a three-way merge if:

  • The left side of the merge does not make any changes to its primary index as a result of the merge.
  • The table is empty after the merge.

An example reproduction case would be a merge where the left side deleted every row in the ancestor, and the right side introduces a schema change.

In this case, because there are no changes the primary index, the index is not re-chunked and the previous root node from the MutableMap is reused. This root node is computed differently from the root node that would be generated by the chunker, specifically with regards to the address_array field on the ProllyTreeNode protobuf:

  • The root node on the MutableMap has address_array explicitly set to an empty array.
  • The root node generated by the chunker omits the address_array field.

These two nodes behave identically, but they have different hashes. As a result, any merge test that fulfills these requirements will fail with a very confusing error message claiming that the two tables are unequal despite neither containing any rows.

This was encountered during https://github.com/dolthub/dolt/pull/7191. It was triggered by an apparently benign change to three way merging that would only attempt to delete a key from a table if the key was already in the table. Attempting to delete the key if it wasn't in the table would be a no-op, but would force the merger to re-chunk the index, avoiding this behavior.

nicktobey avatar Dec 19 '23 00:12 nicktobey