Fix crash when sorting by a nullable field missing on some items

Sorting by a field whose declared type is nullable (its null value is
`None`, e.g. `NullInteger`/`NullFloat`) raised `TypeError: '<' not
supported between instances of ...` when the field was present on some
objects but missing on others, because `FieldSort.sort` compared `None`
against real values.

Group missing values together in the sort key so they are never compared
against present ones: they are ordered first when sorting ascending and
last when descending, matching SQLite's default ordering of NULLs used by
the fast `FixedFieldSort` path.

Fixes #3461.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
TowyTowy
2026-07-13 14:00:57 +02:00
committed by Šarūnas Nejus
parent 923675a0e6
commit 2293d2f60a
3 changed files with 42 additions and 4 deletions

View File

@@ -111,9 +111,8 @@ class FieldSort(Sort):
self.case_insensitive = case_insensitive
def sort(self, objs: list[AnyModel]) -> list[AnyModel]:
# TODO: Conversion and null-detection here. In Python 3,
# comparisons with None fail. We should also support flexible
# attributes with different types without falling over.
# TODO: Support flexible attributes with different types (e.g. a mix
# of strings and numbers) without falling over.
def key(obj: Model) -> Any:
field_val = obj.get(self.field, None)
@@ -126,7 +125,13 @@ class FieldSort(Sort):
field_val = ""
if self.case_insensitive and isinstance(field_val, str):
field_val = field_val.lower()
return field_val
# Nullable types (e.g. ``NullInteger``/``NullFloat``) use ``None``
# as their null value, so a field may be missing on some objects
# and present on others. Comparing ``None`` with a real value
# raises a ``TypeError``, so group all missing values together:
# this places them first when sorting ascending and last when
# descending, matching SQLite's default ordering of NULLs.
return (field_val is not None, field_val)
return sorted(objs, key=key, reverse=not self.ascending)

View File

@@ -55,6 +55,11 @@ Bug fixes
instrumental track, and store that state in ``lyrics_instrumental`` flexible
attribute. Existing ``[Instrumental]`` lyrics are migrated automatically.
:bug:`6719`
- Sorting by a nullable field (for example a flexible attribute with a declared
type whose null value is ``None``) that is present on only some items no
longer crashes with a ``TypeError``. Missing values are now grouped together,
ordered before present ones when sorting ascending and after them when
descending. :bug:`3461`
..
For plugin developers

View File

@@ -253,6 +253,34 @@ class TestNonExistingField:
assert ids_asc == [*null_values_ids, lower_item.id, higher_item.id]
assert ids_desc == [higher_item.id, lower_item.id, *null_values_ids]
@pytest.mark.parametrize(
"field_type,lower,higher",
[
_p(types.NullInteger(), 2, 10, id="null-integer"),
_p(types.NullFloat(), 2.5, 10.5, id="null-float"),
],
)
def test_nullable_field_present_in_some_items(
self, monkeypatch, field_type, lower, higher
):
"""Ordering by a nullable-typed field (whose null value is ``None``)
that is present on only some items should not raise. See #3461.
"""
monkeypatch.setitem(Item._types, "mynull", field_type)
lower_item, higher_item, *items_without_val = self.lib.items("id+")
for item, value in zip((lower_item, higher_item), (lower, higher)):
item.mynull = value
item.store()
null_values_ids = [i.id for i in items_without_val]
ids_asc = [i.id for i in self.lib.items("mynull+ id+")]
ids_desc = [i.id for i in self.lib.items("mynull- id+")]
assert ids_asc == [*null_values_ids, lower_item.id, higher_item.id]
assert ids_desc == [higher_item.id, lower_item.id, *null_values_ids]
def test_negation_interaction(self):
"""Test the handling of negation and sorting together.