Skip to content

Commit 89fe8bd

Browse files
JOhnsonKC201claude
JOhnsonKC201
and
claude
committed
extract(): keep all-NULL rows unlinked even with a pre-existing NULL lookup row
Follow-up to the #186 fix: skipping the all-NULL combination in the lookup INSERT is not enough on its own. When extract() reuses a lookup table that already contains an all-NULL row (e.g. one written by an older sqlite-utils version or created manually), the IS-based foreign-key UPDATE would still match that row and link all-NULL source rows to it. The UPDATE now carries a trailing `WHERE NOT (<col> IS NULL AND ...)` so all-NULL source rows are never assigned a foreign key and keep the NULL that the freshly-added column starts with, regardless of the lookup table's existing contents. Adds test_extract_all_null_stays_null_with_preexisting_null_lookup_row. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 parent 2ba6eb1 commit 89fe8bd

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

‎sqlite_utils/db.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,12 +2210,15 @@ def extract(
22102210
# Now add the new fk_column
22112211
self.add_column(magic_lookup_column, int)
22122212

2213-
# And populate it
2213+
# And populate it. The trailing ``WHERE NOT (<all columns NULL>)`` leaves
2214+
# all-NULL source rows with a NULL foreign key even when the lookup table
2215+
# already contains an all-NULL row (e.g. a reused or legacy lookup table)
2216+
# — the IS-based join would otherwise match that row and link to it (#186).
22142217
self.db.execute(
2215-
"UPDATE {} SET {} = (SELECT id FROM {} WHERE {where})".format(
2216-
quote_identifier(self.name),
2217-
quote_identifier(magic_lookup_column),
2218-
quote_identifier(table),
2218+
"UPDATE {table_name} SET {fk} = (SELECT id FROM {lookup} WHERE {where}) WHERE NOT ({all_null})".format(
2219+
table_name=quote_identifier(self.name),
2220+
fk=quote_identifier(magic_lookup_column),
2221+
lookup=quote_identifier(table),
22192222
where=" AND ".join(
22202223
"{}.{} IS {}.{}".format(
22212224
quote_identifier(self.name),
@@ -2225,6 +2228,12 @@ def extract(
22252228
)
22262229
for column in columns
22272230
),
2231+
all_null=" AND ".join(
2232+
"{}.{} IS NULL".format(
2233+
quote_identifier(self.name), quote_identifier(column)
2234+
)
2235+
for column in columns
2236+
),
22282237
)
22292238
)
22302239
# Figure out the right column order

‎tests/test_extract.py‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,25 @@ def test_extract_multi_column_keeps_partial_null_but_not_all_null(fresh_db):
236236
assert rows[0]["ab_id"] == rows[1]["ab_id"] is not None # partial NULL shared
237237
assert rows[3]["ab_id"] not in (None, rows[0]["ab_id"])
238238
# The lookup table must not contain an all-NULL row.
239-
assert not any(
240-
row["a"] is None and row["b"] is None for row in fresh_db["ab"].rows
239+
assert not any(row["a"] is None and row["b"] is None for row in fresh_db["ab"].rows)
240+
241+
242+
def test_extract_all_null_stays_null_with_preexisting_null_lookup_row(fresh_db):
243+
# Reusing a lookup table that already contains an all-NULL row (e.g. created
244+
# by an older sqlite-utils version) must still leave all-NULL source rows with
245+
# a NULL foreign key — the IS-based join must not link them to that row (#186).
246+
fresh_db["type"].insert_all(
247+
[{"id": 1, "type": None}, {"id": 2, "type": "dog"}], pk="id"
248+
)
249+
fresh_db["creatures"].insert_all(
250+
[
251+
{"id": 1, "name": "Simon", "type": None},
252+
{"id": 2, "name": "Cleo", "type": "dog"},
253+
],
254+
pk="id",
241255
)
256+
fresh_db["creatures"].extract("type")
257+
assert list(fresh_db["creatures"].rows) == [
258+
{"id": 1, "name": "Simon", "type_id": None},
259+
{"id": 2, "name": "Cleo", "type_id": 2},
260+
]

0 commit comments

Comments
 (0)