ALTER TABLE ADD COLUMN ... DEFAULT stores NULL on rows whose dropped trailing column was NULL
- Dominant language
- C
- Stars
- 268
- Forks
- 18
- Avg merge
- 2h 27m
- Merged PRs (30d)
- 447
Description
Found in a full-repo review at `0ba280f06f`.
`ALTER TABLE ... ADD COLUMN ... DEFAULT ` reads back NULL instead of the default, on exactly those rows whose previously-dropped **trailing** column was NULL. The wrong value is durable: it commits, survives a merge, and `PRAGMA integrity_check` reports `ok`.
```sql
CREATE TABLE t(k TEXT PRIMARY KEY, a INT, x TEXT);
INSERT INTO t(k,a) VALUES('n1',1); -- x left NULL
ALTER TABLE t DROP COLUMN x;
ALTER TABLE t ADD COLUMN y INT DEFAULT 7;
SELECT y FROM t;
-- EXPECTED (stock 3.54.0): 7
-- GOT: NULL
```
`UPDATE t SET a=a`, `UPDATE t SET y=y`, `VACUUM` and a `dolt_commit` round trip all leave it NULL. Only `REINDEX` or a PK-rewriting update repairs it.
The `NOT NULL` form does not merely return the wrong value, it refuses a legal statement:
```sql
ALTER TABLE t ADD COLUMN y INT NOT NULL DEFAULT 0;
-- EXPECTED: column added, y = 0
-- GOT: Error: constraint failed (column not added)
```
Two rows of the same table disagree, which is what makes it easy to miss:
```sql
CREATE TABLE t(k TEXT PRIMARY KEY, a INT, x TEXT);
INSERT INTO t(k,a) VALUES('n1',1),('n2',2);
UPDATE t SET x='v' WHERE k='n2';
SELECT dolt_commit('-A','-m','base');
ALTER TABLE t DROP COLUMN x; SELECT dolt_commit('-A','-m','drop x');
ALTER TABLE t ADD COLUMN y INT DEFAULT 7; SELECT dolt_commit('-A','-m','add y');
SELECT group_concat(k||'='||coalesce(y,'NULL'),' ') FROM t;
-- GOT: n1=NULL n2=7
-- PRAGMA integrity_check -> ok
-- dolt_at_t at HEAD agrees with the table, and the value survives a subsequent merge.
```
Scope, measured against stock as a control:
| case | result |
| --- | --- |
| clustered TEXT PK, trailing col NULL | **diverges** |
| rowid INTEGER PK, trailing col NULL | **diverges** |
| keyless table, trailing col NULL | **diverges** |
| `NOT NULL DEFAULT` after the drop | **refused** |
| `TEXT DEFAULT 'dflt'` after the drop | **diverges** (NULL) |
| second `ADD COLUMN` after the poisoned one | correct |
| dropped column was not trailing | correct |
| dropped column held a value | correct |
| no `DROP COLUMN` at all | correct |
## Cause
`src/prolly_btree_mutation.c:831-833` (and the sortkey path at `:948-951`) substitute the **old** record bytes whenever `prollyValuesEqual()` reports the new record equal:
```c
rc = prollyValuesEqual(pOld, nOld, pData, nData, &equal);
if( rc!=SQLITE_OK ) return rc;
if( equal ){
pData = pOld;
nData = nOld;
}
```
`diffRecordsEqualFieldwise` (`src/prolly_diff.c:185-187`) pads a *missing* trailing field with serial type 0, i.e. NULL:
```c
nField = aInfo.nField > bInfo.nField ? aInfo.nField : bInfo.nField;
for(i=0; i
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with src/prolly_btree_mutation.c at the record-preservation short-circuits around lines 831-833 and 948-951, then read diffRecordsEqualFieldwise in src/prolly_diff.c. Use the reported DROP COLUMN and ADD COLUMN SQL reproduction as a regression case. Done means the default is returned after the trailing NULL column is dropped while the #2611 implicit-NULL case remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, sqlite
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100