Repository navigation
Conversation
Seed CSVs were parsed with pandas type inference and the declared `columns` types were only applied afterwards, so information lost during inference could not be recovered: TEXT values like 00123 were loaded as '123' (or '123.0' when the column had an empty value), and integer columns with empty values were read as floats, losing precision above 2**53. When columns are declared, the CSV is still read as before, and declared columns that inference got wrong are restored from a second read of the raw strings: TEXT columns that weren't read as strings keep the values as written, and integer columns read as floats are converted to exact Python ints (left as is if a value isn't an integer literal). Columns pandas already read correctly are untouched, so other seeds keep their column hashes. Signed-off-by: Jack A Ross <jack.ross@gmatter.co>
A TEXT column of TRUE/FALSE values that also has blanks is read by pandas as an object column of Python bools, which the previous check mistook for raw strings, so the values were rendered as 'True'/'False'. Restore a declared TEXT column unless every value is already a string. Signed-off-by: Jack A Ross <jack.ross@gmatter.co>
Vamsi-klu
left a comment
There was a problem hiding this comment.
The leading-zero and integer-precision fixes are covered well. Validation at 4459cc18 passed 37 seed/model tests and 9 dbt seed tests, and the new end-to-end seed test fails against the base as expected.
One compatibility regression needs fixing before merge: restoring declared TEXT columns can turn a configured numeric NA value back into a non-NULL string. With na_values=[1], 1.00 changes from SQL NULL on the base to '1.00' on this branch. The inline comment includes the reproduction. Please preserve the original null mask and cover numeric NA markers in a regression test.
Both current-head workflows also report action_required with no jobs executed, so CI still needs to run.
|
|
||
| raw_df = self._read_csv(dtype=str) | ||
| for name in text_columns: | ||
| df[name] = raw_df[name] |
There was a problem hiding this comment.
Could we preserve the original Series’ null mask when restoring a TEXT column here? With CsvSettings(na_values=[1]) and CSV code,id\n1.00,10\n2,20\n,30\n, declaring code TEXT, id INT produces (NULL, 10) on the base, but this branch produces ('1.00', 10). I reproduced that through rendered SQL executed in DuckDB. The dtype=str read handles numeric NA matches differently, so assigning it back revives a configured missing value. Please retain the original null positions during restoration and add a regression for numeric NA markers, including their string form.
Description
Fixes #6135.
When a
SEEDmodel declarescolumns, its CSV was still parsed with pandas type inference, and the declared types were applied only afterwards inSeedModel.render_seed. That cast can't undo what inference already lost:TEXTvalues like00123were loaded as'123', or'123.0'when the column also had an empty value.Fix
CsvSeedReadernow receives the model's declaredcolumns_to_types. It still reads the CSV exactly as before, then repairs only the declared columns that inference got wrong:ints andNone. If any value isn't an integer literal (1.5,1e3), the column keeps the floats pandas inferred, exactly as today.The raw strings come from a second
read_csv(dtype=str)with the samecsv_settings. It runs only when at least one declared column needs repair.I deliberately avoided passing
dtypehints such as"Int64"toread_csv. In pandas 2.3,dtype="Int64"silently wraps values in [2^63, 2^64):9223372036854775808becomes-9223372036854775808. When the column also has a blank, the same values raiseOverflowError. Reading as before and repairing afterwards means no CSV that loads today stops loading or changes, unless it hit this bug.Behaviour changes
truestaystrue(previouslyTrue) and1e3stays1e3(previously1000.0). Empty values and NA markers (na_values,keep_default_na) still load asNULL.NULLinstead of floats.column_types: {zip: varchar}override now keeps00501. Previously it was loaded as'501', while dbt itself keeps'00501'.int, so integer columns with blanks now load as integers instead of floats.double) are unaffected.Upgrade note
Seed fingerprints include per-column hashes of the parsed data. Seeds that actually hit this bug get new column hashes, so they appear as modified once after upgrading and are reloaded with the corrected values. This covers declared TEXT columns pandas had read as numbers, and declared integer columns with blanks, including dbt seeds whose agate-inferred integer columns have blanks. Other seeds keep identical hashes;
test_column_hashes_only_change_for_seeds_that_were_read_incorrectlycovers both sides.Declared TEXT columns with numeric-looking values and no leading zeros (e.g.
1, 2, 3) also change hash, even though their loaded values are the same. I couldn't find a cheap way to avoid this without keeping the old, incorrect parse around just for hashing.The docs for seed models (
docs/concepts/models/seed_models.md) gain a short paragraph explaining that declaredcolumnscontrol how CSV values are read.Test Plan
New tests only; no existing assertions were changed.
tests/core/test_seed.py, reader level:9007199254740993stays exact.csv_settings(delimiter,na_values) are honoured by the re-read.tests/core/test_model.py::test_seed_model_declared_types_are_applied_when_reading_csv:render_seedoutput for TEXT, INT, DOUBLE, BOOLEAN and DATE columns.VALUESrows, e.g.('00123', 1, 1.5, TRUE, CAST('2026-01-02' AS DATE)).I also reproduced the issue end to end in a DuckDB project before and after the change. Other engines receive the same DataFrame, and I did not test them against live warehouses.
Commands (Python 3.12, pandas 2.3.3):
make style: ruff, ruff-format, mypy and valid-migrations all pass.make fast-test: 2675 passed, 6 skipped. Isolated groups: 5 passed, 1 passed and 190 passed.pytest tests/dbt -k seed: 9 passed.pytest tests/core/test_seed.py tests/core/test_model.py tests/core/test_snapshot.py: 503 passed.Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO