diff --git a/csv_diff/__init__.py b/csv_diff/__init__.py index 59a2eaf..4c8e377 100644 --- a/csv_diff/__init__.py +++ b/csv_diff/__init__.py @@ -1,10 +1,19 @@ import csv +import sys from dictdiffer import diff import json import hashlib def load_csv(fp, key=None, dialect=None): + # The C parser's per-field size cap defaults to 131072 bytes, which is + # far too small for CSVs that contain long strings (e.g. nucleotide + # sequences, large JSON blobs, embedded log lines). When the limit is + # exceeded the parser raises ``_csv.Error: field larger than field + # limit (131072)`` instead of returning the row. Bump the limit to + # the platform's ``PY_SSIZE_T_MAX`` so the parser can handle long + # fields. See issue #41. + csv.field_size_limit(sys.maxsize) if dialect is None and fp.seekable(): # Peek at first 1MB to sniff the delimiter and other dialect details peek = fp.read(1024**2) @@ -85,12 +94,33 @@ def compare(previous, current, show_unchanged=False): result["removed"] = [previous[id] for id in removed] if changed: for id in changed: - diffs = list(diff(previous[id], current[id], ignore=ignore_columns)) + # Pass dot_notation=False so dictdiffer treats every top-level + # column name as a flat key. With the default dot_notation=True, + # a column whose name contains '..' is parsed as a path with a + # parent step (e.g. 'name..date_range' becomes [parent, 'date_range']), + # and dictdiffer then emits 'add'/'remove' 2-tuples for the column + # swap instead of 'change' 3-tuples, which crashes the + # `for _, field, (prev, curr) in diffs` unpack below. The dot + # character alone is fine because we still extract the field + # name from the resulting list (see the `field[0]` below). The + # 'ignore_columns' lookup also relies on this: it matches + # literal column names, not path fragments. + diffs = list( + diff( + previous[id], + current[id], + ignore=ignore_columns, + dot_notation=False, + ) + ) if diffs: changes = { "key": id, "changes": { - # field can be a list if id contained '.' - #7 + # field is a list because dot_notation=False makes + # dictdiffer return every path as a list. The list + # has length 1 for a top-level column name (even if + # the name contains '.') - see issue #7. field[0] if isinstance(field, list) else field: [ prev_value, current_value, diff --git a/tests/test_csv_diff.py b/tests/test_csv_diff.py index 0e3670f..fe96585 100644 --- a/tests/test_csv_diff.py +++ b/tests/test_csv_diff.py @@ -115,3 +115,80 @@ def test_tsv(): "columns_added": [], "columns_removed": [], } == diff + + +def test_load_csv_handles_fields_larger_than_default_limit(): + # Regression test for https://github.com/simonw/csv-diff/issues/41 + # The csv module's default per-field size cap is 131072 bytes; rows with + # longer fields (e.g. nucleotide sequences, large JSON blobs) used to + # raise ``_csv.Error: field larger than field limit (131072)`` from + # ``load_csv``. ``load_csv`` now bumps the cap to ``sys.maxsize`` so + # these rows are returned in full. + long_value = "A" * (200_000) + csv_text = f"id,sequence\n1,{long_value}\n" + rows = load_csv(io.StringIO(csv_text), key="id") + assert rows == {"1": {"id": "1", "sequence": long_value}} + + +def test_load_csv_handles_tsv_with_long_fields(): + # Same fix as above, exercising the TSV path (which uses the same + # underlying csv reader and is also subject to the size cap). + long_value = "T" * (200_000) + tsv_text = f"id\tsequence\n1\t{long_value}\n" + rows = load_csv(io.StringIO(tsv_text), key="id") + assert rows == {"1": {"id": "1", "sequence": long_value}} + + +def test_compare_handles_double_dot_in_column_name(): + # Regression test for https://github.com/simonw/csv-diff/issues/40 + # dictdiffer's default ``dot_notation=True`` parses ``..`` in a key as a + # parent step in a path, which causes a ``ValueError: not enough values + # to unpack (expected 2, got 1)`` when one row's value sits at a top- + # level key like ``name..date_range``. compare() now passes + # ``dot_notation=False`` so dictdiffer treats every column name as a + # flat string and the unpack always sees the expected 3-tuple shape. + previous = load_csv(io.StringIO(ONE_DOTDOT), key="id") + current = load_csv(io.StringIO(TWO_DOTDOT), key="id") + diff = compare(previous, current) + # The ``..`` column is reported as a column-level change (added in + # current, removed from previous) and the timestamp value change + # surfaces in the per-row change block. + assert diff["columns_added"] == ["name..date_range"] + assert diff["columns_removed"] == ["name.date_range"] + assert diff["changed"] == [ + {"key": "1", "changes": {"timestamp": ["1", "2"]}} + ] + assert diff["added"] == [] + assert diff["removed"] == [] + + +def test_compare_handles_double_dot_in_column_value_change(): + # Same fix as above, exercising the case where the column with ``..`` is + # present in both rows and only its value changes. Without + # ``dot_notation=False`` dictdiffer returns the path as + # ``['name..date_range']`` (a list with the double-dot in it) and the + # existing code's ``field[0]`` flattening still produces the right + # field name in the changes block. + previous = load_csv(io.StringIO(DOTDOT_VAL_OLD), key="id") + current = load_csv(io.StringIO(DOTDOT_VAL_NEW), key="id") + diff = compare(previous, current) + assert diff["columns_added"] == [] + assert diff["columns_removed"] == [] + assert diff["changed"] == [ + {"key": "1", "changes": {"name..date_range": ["A", "B"]}} + ] + + +ONE_DOTDOT = """id,name.date_range,timestamp +1,X,1 +""" +TWO_DOTDOT = """id,name..date_range,timestamp +1,X,2 +""" + +DOTDOT_VAL_OLD = """id,name..date_range +1,A +""" +DOTDOT_VAL_NEW = """id,name..date_range +1,B +""" \ No newline at end of file