dolthub / dolthub/doltlite

dolt_diff and dolt_diff_summary cannot tell a table rename from a drop plus a create

Open
#2,016 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
268
Forks
18
Avg merge
2h 27m
Merged PRs (30d)
447

Description

`dolt_diff_summary` calls a drop+create a rename, and `dolt_diff` splits a rename into a drop plus an add. Both are the same root cause: the catalog has no table identity that survives a rename, so neither surface can tell the two apart.

The `dolt_diff_summary` half is the one that matters — it is merged and returns a wrong answer today, not a cosmetic difference.

Pre-existing on master (`069a59e7d6`). Found while looking at the `dolt_diff` rename pairing. Related: #1999, which is the same trap on `dolt_diff_stat`.

## Repro 1 — `dolt_diff_summary` reports a rename that never happened

```sql
CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT);
INSERT INTO t VALUES(1,'a');
SELECT dolt_commit('-A','-m','c1');
DROP TABLE t;
CREATE TABLE u(id INTEGER PRIMARY KEY, w TEXT); -- different table, different columns
INSERT INTO u VALUES(5,'z');
SELECT dolt_commit('-A','-m','drop_add');

SELECT * FROM dolt_diff_summary('','');
```

| | result |
|---|---|
| doltlite | `t, u, renamed, 1, 1` |
| Dolt 2.2.2 | `t, "", dropped, true, true`
`"", u, added, true, true` |

`t` was dropped and an unrelated `u` was created. doltlite claims one was renamed into the other.

## Repro 2 — `dolt_diff` splits a rename into two rows

```sql
CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT);
INSERT INTO t VALUES(1,'a');
SELECT dolt_commit('-A','-m','c1');
ALTER TABLE t RENAME TO u;
INSERT INTO u VALUES(2,'b');
SELECT dolt_commit('-A','-m','rename_and_insert');

SELECT message, data_change, schema_change, table_name FROM dolt_diff;
```

| | rows for the rename commit |
|---|---|
| doltlite | `rename_and_insert, true, true, u`
`rename_and_insert, true, true, t` |
| Dolt 2.2.2 | `rename_and_insert, true, true, u` |

## Root cause

`struct TableEntry` (`src/doltlite_catalog_types.h`) carries only `iTable` and `zName`. Neither is an identity:

- `zName` changes on rename, by definition.
- `iTable` is **canonical by sorted table name**, assigned positionally at serialization (`src/prolly_btree_catalog.c:1104`). Creating `zebra, apple, mango` yields `apple=2, mango=3, zebra=4`. Renaming a table changes its number, and can change other tables' numbers too.

So after a commit, "renamed `t` to `u`" and "dropped `t`, created `u`" leave **identical** catalog state. No amount of comparing the two catalogs can separate them. Dolt distinguishes them because it stores real table identity.

That numbering is not an accident to be fixed. It is what delivers the history-independence contract, quoting `prolly_btree_catalog.c:1104`:

> Canonical numbering: rows sort by type rank then name, and numbering is positional, so the blob is a pure function of the logical catalog (same schema reached through any DDL order hashes identically -- the history-independence contract).

Confirmed live: `CREATE t; INSERT; ALTER TABLE t RENAME TO u` and `CREATE u; INSERT` both give `dolt_hashof_table('u')` = `c250395be252b3f7ef4f459887290fd9f229c2ec`.

## Do not "fix" it by pairing on `iTable`

`dolt_diff_summary` already does this (`dsRenamePartner`, `src/doltlite_diff_stat.c:704`) and that is exactly why Repro 1 is wrong. Porting the same helper into `dolt_diff` fixes the rename cases and silently breaks drops. Measured, against Dolt:

| scenario | master | with `iTable` pairing | Dolt |
|---|---|---|---|
| pure rename | 2 rows | **1 row** ✔ | 1 row |
| rename + insert | 2 rows | **1 row** ✔ | 1 row |
| rename `t`→`u` + drop unrelated `d` | `d`, `t`, `u` | **`t`, `u`** ✘ lost the `d` drop | `d`, `u` |
| drop `t` + create `u` | `t`, `u` | **`u`** ✘ lost the `t` drop | `t`, `u` |

Both regressions make a dropped table disappear from the diff, which is worse than the extra row being removed. A "pair only when exactly one name is unmatched on each side" guard does not help either: Repro 1 has exactly one unmatched name per side and Dolt still calls it drop+add.

## What an actual fix costs

Two options, both real changes:

1. **Stable table identity in the catalog.** Persist a per-table id assigned at creation that survives rename. Fixes all three surfaces at the source. Abandons the history-independence contract by construction (the catalog would have to remember the table was once `t`), changes every catalog and table hash, and needs a review of the merge/diff code that assumes canonical numbering.
2. **Record the rename in the commit rather than the catalog.** Commits are already history, so a rename mapping there costs nothing in history-independence and does not rehash existing databases. Requires capturing renames in the session/working set as the DDL runs, plus an additive commit-format change.

Neither is worth a format change purely for rename attribution in diff output, which is why this is filed rather than fixed.

## Also found while investigating — unrelated, want their own issues if picked up

Both verified against a pre-change baseline binary, so neither is caused by the work above.

- **Table swap.** `ALTER TABLE t RENAME TO tmp; ALTER TABLE u RENAME TO t; ALTER TABLE tmp RENAME TO u;` — `dolt_diff` gives `schema_change=false` for both tables, Dolt gives `true`.
- **Empty table creation.** Creating a table and committing without inserting rows gives `data_change=true` in `dolt_diff`, Dolt gives `false`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with struct TableEntry in src/doltlite_catalog_types.h, canonical numbering in src/prolly_btree_catalog.c:1104, and rename pairing in src/doltlite_diff_stat.c:704. Run both SQL reproducers against the baseline and compare dolt_diff_summary and dolt_diff with Dolt; done requires a reviewed approach that distinguishes renames from drop-plus-create without losing unrelated drops.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, sqlite
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.