Cosmetic parity nit, likely low priority — an all-zero row and no row arguably say the same thing. Filing mainly to record the characterization, and the trap in the obvious fix.
Pre-existing on master. Found while doing #1998.
Repro
CREATE TABLE t(id INT PRIMARY KEY, v INT);
INSERT INTO t VALUES(1,10),(2,20);
SELECT dolt_commit('-Am','seed');
ALTER TABLE t RENAME COLUMN v TO w;
SELECT dolt_commit('-Am','c2');
SELECT * FROM dolt_diff_stat('HEAD~1','HEAD');
doltlite returns one row, everything zero including the row counts:
t ru=0 ra=0 rd=0 rm=0 ca=0 cd=0 cm=0 orc=0 nrc=0
Dolt 2.2.2 returns no rows. Same on an empty table, and for ALTER TABLE t MODIFY COLUMN v VARCHAR(50).
Do not "fix" it by narrowing the emission gate
The tempting change is to drop the schemaChanged exception in dstAdvance (src/doltlite_diff_stat.c), so a table with no counters gets no row. I tried it. It makes this case pass and breaks diff_stat_revert_schema_change_with_later_added_table in test/vc_oracle_diff_stat_test.sh, where Dolt does emit an all-zero row.
What Dolt actually does, as far as I probed:
| schema-only change, no data change |
Dolt |
ALTER TABLE t RENAME COLUMN v TO w |
no row |
ALTER TABLE t MODIFY COLUMN v VARCHAR(50) |
no row |
recreate with a CHECK, DROP TABLE t, RENAME TO t |
all-zero row |
So the row appears when the table was recreated, not when its columns changed in place. Matching that needs table-identity reasoning rather than a counter test, which is why #1998 left it alone.
rename_column_no_data_change in test/vc_oracle_diff_stat_test.sh currently runs summary-only for this reason, with a comment pointing at the trap. If this is closed as wontfix, that case can stay as it is.
Cosmetic parity nit, likely low priority — an all-zero row and no row arguably say the same thing. Filing mainly to record the characterization, and the trap in the obvious fix.
Pre-existing on master. Found while doing #1998.
Repro
doltlite returns one row, everything zero including the row counts:
Dolt 2.2.2 returns no rows. Same on an empty table, and for
ALTER TABLE t MODIFY COLUMN v VARCHAR(50).Do not "fix" it by narrowing the emission gate
The tempting change is to drop the
schemaChangedexception indstAdvance(src/doltlite_diff_stat.c), so a table with no counters gets no row. I tried it. It makes this case pass and breaksdiff_stat_revert_schema_change_with_later_added_tableintest/vc_oracle_diff_stat_test.sh, where Dolt does emit an all-zero row.What Dolt actually does, as far as I probed:
ALTER TABLE t RENAME COLUMN v TO wALTER TABLE t MODIFY COLUMN v VARCHAR(50)DROP TABLE t,RENAME TO tSo the row appears when the table was recreated, not when its columns changed in place. Matching that needs table-identity reasoning rather than a counter test, which is why #1998 left it alone.
rename_column_no_data_changeintest/vc_oracle_diff_stat_test.shcurrently runs summary-only for this reason, with a comment pointing at the trap. If this is closed as wontfix, that case can stay as it is.