dolt_diff and dolt_diff_summary cannot tell a table rename from a drop plus a create
- 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