From 8f0d7f51e8eaa1e3dba9153065c2cb6422829b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:44:02 +0200 Subject: [PATCH 1/8] feat(InstanceSort): validate direction, add is_index_aligned and _apply_postgres_defaults_or_maybe_warn --- .../data_classes/data_modeling/instances.py | 51 +++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index 8bf5e35c94..09e4b17989 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -30,7 +30,7 @@ overload, ) -from typing_extensions import Self +from typing_extensions import Self, override from cognite.client._constants import OMITTED, Omitted from cognite.client.data_classes._base import ( @@ -1346,11 +1346,56 @@ class InstancesApply: class InstanceSort(DataModelingSort): def __init__( self, - property: list[str] | tuple[str, ...], + property: list[str] | tuple[str, str] | tuple[str, str, str], direction: Literal["ascending", "descending"] = "ascending", nulls_first: bool | None = None, ) -> None: - super().__init__(property, direction, nulls_first) + normalized = direction.casefold() + if normalized not in ("ascending", "descending"): + raise ValueError(f"direction must be 'ascending' or 'descending', got {direction!r}") + + super().__init__(property, normalized, nulls_first) + + # We override _load to get the more strict __init__ validation on 'direction' because we need it to + # be valid for the possible later automatic choice of nulls_first: + @override + @classmethod + def _load(cls, resource: dict[str, Any]) -> Self: + if not isinstance(resource, dict): + raise TypeError(f"Resource must be mapping, not {type(resource)}") + + return cls( + property=resource["property"], + direction=resource.get("direction", "ascending"), + nulls_first=resource.get("nullsFirst"), + ) + + @property + def is_index_aligned(self) -> bool: + """True when nulls_first matches the direction for PostgreSQL index utilization (None counts as aligned).""" + if self.nulls_first is None: + return True + return self.nulls_first is (self.direction == "descending") + + def _apply_postgres_defaults_or_maybe_warn(self) -> None: + """Resolve nulls_first for PostgreSQL index alignment, warning if the explicit value is misaligned. + + When nulls_first is None, sets it to the index-compatible value. When explicitly set but + misaligned, emits a UserWarning. + """ + if self.nulls_first is None: + self.nulls_first = self.direction == "descending" + + elif not self.is_index_aligned: + import warnings + + warnings.warn( + f"InstanceSort: direction={self.direction!r} with nulls_first={self.nulls_first} is not " + f"index-aligned and will likely prevent database index utilization. " + f"Use nulls_first={self.direction == 'descending'} (or omit it) for optimal performance.", + UserWarning, + stacklevel=3, + ) @dataclass From 0e39b76e71e65edb4a6b746a63b671a4d76d0f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:44:06 +0200 Subject: [PATCH 2/8] feat(query): add _iter_sorts traversal and _prepare_sorts to Query(Sync) class hierarchy --- .../data_classes/data_modeling/query.py | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/cognite/client/data_classes/data_modeling/query.py b/cognite/client/data_classes/data_modeling/query.py index 627564c19c..ad391b5956 100644 --- a/cognite/client/data_classes/data_modeling/query.py +++ b/cognite/client/data_classes/data_modeling/query.py @@ -2,7 +2,7 @@ from abc import ABC from collections import UserDict -from collections.abc import Mapping, MutableMapping, Sequence +from collections.abc import Iterator, Mapping, MutableMapping, Sequence from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar @@ -85,6 +85,9 @@ def _load_list( class SelectBase(CogniteResource, ABC): sources: list[SourceSelector] = field(default_factory=list) + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from () + def dump(self, camel_case: bool = True) -> dict[str, Any]: output: dict[str, Any] = {} if self.sources: @@ -133,6 +136,9 @@ class Select(SelectBase): sort: list[InstanceSort] = field(default_factory=list) limit: int | None = None + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from self.sort + def dump(self, camel_case: bool = True) -> dict[str, Any]: output = super().dump(camel_case) if self.sort: @@ -179,6 +185,16 @@ def instance_type_by_result_expression(self) -> dict[str, type[NodeListWithCurso for k, v in self.with_.items() } + def _iter_sorts(self) -> Iterator[InstanceSort]: + for expr in self.with_.values(): + yield from expr._iter_sorts() + for sel in self.select.values(): + yield from sel._iter_sorts() + + def _prepare_sorts(self) -> None: + for sort in self._iter_sorts(): + sort._apply_postgres_defaults_or_maybe_warn() + def dump(self, camel_case: bool = True) -> dict[str, Any]: output: dict[str, Any] = { "with": {k: v.dump(camel_case) for k, v in self.with_.items()}, @@ -281,6 +297,9 @@ class ResultSetExpressionBase(CogniteResource, ABC): def _load_sort(resource: dict[str, Any], name: str) -> list[InstanceSort]: return [InstanceSort.load(sort) for sort in resource.get(name, [])] + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from () + @staticmethod def _init_through(through: list[str] | tuple[str, str, str] | PropertyId | None) -> PropertyId | None: def error() -> Never: @@ -336,6 +355,9 @@ def __eq__(self, other: object) -> bool: return NotImplemented return type(self) is type(other) and self.dump() == other.dump() + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from self.sort + @dataclass(eq=False) # Prevents @dataclass from generating its own __eq__, so the parent's is used class NodeResultSetExpression(NodeOrEdgeResultSetExpression): @@ -429,6 +451,10 @@ class EdgeResultSetExpression(NodeOrEdgeResultSetExpression): limit_each: int | None = None post_sort: list[InstanceSort] = field(default_factory=list) + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from self.sort + yield from self.post_sort + @classmethod def _load(cls, resource: dict[str, Any]) -> Self: query_edge = resource["edges"] @@ -494,6 +520,9 @@ def __eq__(self, other: object) -> bool: return NotImplemented return type(self) is type(other) and self.dump() == other.dump() + def _iter_sorts(self) -> Iterator[InstanceSort]: + yield from self.backfill_sort + @classmethod def _load(cls, resource: dict[str, Any]) -> ResultSetExpressionSync: if "nodes" in resource: From b395b024ea2357f6e920a6efcd1b089258bf7d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:44:11 +0200 Subject: [PATCH 3/8] feat(Instances API): wire sort preparation at PostgreSQL-backed call sites --- cognite/client/_api/data_modeling/instances.py | 12 ++++++++---- cognite/client/_sync_api/data_modeling/instances.py | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cognite/client/_api/data_modeling/instances.py b/cognite/client/_api/data_modeling/instances.py index 4b01a2c928..163652f648 100644 --- a/cognite/client/_api/data_modeling/instances.py +++ b/cognite/client/_api/data_modeling/instances.py @@ -894,6 +894,7 @@ async def subscribe( >>> subscription_context.cancel() """ + query._prepare_sorts() subscription_context = SubscriptionContext() async def _poll_loop() -> None: @@ -968,10 +969,11 @@ def _create_other_params( f"Received in `sources` argument for views: {with_properties}." ) if sort: - if isinstance(sort, (InstanceSort, dict)): - other_params["sort"] = [cls._dump_instance_sort(sort)] - else: - other_params["sort"] = [cls._dump_instance_sort(s) for s in sort] + sorts_seq = [sort] if isinstance(sort, (InstanceSort, dict)) else list(sort) + for s in sorts_seq: + if isinstance(s, InstanceSort): + s._apply_postgres_defaults_or_maybe_warn() + other_params["sort"] = [cls._dump_instance_sort(s) for s in sorts_seq] if instance_type: other_params["instanceType"] = instance_type if debug: @@ -1647,6 +1649,7 @@ async def query( >>> res = client.data_modeling.instances.query(query, debug=debug_params) >>> print(res.debug) """ + query._prepare_sorts() return await self._query_or_sync(query, "query", include_typing=include_typing, debug=debug) async def sync( @@ -1749,6 +1752,7 @@ async def sync( >>> res = client.data_modeling.instances.sync(query, debug=debug_params) >>> print(res.debug) """ + query._prepare_sorts() return await self._query_or_sync(query, "sync", include_typing=include_typing, debug=debug) async def _query_or_sync( diff --git a/cognite/client/_sync_api/data_modeling/instances.py b/cognite/client/_sync_api/data_modeling/instances.py index 138df8b755..2d660424c2 100644 --- a/cognite/client/_sync_api/data_modeling/instances.py +++ b/cognite/client/_sync_api/data_modeling/instances.py @@ -1,6 +1,6 @@ """ =============================================================================== -c0af4f83cd8ffa0aaed6fa641bde9a98 +f57e38da23620654d634c19560399f41 This file is auto-generated from the Async API modules, - do not edit manually! =============================================================================== """ From 08bfda9074e54391bc57dee6a4030c988b73a322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:44:35 +0200 Subject: [PATCH 4/8] docs(InstanceSort): add docstring with examples and index-alignment tip --- .../data_classes/data_modeling/instances.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index 09e4b17989..f21495ae4b 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -1344,6 +1344,59 @@ class InstancesApply: class InstanceSort(DataModelingSort): + """Sort order for an instance query. + + Args: + property (list[str] | tuple[str, str] | tuple[str, str, str]): The property to sort by, given as a path, e.g. + ``("mySpace", "myView/v1", "myProperty")`` or ``["node", "externalId"]``. + direction (Literal['ascending', 'descending']): Sort direction. Case-insensitive. Defaults to ``"ascending"``. + nulls_first (bool | None): Where to place ``null`` values. Defaults to ``None`` (auto). See tip below. + + Tip: + For the backend database to use an index when sorting nullable properties, the ``nulls_first`` setting + must match the sort direction: + + - ``ascending`` → nulls last (``nulls_first=False``) + - ``descending`` → nulls first (``nulls_first=True``) + + When ``nulls_first=None`` (the default), the correct value is chosen automatically. Passing the + opposite combination is still accepted and sent to the API as-is, but may trigger a warning + for API endpoints that support index utilization if an unsupported combination is used. + + Examples: + + Sort by a view property ascending (default): + + >>> from cognite.client.data_classes.data_modeling import InstanceSort + >>> sort = InstanceSort(("mySpace", "myView/v1", "myProperty")) + + Can also use a ViewId to simplify the property path: + + >>> from cognite.client.data_classes.data_modeling import ViewId + >>> view_id = ViewId("mySpace", "myView", "v1") + >>> sort = InstanceSort(view_id.as_property_ref("myProperty")) + + Sort descending: + + >>> sort = InstanceSort( + ... view_id.as_property_ref("myProperty"), + ... direction="descending", + ... ) + + Sort by a base property: + + >>> sort = InstanceSort(["node", "externalId"], direction="ascending") + + Force a specific null placement (first/last). A UserWarning will fire at relevant API call + sites when this conflicts with index alignment: + + >>> sort = InstanceSort( + ... ("mySpace", "myView/v1", "myProperty"), + ... direction="descending", + ... nulls_first=True, + ... ) + """ + def __init__( self, property: list[str] | tuple[str, str] | tuple[str, str, str], From 12103eaa7c78e13991f23dd0fecc8eae8db679ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:44:42 +0200 Subject: [PATCH 5/8] test(data_modeling): add TestInstanceSort and TestQueryIterSorts --- .../test_data_models/test_instances.py | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py index d23269566a..773250e9f1 100644 --- a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py +++ b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from datetime import date, datetime from typing import Any, cast @@ -12,6 +13,7 @@ EdgeApply, EdgeList, Float64, + InstanceSort, Node, NodeApply, NodeId, @@ -638,3 +640,145 @@ def test_to_pandas(self) -> None: df = info.to_pandas() pd.testing.assert_frame_equal(df, expected) + + +class TestInstanceSort: + def test_nulls_first_stays_none_when_omitted(self) -> None: + sort = InstanceSort(["node", "externalId"], direction="ascending") + assert sort.nulls_first is None + sort = InstanceSort(["node", "externalId"], direction="descending") + assert sort.nulls_first is None + + def test_no_warning_at_construction_for_any_combination(self) -> None: + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + InstanceSort(["node", "externalId"], direction="ascending") + InstanceSort(["node", "externalId"], direction="descending") + InstanceSort(["node", "externalId"], direction="ascending", nulls_first=False) + InstanceSort(["node", "externalId"], direction="ascending", nulls_first=True) + InstanceSort(["node", "externalId"], direction="descending", nulls_first=True) + InstanceSort(["node", "externalId"], direction="descending", nulls_first=False) + assert len(w) == 0 + + @pytest.mark.parametrize( + "direction, nulls_first", + [ + ("ascending", True), + ("descending", False), + ], + ) + def test_non_index_aligned_nulls_first_is_stored_as_given(self, direction: str, nulls_first: bool) -> None: + sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] + assert sort.nulls_first is nulls_first + + def test_load_does_not_override_nulls_first(self) -> None: + raw = {"property": ["node", "externalId"], "direction": "ascending", "nullsFirst": True} + sort = InstanceSort._load(raw) + assert sort.direction == "ascending" + assert sort.nulls_first is True + + def test_dump_omits_nulls_first_when_none(self) -> None: + sort = InstanceSort(["space", "externalId"], direction="descending") + dumped = sort.dump(camel_case=True) + assert dumped["direction"] == "descending" + assert "nullsFirst" not in dumped + + @pytest.mark.parametrize( + "direction, nulls_first, expected", + [ + ("ascending", False, True), + ("descending", True, True), + ("ascending", True, False), + ("descending", False, False), + ("ascending", None, True), + ("descending", None, True), + ], + ) + def test_is_index_aligned(self, direction: str, nulls_first: bool | None, expected: bool) -> None: + sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] + assert sort.is_index_aligned is expected + + def test__apply_postgres_defaults_or_maybe_warn_resolves_none(self) -> None: + sort = InstanceSort(["node", "externalId"], direction="ascending") + sort._apply_postgres_defaults_or_maybe_warn() + assert sort.nulls_first is False + sort = InstanceSort(["node", "externalId"], direction="descending") + sort._apply_postgres_defaults_or_maybe_warn() + assert sort.nulls_first is True + + @pytest.mark.parametrize("bad_direction", ["asc", "desc", "random"]) + def test_invalid_direction_raises(self, bad_direction: str) -> None: + with pytest.raises(ValueError, match="direction must be"): + InstanceSort(["node", "externalId"], direction=bad_direction) # type: ignore[arg-type] + + def test_invalid_direction_error_shows_original_value(self) -> None: + with pytest.raises(ValueError, match="'Asc'"): + InstanceSort(["node", "externalId"], direction="Asc") # type: ignore[arg-type] + + @pytest.mark.parametrize( + "direction, nulls_first", + [ + ("ascending", True), + ("descending", False), + ], + ) + def test_warn_fires_for_non_index_aligned_sorts(self, direction: str, nulls_first: bool) -> None: + sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + sort._apply_postgres_defaults_or_maybe_warn() + assert len(w) == 1 + assert issubclass(w[0].category, UserWarning) + assert "not index-aligned" in str(w[0].message) + assert "index utilization" in str(w[0].message) + + @pytest.mark.parametrize( + "direction, nulls_first", + [ + ("ascending", False), + ("descending", True), + ("ascending", None), + ("descending", None), + ], + ) + def test_no_warn_for_aligned_or_unset_sorts(self, direction: str, nulls_first: bool | None) -> None: + sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + sort._apply_postgres_defaults_or_maybe_warn() + assert len(w) == 0 + + +class TestQueryIterSorts: + def test_collects_sorts_from_with_and_select(self) -> None: + from cognite.client.data_classes.data_modeling import ViewId + from cognite.client.data_classes.data_modeling.query import ( + NodeResultSetExpression, + Query, + Select, + SourceSelector, + ) + + view = ViewId("s", "v", "1") + sort_a = InstanceSort(view.as_property_ref("a")) + sort_b = InstanceSort(view.as_property_ref("b")) + query = Query( + with_={"nodes": NodeResultSetExpression(sort=[sort_a])}, + select={"nodes": Select([SourceSelector(view, ["a"])], sort=[sort_b])}, + ) + result = list(query._iter_sorts()) + assert sort_a in result + assert sort_b in result + assert len(result) == 2 + + def test_collects_backfill_sort_from_sync_query(self) -> None: + from cognite.client.data_classes.data_modeling.query import ( + NodeResultSetExpressionSync, + QuerySync, + ) + + sort = InstanceSort(["node", "externalId"]) + query = QuerySync(with_={"nodes": NodeResultSetExpressionSync(backfill_sort=[sort])}, select={}) + result = list(query._iter_sorts()) + assert sort in result + assert len(result) == 1 From 7e9cecfe73dbb46780f0ce8189a14f92cbf0cd32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Fri, 8 May 2026 21:51:26 +0200 Subject: [PATCH 6/8] mypy is not clever enough... --- cognite/client/data_classes/data_modeling/instances.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index f21495ae4b..32fbd8dbf0 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -1407,7 +1407,7 @@ def __init__( if normalized not in ("ascending", "descending"): raise ValueError(f"direction must be 'ascending' or 'descending', got {direction!r}") - super().__init__(property, normalized, nulls_first) + super().__init__(property, normalized, nulls_first) # type: ignore [arg-type] # We override _load to get the more strict __init__ validation on 'direction' because we need it to # be valid for the possible later automatic choice of nulls_first: From 804925f984c2aeeebdfa5fab1ac9b1828cec7db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Tue, 12 May 2026 09:07:31 +0200 Subject: [PATCH 7/8] remove mentions of 'postgres' --- cognite/client/_api/data_modeling/instances.py | 2 +- cognite/client/_sync_api/data_modeling/instances.py | 2 +- cognite/client/data_classes/data_modeling/instances.py | 4 ++-- cognite/client/data_classes/data_modeling/query.py | 2 +- .../test_data_models/test_instances.py | 10 +++++----- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cognite/client/_api/data_modeling/instances.py b/cognite/client/_api/data_modeling/instances.py index 163652f648..823449a91e 100644 --- a/cognite/client/_api/data_modeling/instances.py +++ b/cognite/client/_api/data_modeling/instances.py @@ -972,7 +972,7 @@ def _create_other_params( sorts_seq = [sort] if isinstance(sort, (InstanceSort, dict)) else list(sort) for s in sorts_seq: if isinstance(s, InstanceSort): - s._apply_postgres_defaults_or_maybe_warn() + s._apply_defaults_or_maybe_warn() other_params["sort"] = [cls._dump_instance_sort(s) for s in sorts_seq] if instance_type: other_params["instanceType"] = instance_type diff --git a/cognite/client/_sync_api/data_modeling/instances.py b/cognite/client/_sync_api/data_modeling/instances.py index 2d660424c2..c8ce73b319 100644 --- a/cognite/client/_sync_api/data_modeling/instances.py +++ b/cognite/client/_sync_api/data_modeling/instances.py @@ -1,6 +1,6 @@ """ =============================================================================== -f57e38da23620654d634c19560399f41 +aaf1ed2303de6e9368821b155854a874 This file is auto-generated from the Async API modules, - do not edit manually! =============================================================================== """ diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index 32fbd8dbf0..27ed7f96d9 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -1430,8 +1430,8 @@ def is_index_aligned(self) -> bool: return True return self.nulls_first is (self.direction == "descending") - def _apply_postgres_defaults_or_maybe_warn(self) -> None: - """Resolve nulls_first for PostgreSQL index alignment, warning if the explicit value is misaligned. + def _apply_defaults_or_maybe_warn(self) -> None: + """Resolve nulls_first for database index alignment, warning if the explicit value is misaligned. When nulls_first is None, sets it to the index-compatible value. When explicitly set but misaligned, emits a UserWarning. diff --git a/cognite/client/data_classes/data_modeling/query.py b/cognite/client/data_classes/data_modeling/query.py index ad391b5956..e417edb7df 100644 --- a/cognite/client/data_classes/data_modeling/query.py +++ b/cognite/client/data_classes/data_modeling/query.py @@ -193,7 +193,7 @@ def _iter_sorts(self) -> Iterator[InstanceSort]: def _prepare_sorts(self) -> None: for sort in self._iter_sorts(): - sort._apply_postgres_defaults_or_maybe_warn() + sort._apply_defaults_or_maybe_warn() def dump(self, camel_case: bool = True) -> dict[str, Any]: output: dict[str, Any] = { diff --git a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py index 773250e9f1..d58aab221b 100644 --- a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py +++ b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py @@ -698,12 +698,12 @@ def test_is_index_aligned(self, direction: str, nulls_first: bool | None, expect sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] assert sort.is_index_aligned is expected - def test__apply_postgres_defaults_or_maybe_warn_resolves_none(self) -> None: + def test__apply_defaults_or_maybe_warn_resolves_none(self) -> None: sort = InstanceSort(["node", "externalId"], direction="ascending") - sort._apply_postgres_defaults_or_maybe_warn() + sort._apply_defaults_or_maybe_warn() assert sort.nulls_first is False sort = InstanceSort(["node", "externalId"], direction="descending") - sort._apply_postgres_defaults_or_maybe_warn() + sort._apply_defaults_or_maybe_warn() assert sort.nulls_first is True @pytest.mark.parametrize("bad_direction", ["asc", "desc", "random"]) @@ -726,7 +726,7 @@ def test_warn_fires_for_non_index_aligned_sorts(self, direction: str, nulls_firs sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") - sort._apply_postgres_defaults_or_maybe_warn() + sort._apply_defaults_or_maybe_warn() assert len(w) == 1 assert issubclass(w[0].category, UserWarning) assert "not index-aligned" in str(w[0].message) @@ -745,7 +745,7 @@ def test_no_warn_for_aligned_or_unset_sorts(self, direction: str, nulls_first: b sort = InstanceSort(["node", "externalId"], direction=direction, nulls_first=nulls_first) # type: ignore[arg-type] with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") - sort._apply_postgres_defaults_or_maybe_warn() + sort._apply_defaults_or_maybe_warn() assert len(w) == 0 From 835cba8635fd80362f67860163fd744d00cc0549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Tue, 12 May 2026 09:43:42 +0200 Subject: [PATCH 8/8] copy user query/sort objects to avoid mutation --- .../client/_api/data_modeling/instances.py | 28 ++++++++-------- .../_sync_api/data_modeling/instances.py | 2 +- .../data_classes/data_modeling/instances.py | 3 +- .../data_classes/data_modeling/query.py | 9 ++++-- .../test_data_models/test_instances.py | 32 +++++++++++++++++++ 5 files changed, 57 insertions(+), 17 deletions(-) diff --git a/cognite/client/_api/data_modeling/instances.py b/cognite/client/_api/data_modeling/instances.py index 823449a91e..85dc4317e2 100644 --- a/cognite/client/_api/data_modeling/instances.py +++ b/cognite/client/_api/data_modeling/instances.py @@ -5,6 +5,7 @@ import logging import random from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Sequence +from copy import copy from datetime import datetime, timedelta, timezone from typing import ( TYPE_CHECKING, @@ -894,7 +895,7 @@ async def subscribe( >>> subscription_context.cancel() """ - query._prepare_sorts() + query = query._get_query_with_defaults_applied() subscription_context = SubscriptionContext() async def _poll_loop() -> None: @@ -970,20 +971,19 @@ def _create_other_params( ) if sort: sorts_seq = [sort] if isinstance(sort, (InstanceSort, dict)) else list(sort) + result = [] for s in sorts_seq: if isinstance(s, InstanceSort): - s._apply_defaults_or_maybe_warn() - other_params["sort"] = [cls._dump_instance_sort(s) for s in sorts_seq] + s = copy(s)._apply_defaults_or_maybe_warn().dump(camel_case=True) + result.append(s) + other_params["sort"] = result + if instance_type: other_params["instanceType"] = instance_type if debug: other_params["debug"] = debug.dump() return other_params - @staticmethod - def _dump_instance_sort(sort: InstanceSort | dict) -> dict: - return sort.dump(camel_case=True) if isinstance(sort, InstanceSort) else sort - async def apply( self, nodes: NodeApply | Sequence[NodeApply] | None = None, @@ -1306,11 +1306,14 @@ async def search( body["targetUnits"] = [unit.dump(camel_case=True) for unit in target_units] if sort: sorts = sort if isinstance(sort, Sequence) else [sort] - for sort_spec in sorts: - nulls_first = sort_spec.get("nullsFirst") if isinstance(sort_spec, dict) else sort_spec.nulls_first - if nulls_first is not None: + result = [] + for s in sorts: + if isinstance(s, InstanceSort): + s = s.dump(camel_case=True) + if s.get("nullsFirst") is not None: raise ValueError("nulls_first argument is not supported when sorting on instance search") - body["sort"] = [self._dump_instance_sort(s) for s in sorts] + result.append(s) + body["sort"] = result semaphore = self._get_semaphore("search") res = await self._post(url_path=self._RESOURCE_PATH + "/search", json=body, semaphore=semaphore) @@ -1649,7 +1652,6 @@ async def query( >>> res = client.data_modeling.instances.query(query, debug=debug_params) >>> print(res.debug) """ - query._prepare_sorts() return await self._query_or_sync(query, "query", include_typing=include_typing, debug=debug) async def sync( @@ -1752,7 +1754,6 @@ async def sync( >>> res = client.data_modeling.instances.sync(query, debug=debug_params) >>> print(res.debug) """ - query._prepare_sorts() return await self._query_or_sync(query, "sync", include_typing=include_typing, debug=debug) async def _query_or_sync( @@ -1762,6 +1763,7 @@ async def _query_or_sync( include_typing: bool, debug: DebugParameters | None, ) -> QueryResult: + query = query._get_query_with_defaults_applied() headers: None | dict[str, str] = None body = query.dump(camel_case=True) if include_typing: diff --git a/cognite/client/_sync_api/data_modeling/instances.py b/cognite/client/_sync_api/data_modeling/instances.py index c8ce73b319..efff8e8be2 100644 --- a/cognite/client/_sync_api/data_modeling/instances.py +++ b/cognite/client/_sync_api/data_modeling/instances.py @@ -1,6 +1,6 @@ """ =============================================================================== -aaf1ed2303de6e9368821b155854a874 +15c69385f1ab10e31adbde3483ee7588 This file is auto-generated from the Async API modules, - do not edit manually! =============================================================================== """ diff --git a/cognite/client/data_classes/data_modeling/instances.py b/cognite/client/data_classes/data_modeling/instances.py index 27ed7f96d9..1c6e1677ed 100644 --- a/cognite/client/data_classes/data_modeling/instances.py +++ b/cognite/client/data_classes/data_modeling/instances.py @@ -1430,7 +1430,7 @@ def is_index_aligned(self) -> bool: return True return self.nulls_first is (self.direction == "descending") - def _apply_defaults_or_maybe_warn(self) -> None: + def _apply_defaults_or_maybe_warn(self) -> Self: """Resolve nulls_first for database index alignment, warning if the explicit value is misaligned. When nulls_first is None, sets it to the index-compatible value. When explicitly set but @@ -1449,6 +1449,7 @@ def _apply_defaults_or_maybe_warn(self) -> None: UserWarning, stacklevel=3, ) + return self @dataclass diff --git a/cognite/client/data_classes/data_modeling/query.py b/cognite/client/data_classes/data_modeling/query.py index e417edb7df..31e00668dd 100644 --- a/cognite/client/data_classes/data_modeling/query.py +++ b/cognite/client/data_classes/data_modeling/query.py @@ -3,6 +3,7 @@ from abc import ABC from collections import UserDict from collections.abc import Iterator, Mapping, MutableMapping, Sequence +from copy import deepcopy from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar @@ -191,9 +192,13 @@ def _iter_sorts(self) -> Iterator[InstanceSort]: for sel in self.select.values(): yield from sel._iter_sorts() - def _prepare_sorts(self) -> None: - for sort in self._iter_sorts(): + def _get_query_with_defaults_applied(self) -> Self: + """TODO: We could verify (or just warn), when Query and Sync-versions are mixed or used in the wrong setting.""" + # We don't want to mutate the user's original query, so we make a deepcopy and apply defaults to that: + query = deepcopy(self) + for sort in query._iter_sorts(): sort._apply_defaults_or_maybe_warn() + return query def dump(self, camel_case: bool = True) -> dict[str, Any]: output: dict[str, Any] = { diff --git a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py index d58aab221b..34f8fb73e7 100644 --- a/tests/tests_unit/test_data_classes/test_data_models/test_instances.py +++ b/tests/tests_unit/test_data_classes/test_data_models/test_instances.py @@ -782,3 +782,35 @@ def test_collects_backfill_sort_from_sync_query(self) -> None: result = list(query._iter_sorts()) assert sort in result assert len(result) == 1 + + def test_get_query_with_defaults_applied_does_not_mutate_original(self) -> None: + from cognite.client.data_classes.data_modeling import ViewId + from cognite.client.data_classes.data_modeling.query import ( + NodeResultSetExpression, + Query, + Select, + SourceSelector, + ) + + view = ViewId("s", "v", "1") + sort_a = InstanceSort(view.as_property_ref("a")) + sort_b = InstanceSort(view.as_property_ref("b")) + query = Query( + with_={"nodes": NodeResultSetExpression(sort=[sort_a])}, + select={"nodes": Select([SourceSelector(view, ["a"])], sort=[sort_b])}, + ) + + prepared = query._get_query_with_defaults_applied() + + # Original sorts untouched + assert sort_a.nulls_first is None + assert sort_b.nulls_first is None + # Prepared copy has resolved values + assert all(s.nulls_first is not None for s in prepared._iter_sorts()) + + def test_list_sort_does_not_mutate_original(self) -> None: + from cognite.client._api.data_modeling.instances import InstancesAPI + + sort = InstanceSort(["node", "externalId"], direction="ascending") + InstancesAPI._create_other_params(include_typing=False, sort=sort, sources=None, instance_type=None) + assert sort.nulls_first is None