Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 21 additions & 1 deletion ormar/models/excludable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
"""
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions ormar/models/mixins/excludable_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
70 changes: 70 additions & 0 deletions tests/test_queries/test_values_and_values_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
]