mirror of
https://github.com/simonw/sqlite-utils.git
synced 2026-10-24 18:27:01 +02:00
analyze_column(): handle null values in the common-value analysis
Three ways a column containing nulls produced a wrong answer, all from
nulls not being accounted for.
1. The "single distinct value" shortcut. count(distinct col) skips
nulls in SQL, so a column holding nulls plus one other value reports
num_distinct == 1 and took the shortcut, which reports that one value
with a count of total_rows:
insert [{'c': None}, {'c': None}, {'c': 'a'}, {'c': 'a'}]
analyze_column('c').most_common # was [(None, 4)]
select c, count(*) group by c # is [(None, 2), ('a', 2)]
The shortcut now requires that the column has no nulls, so those
columns fall through to the group by like any other.
2. The tie-break sort compared the values directly, so a null and a
string with the same count raised TypeError:
analyze_column('c', common_limit=5)
# TypeError: '<' not supported between instances of 'NoneType' and 'str'
sort_key() keeps nulls out of the comparison and sorts them last,
which keeps the existing ordering of non-null ties - the test for
'Terryterryterry' ahead of 'Kumar' at equal counts still passes.
3. truncate() turned a null into the string 'None', so a null was
indistinguishable from a column really holding 'None'. That is the
path analyze-tables takes, since it passes value_truncate=80.
num_distinct itself is unchanged: it still follows SQL and reports 0 for
an all-null column, which test_analyze_table_column_all_nulls pins. But
num_distinct can therefore be lower than len(most_common) when a column
holds nulls alongside all-distinct values - {'c': None, 'a', 'b'} gives
num_distinct 2 and three most_common entries. Whether null should count
as a distinct value is a question about what the number means, so I have
left it alone rather than change an output the tests already pin.
Tests: 6 new cases in tests/test_analyze.py, all failing on 6bc1d33.
Suite 1506 passed, 16 skipped. mypy, pyright, flake8, ty, black and
cog --check clean.
This commit is contained in:
parent
6bc1d33d58
commit
8e5dfdd409
2 changed files with 77 additions and 3 deletions
|
|
@ -5323,6 +5323,8 @@ class Table(Queryable):
|
|||
total_rows = db[table].count
|
||||
|
||||
def truncate(value):
|
||||
if value is None:
|
||||
return None
|
||||
if value_truncate is None or isinstance(value, (float, int)):
|
||||
return value
|
||||
value = str(value)
|
||||
|
|
@ -5330,6 +5332,13 @@ class Table(Queryable):
|
|||
value = value[:value_truncate] + "..."
|
||||
return value
|
||||
|
||||
def sort_key(pair):
|
||||
# Counts are the primary key. Ties are broken on the value
|
||||
# itself, so keep nulls out of the comparison - None cannot be
|
||||
# ordered against a string or a number - and sort them last.
|
||||
value = pair[0]
|
||||
return (pair[1], value is not None, "" if value is None else value)
|
||||
|
||||
table_quoted = quote_identifier(table)
|
||||
column_quoted = quote_identifier(column)
|
||||
num_null = db.execute(
|
||||
|
|
@ -5343,7 +5352,8 @@ class Table(Queryable):
|
|||
).fetchone()[0]
|
||||
most_common_results = None
|
||||
least_common_results = None
|
||||
if num_distinct == 1:
|
||||
if num_distinct == 1 and not num_null:
|
||||
# Every row holds that one value, so skip the group by
|
||||
value = db.execute(
|
||||
f"select {column_quoted} from {table_quoted} limit 1"
|
||||
).fetchone()[0]
|
||||
|
|
@ -5363,7 +5373,7 @@ class Table(Queryable):
|
|||
f"limit {common_limit}"
|
||||
).fetchall()
|
||||
]
|
||||
most_common_results.sort(key=lambda p: (p[1], p[0]), reverse=True)
|
||||
most_common_results.sort(key=sort_key, reverse=True)
|
||||
if least_common:
|
||||
if num_distinct <= common_limit:
|
||||
# No need to run the query if it will just return the results in reverse order
|
||||
|
|
@ -5378,7 +5388,7 @@ class Table(Queryable):
|
|||
f"limit {common_limit}"
|
||||
).fetchall()
|
||||
]
|
||||
least_common_results.sort(key=lambda p: (p[1], p[0]))
|
||||
least_common_results.sort(key=sort_key)
|
||||
return ColumnDetails(
|
||||
self.name,
|
||||
column,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue