mirror of
https://github.com/simonw/sqlite-utils.git
synced 2026-09-17 22:14:09 +02:00
Don't extract NULL values into a lookup row (#186)
`table.extract()` built its lookup table with
`INSERT OR IGNORE ... SELECT DISTINCT <cols> FROM <table>`, which included
the all-NULL combination. That created a spurious lookup row for NULL and
pointed every NULL source row at it, instead of leaving those rows with a
NULL foreign key.
Before:
db["creatures"].extract("type")
# type lookup: [{"id": 1, "type": None}, {"id": 2, "type": "dog"}]
# creatures: Simon -> type_id=1, Natalie -> type_id=1, Cleo -> type_id=2
After:
# type lookup: [{"id": 1, "type": "dog"}]
# creatures: Simon -> type_id=None, Natalie -> type_id=None, Cleo -> type_id=1
A row whose extracted columns are entirely NULL represents "no value", so it
now keeps a NULL foreign key and no lookup row is created for it. The fix adds
a `WHERE NOT (<col> IS NULL AND ...)` guard to the lookup INSERT; the existing
`IS`-based foreign-key UPDATE then leaves those rows NULL automatically (the
subquery finds no matching lookup row).
For multi-column extracts, only the fully-NULL combination is skipped — a
partial-NULL combination (some extracted columns set, others NULL) is a
genuine distinct value and is still extracted and shared between matching rows.
Updates test_extract_works_with_null_values to assert the corrected behaviour
and adds regression tests for the single-column and multi-column cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
401fb6949c
commit
dad463e9c2
2 changed files with 55 additions and 3 deletions
|
|
@ -2267,14 +2267,22 @@ class Table(Queryable):
|
|||
)
|
||||
lookup_columns = [(rename.get(col) or col) for col in columns]
|
||||
lookup_table.create_index(lookup_columns, unique=True, if_not_exists=True)
|
||||
# Don't create a lookup row for the all-NULL combination: a row whose
|
||||
# extracted columns are entirely NULL represents "no value", so it
|
||||
# should keep a NULL foreign key rather than point at a NULL lookup row
|
||||
# (#186). Rows with a partial NULL (some extracted columns set, others
|
||||
# NULL) are a genuine distinct value and are still extracted.
|
||||
self.db.execute(
|
||||
"INSERT OR IGNORE INTO {} ({lookup_columns}) SELECT DISTINCT {table_cols} FROM {}".format(
|
||||
"INSERT OR IGNORE INTO {} ({lookup_columns}) SELECT DISTINCT {table_cols} FROM {} WHERE NOT ({all_null})".format(
|
||||
quote_identifier(table),
|
||||
quote_identifier(self.name),
|
||||
lookup_columns=", ".join(
|
||||
quote_identifier(c) for c in lookup_columns
|
||||
),
|
||||
table_cols=", ".join(quote_identifier(c) for c in columns),
|
||||
all_null=" AND ".join(
|
||||
"{} IS NULL".format(quote_identifier(c)) for c in columns
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -177,6 +177,8 @@ def test_extract_error_on_incompatible_existing_lookup_table(fresh_db):
|
|||
|
||||
|
||||
def test_extract_works_with_null_values(fresh_db):
|
||||
# A NULL extracted value represents "no value", so it should keep a NULL
|
||||
# foreign key rather than be turned into a lookup row of its own (#186).
|
||||
fresh_db["listens"].insert_all(
|
||||
[
|
||||
{"id": 1, "track_title": "foo", "album_title": "bar"},
|
||||
|
|
@ -189,9 +191,51 @@ def test_extract_works_with_null_values(fresh_db):
|
|||
)
|
||||
assert list(fresh_db["listens"].rows) == [
|
||||
{"id": 1, "track_title": "foo", "album_id": 1},
|
||||
{"id": 2, "track_title": "baz", "album_id": 2},
|
||||
{"id": 2, "track_title": "baz", "album_id": None},
|
||||
]
|
||||
assert list(fresh_db["albums"].rows) == [
|
||||
{"id": 1, "album_title": "bar"},
|
||||
{"id": 2, "album_title": None},
|
||||
]
|
||||
|
||||
|
||||
def test_extract_does_not_create_lookup_row_for_all_null(fresh_db):
|
||||
# Single-column: every NULL keeps a NULL fk and no NULL lookup row is made.
|
||||
fresh_db["creatures"].insert_all(
|
||||
[
|
||||
{"id": 1, "name": "Simon", "type": None},
|
||||
{"id": 2, "name": "Natalie", "type": None},
|
||||
{"id": 3, "name": "Cleo", "type": "dog"},
|
||||
],
|
||||
pk="id",
|
||||
)
|
||||
fresh_db["creatures"].extract("type")
|
||||
assert list(fresh_db["creatures"].rows) == [
|
||||
{"id": 1, "name": "Simon", "type_id": None},
|
||||
{"id": 2, "name": "Natalie", "type_id": None},
|
||||
{"id": 3, "name": "Cleo", "type_id": 1},
|
||||
]
|
||||
assert list(fresh_db["type"].rows) == [{"id": 1, "type": "dog"}]
|
||||
|
||||
|
||||
def test_extract_multi_column_keeps_partial_null_but_not_all_null(fresh_db):
|
||||
# Multi-column: a row whose extracted columns are *all* NULL keeps a NULL
|
||||
# fk, but a partial-NULL combination is a genuine distinct value and is
|
||||
# still extracted (and shared between matching rows) (#186).
|
||||
fresh_db["t"].insert_all(
|
||||
[
|
||||
{"id": 1, "a": "x", "b": None},
|
||||
{"id": 2, "a": "x", "b": None},
|
||||
{"id": 3, "a": None, "b": None},
|
||||
{"id": 4, "a": "y", "b": "z"},
|
||||
],
|
||||
pk="id",
|
||||
)
|
||||
fresh_db["t"].extract(["a", "b"], table="ab", fk_column="ab_id")
|
||||
rows = list(fresh_db["t"].rows)
|
||||
assert rows[2]["ab_id"] is None # all-NULL row -> NULL fk
|
||||
assert rows[0]["ab_id"] == rows[1]["ab_id"] is not None # partial NULL shared
|
||||
assert rows[3]["ab_id"] not in (None, rows[0]["ab_id"])
|
||||
# The lookup table must not contain an all-NULL row.
|
||||
assert not any(
|
||||
row["a"] is None and row["b"] is None for row in fresh_db["ab"].rows
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue