From d28ecdecbf730cea5cda1540ebf8e35af16845fd Mon Sep 17 00:00:00 2001 From: collerek Date: Wed, 13 May 2026 14:06:37 +0200 Subject: [PATCH] fix parent models fields leaking into values and values_list despite not being explicitly mentioned in fields --- docs/releases.md | 8 +++ ormar/models/excludable.py | 22 +++++- ormar/models/mixins/excludable_mixin.py | 4 ++ .../test_values_and_values_list.py | 70 +++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/docs/releases.md b/docs/releases.md index 1e207ab22..36db9cc53 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -14,6 +14,14 @@ their through models for m2m - are now excluded from the projected result, matching the columns the caller actually requested. [#800](https://github.com/collerek/ormar/issues/800) +* Fix `values()` / `values_list()` leaking main-model columns when + `fields()` named only nested fields (e.g. + `values_list("garage__owner", flatten=True)`). The flat projection now + drops every column from any model the caller never named - including + the parent FK column and any in-between models the path passes through + - so `flatten=True` returns the requested nested value instead of the + first leaked main-model column. + [#1060](https://github.com/collerek/ormar/issues/1060) ## 0.25.0 diff --git a/ormar/models/excludable.py b/ormar/models/excludable.py index 8e5028710..217811c65 100644 --- a/ormar/models/excludable.py +++ b/ormar/models/excludable.py @@ -241,11 +241,17 @@ class Excludable: """ Class that keeps sets of fields to include, exclude, and flatten for a single model at a given alias/relation level. + + ``pk_only`` is a short-circuit consulted by :meth:`own_table_columns`: when + set, only the primary-key column is projected for this model and the + ``include``/``exclude`` sets are ignored. Producers should not combine + ``pk_only=True`` with a non-empty ``include``/``exclude`` - the flag wins. """ include: set = field(default_factory=set) exclude: set = field(default_factory=set) flatten: set = field(default_factory=set) + pk_only: bool = False def get_copy(self) -> "Excludable": """ @@ -258,6 +264,7 @@ def get_copy(self) -> "Excludable": _copy.include = {x for x in self.include} _copy.exclude = {x for x in self.exclude} _copy.flatten = {x for x in self.flatten} + _copy.pk_only = self.pk_only return _copy def set_values(self, value: set, slot: Slot) -> None: @@ -758,6 +765,10 @@ def with_projection_exclusions( excludable = ExcludableItems.from_excludable(self) + # If fields() never names the source model, project only its PK. + if not excludable._referenced_at(source_model, ()): + excludable.get(source_model).pk_only = True + for path in select_related: parts = tuple(path.split("__")) # Snapshot which prefixes carry a fields() reference before any @@ -778,7 +789,16 @@ def with_projection_exclusions( source_model, parts[:i] ) parent_exc = excludable.get(parent_model, alias=parent_alias) - if parent_exc.is_explicitly_included(segment) or kept_deeper: + if parent_exc.is_explicitly_included(segment): + continue + if kept_deeper: + # Intermediate relation: keep the join but project + # PK-only when this segment was never explicitly named. + if not referenced[i]: + target_alias, target_model = excludable._resolve_path( + source_model, parts[: i + 1] + ) + excludable.get(target_model, alias=target_alias).pk_only = True continue field = parent_model.ormar_config.model_fields[segment] parent_exc.exclude.add(segment) diff --git a/ormar/models/mixins/excludable_mixin.py b/ormar/models/mixins/excludable_mixin.py index 28c65ab7b..c83a7ff07 100644 --- a/ormar/models/mixins/excludable_mixin.py +++ b/ormar/models/mixins/excludable_mixin.py @@ -110,6 +110,10 @@ def own_table_columns( :rtype: list[str] """ model_excludable = excludable.get(model_cls=model, alias=alias) # type: ignore + if model_excludable.pk_only: + if not add_pk_columns: + return [] + return cls._populate_pk_column(model=model, columns=[], use_alias=use_alias) has_include = bool(model_excludable.include) has_exclude = bool(model_excludable.exclude) diff --git a/tests/test_queries/test_values_and_values_list.py b/tests/test_queries/test_values_and_values_list.py index 639d3c56f..427eda654 100644 --- a/tests/test_queries/test_values_and_values_list.py +++ b/tests/test_queries/test_values_and_values_list.py @@ -548,3 +548,73 @@ async def test_filter_related_without_fields_keeps_all_columns(): "category__created_by": 1, }, ] + + +@pytest.mark.asyncio +async def test_nested_only_fields_drops_main_columns_in_values(): + async with base_ormar_config.database: + posts = await ( + Post.objects.select_related("category").fields(["category__name"]).values() + ) + assert posts == [ + {"category__name": "News"}, + {"category__name": "News"}, + {"category__name": "News"}, + ] + + +@pytest.mark.asyncio +async def test_nested_only_fields_drops_main_columns_in_values_list(): + async with base_ormar_config.database: + posts = await ( + Post.objects.select_related("category") + .fields(["category__name"]) + .values_list() + ) + assert posts == [("News",), ("News",), ("News",)] + + +@pytest.mark.asyncio +async def test_nested_only_fields_flatten_returns_nested_value(): + """ + Reproducer for issue #1060. Without the fix, flatten=True picks up + Post.id (the first leaked main-model column) instead of the nested + value the caller actually requested. + """ + async with base_ormar_config.database: + names = await ( + Post.objects.select_related("category") + .fields(["category__name"]) + .values_list(flatten=True) + ) + assert names == ["News", "News", "News"] + + +@pytest.mark.asyncio +async def test_deeper_nested_only_fields_drops_main_columns(): + async with base_ormar_config.database: + posts = await ( + Post.objects.select_related("category__created_by") + .fields(["category__created_by__name"]) + .values() + ) + assert posts == [ + {"category__created_by__name": "Anonymous"}, + {"category__created_by__name": "Anonymous"}, + {"category__created_by__name": "Anonymous"}, + ] + + +@pytest.mark.asyncio +async def test_main_referenced_keeps_main_columns(): + async with base_ormar_config.database: + posts = await ( + Post.objects.select_related("category") + .fields(["name", "category__name"]) + .values() + ) + assert posts == [ + {"name": "Ormar strikes again!", "category__name": "News"}, + {"name": "Why don't you use ormar yet?", "category__name": "News"}, + {"name": "Check this out, ormar now for free", "category__name": "News"}, + ]