Table.analyze_column() gives a wrong answer for columns containing nulls, in three separate ways. All three are in sqlite_utils/db.py, and all three come from nulls not being accounted for.
1. The "single distinct value" shortcut reports a count that is too high
count(distinct col) skips nulls in SQL. So a column holding nulls plus one other value reports num_distinct == 1 and takes the shortcut that skips the group by:
if num_distinct == 1:
value = db.execute(f"select {column_quoted} from {table_quoted} limit 1").fetchone()[0]
most_common_results = [(truncate(value), total_rows)]
>>> db["t"].insert_all([{"c": None}, {"c": None}, {"c": "a"}, {"c": "a"}])
>>> db["t"].analyze_column("c").most_common
[(None, 4)]
>>> db.execute("select c, count(*) from t group by c").fetchall()
[(None, 2), ('a', 2)]
One value, counted four times, with the two nulls not mentioned at all. The shortcut is only valid when every row holds that one value.
This is reachable from the CLI too — sqlite-utils analyze-tables passes value_truncate=80, and truncate() then renders the null as the string "None", so the output reads 4: None for a column that is half null.
2. A null tied on count with a string raises TypeError
Both sorts break ties on the value itself, and None cannot be ordered against a string:
>>> db["t"].insert_all([{"c": None}, {"c": "a"}] + [{"c": c} for c in "bcdefg" for _ in range(2)])
>>> db["t"].analyze_column("c", common_limit=5).least_common
TypeError: '<' not supported between instances of 'NoneType' and 'str'
most_common has the same shape. Note that problem 1 currently hides this for some inputs, since the shortcut skips the sort — so fixing the shortcut alone would turn a wrong answer into a crash for some of them.
3. A null renders as the string "None"
>>> db["t"].insert_all([{"c": None}, {"c": None}, {"c": "None"}])
>>> db["t"].analyze_column("c", total_rows=3, value_truncate=80).most_common
[('None', 2), ('None', 1)]
Two entries, both spelled "None", and the reader cannot tell which was a null. With --save that string is persisted into _analyze_tables_.most_common too.
A question I did not want to answer on my own
num_distinct itself follows SQL, so an all-null column reports 0 — test_analyze_table_column_all_nulls pins that. But that leaves num_distinct able to disagree with the list it accompanies:
>>> db["t"].insert_all([{"c": None}, {"c": "a"}, {"c": "b"}])
>>> d = db["t"].analyze_column("c")
>>> d.num_distinct, len(d.most_common)
(2, 3)
Counting null as a distinct value fixes that and is a one-line change, but it changes what Distinct values: means for every column containing nulls, and whether a null should count is a question about the number's meaning rather than a defect. I would rather you decided that.
What I propose
Three changes, all confined to analyze_column():
- require no nulls for the single-distinct-value shortcut, so those columns fall through to the group by
- a
sort_key() that keeps nulls out of the tie-break comparison and sorts them last, preserving the current ordering of non-null ties (test_analyze_tables.py asserts "Terryterryterry" before "Kumar" at equal counts)
truncate() returns None unchanged instead of str()-ing it
Plus six cases in tests/test_analyze.py, all failing on 6bc1d33, and one that pins the current num_distinct semantics so the question above is documented either way.
Full suite and all six CI gates (mypy, pyright, flake8, ty, black --check, cog --check) pass with those changes applied.
I have this ready to send as a PR — say the word and I will open it, or if you would rather reshape it first, especially on the num_distinct question, tell me which way you want it and I will rework before sending anything.
Table.analyze_column()gives a wrong answer for columns containing nulls, in three separate ways. All three are insqlite_utils/db.py, and all three come from nulls not being accounted for.1. The "single distinct value" shortcut reports a count that is too high
count(distinct col)skips nulls in SQL. So a column holding nulls plus one other value reportsnum_distinct == 1and takes the shortcut that skips the group by:One value, counted four times, with the two nulls not mentioned at all. The shortcut is only valid when every row holds that one value.
This is reachable from the CLI too —
sqlite-utils analyze-tablespassesvalue_truncate=80, andtruncate()then renders the null as the string"None", so the output reads4: Nonefor a column that is half null.2. A null tied on count with a string raises
TypeErrorBoth sorts break ties on the value itself, and
Nonecannot be ordered against a string:most_commonhas the same shape. Note that problem 1 currently hides this for some inputs, since the shortcut skips the sort — so fixing the shortcut alone would turn a wrong answer into a crash for some of them.3. A null renders as the string
"None"Two entries, both spelled
"None", and the reader cannot tell which was a null. With--savethat string is persisted into_analyze_tables_.most_commontoo.A question I did not want to answer on my own
num_distinctitself follows SQL, so an all-null column reports0—test_analyze_table_column_all_nullspins that. But that leavesnum_distinctable to disagree with the list it accompanies:Counting null as a distinct value fixes that and is a one-line change, but it changes what
Distinct values:means for every column containing nulls, and whether a null should count is a question about the number's meaning rather than a defect. I would rather you decided that.What I propose
Three changes, all confined to
analyze_column():sort_key()that keeps nulls out of the tie-break comparison and sorts them last, preserving the current ordering of non-null ties (test_analyze_tables.pyasserts"Terryterryterry"before"Kumar"at equal counts)truncate()returnsNoneunchanged instead ofstr()-ing itPlus six cases in
tests/test_analyze.py, all failing on6bc1d33, and one that pins the currentnum_distinctsemantics so the question above is documented either way.Full suite and all six CI gates (
mypy,pyright,flake8,ty,black --check,cog --check) pass with those changes applied.I have this ready to send as a PR — say the word and I will open it, or if you would rather reshape it first, especially on the
num_distinctquestion, tell me which way you want it and I will rework before sending anything.