Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix query edge cases #213

Merged
merged 5 commits into from
Oct 24, 2024
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
12 changes: 10 additions & 2 deletions docs/queries/queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ users = await User.query.filter(User.columns.id.in_([1, 2, 3]))
!!! Warning
The `columns` refers to the columns of the underlying SQLAlchemy table.

!!! Warning
This works only for the main model of a QuerySet. Related are handled via `f"{hash_tablkey(tablekey=model.table.key, prefix=...)}_{columnkey}"`.
You can pass the column via `sqlalchemy.column` (lowercase column).

All the operations you would normally do in SQLAlchemy syntax, are allowed here.

##### Using `and_` and `or_` with kwargs
Expand All @@ -277,7 +281,7 @@ users = await User.query.filter(and_.from_kwargs(User, name="foo", email="foo@ex
users = await User.query.filter(and_.from_kwargs(User, **my_dict))
```

#### Global OR
#### OR

Edgy QuerySet can do global ORs. This means you can attach new OR clauses also later.

Expand Down Expand Up @@ -313,7 +317,7 @@ user_query = user_query.or_(user_query, {"email": "outlook"}, {"email": "gmail"}
users = await user_query
```

#### Passing multiple keyword based filters
##### Passing multiple keyword based filters

You can also passing multiple keyword based filters by providing them as a dictionary

Expand All @@ -322,7 +326,10 @@ user_query = User.query.or_({"active": True}, {"email": "outlook"}, {"email": "g
# active users or users with email gmail or outlook are retrieved
users = await user_query
```
##### Local only OR

If the special mode of or_ is not wanted there is a function named `local_or`. It is similar
to the or_ function except it doesn't have the global OR mode.

### Limit

Expand Down Expand Up @@ -1090,6 +1097,7 @@ The pendant in a model are `identifying_clauses`.
query = Model.query.filter(id=1)
# ensures that the db connection doesn't drop during operation
async with query.database as database:
# when using joins a exist subquery is generated
expression = query.table.select().where(await query.build_where_clause())
# as generic sql
print(str(expression))
Expand Down
14 changes: 13 additions & 1 deletion docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,29 @@ hide:

# Release Notes

## Unreleased
## 0.20.0

### Added

- Add DurationField.
- Allow passing `max_digits` to FloatField.
- Add `local_or` function to QuerySets.

### Changed

- Only the main table of a queryset is queryable via `model_class.columns.foo == foo`. Select related models have now an unique name for their path.
The name can be retrieved via `tables_and_models` or using `f"{hash_tablekey(...)}_{column}"`.
- Breaking: Alter tables_and_models to use the prefix as key with '' for the maintable and model.
- Breaking: Functions passed to filter functions reveive now a second positional parameter `tables_and_models`.
- `build_where_clause` conditionally uses a subquery.
- Rename QueryType to QuerySetType. The old name stays as an alias.

### Fixed

- Triggering load on non-existent field when reflecting.
- InspectDB mapping was incorrect.
- Fix query edge cases.
- Fix using related queries with update/delete.


## 0.19.1
Expand Down
12 changes: 9 additions & 3 deletions edgy/core/db/models/mixins/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,19 @@ def get_columns_for_name(self: "Model", name: str) -> Sequence["sqlalchemy.Colum
else:
return cast(Sequence["sqlalchemy.Column"], _empty)

def identifying_clauses(self) -> list[Any]:
def identifying_clauses(self, prefix: str = "") -> list[Any]:
# works only if the class of the model is the main class of the queryset
# TODO: implement prefix handling and return generic column without table attached
if prefix:
raise NotImplementedError()
clauses: list[Any] = []
for field_name in self.identifying_db_fields:
field = self.meta.fields.get(field_name)
if field is not None:
for column, value in field.clean(field_name, self.__dict__[field_name]).items():
clauses.append(getattr(self.table.columns, column) == value)
for column_name, value in field.clean(
field_name, self.__dict__[field_name]
).items():
clauses.append(getattr(self.table.columns, column_name) == value)
else:
clauses.append(
getattr(self.table.columns, field_name) == self.__dict__[field_name]
Expand Down
137 changes: 57 additions & 80 deletions edgy/core/db/models/mixins/row.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import asyncio
from collections.abc import Sequence
from typing import TYPE_CHECKING, Any, Optional
from typing import TYPE_CHECKING, Any, Optional, cast

from edgy.core.db.fields.base import RelationshipField
from edgy.core.db.models.utils import apply_instance_extras
Expand Down Expand Up @@ -28,10 +28,7 @@ def can_load_from_row(cls: type["Model"], row: "Row", table: "Table") -> bool:
return bool(
cls.meta.registry
and not cls.meta.abstract
and all(
row._mapping.get(f"{table.key.replace('.', '_')}_{col}") is not None
for col in cls.pkcolumns
)
and all(row._mapping.get(f"{table.name}_{col}") is not None for col in cls.pkcolumns)
)

@classmethod
Expand Down Expand Up @@ -87,17 +84,13 @@ async def from_sqla_row(
detail=f'Selected field "{field_name}" is not a RelationshipField on {cls}.'
) from None

_prefix = field_name if not prefix else f"{prefix}__{field_name}"
# stop selecting when None. Related models are not available.
if not model_class.can_load_from_row(
row,
tables_and_models[
model_class.meta.tablename
if using_schema is None
else f"{using_schema}.{model_class.meta.tablename}"
][0],
tables_and_models[_prefix][0],
):
continue
_prefix = field_name if not prefix else f"{prefix}__{field_name}"

if remainder:
# don't pass table, it is only for the main model_class
Expand Down Expand Up @@ -130,9 +123,7 @@ async def from_sqla_row(
for k, v in item.items():
setattr(old_select_related_value, k, v)
return old_select_related_value
table_columns = tables_and_models[
cls.meta.tablename if using_schema is None else f"{using_schema}.{cls.meta.tablename}"
][0].columns
table_columns = tables_and_models[prefix][0].columns
# Populate the related names
# Making sure if the model being queried is not inside a select related
# This way it is not overritten by any value
Expand All @@ -153,12 +144,15 @@ async def from_sqla_row(
child_item = {}
for column_name in columns_to_check:
column = getattr(table_columns, column_name, None)
if (
column is not None
and f"{column.table.key.replace('.', '_')}_{column.key}" in row._mapping
):
if column_name is None:
continue
columnkeyhash = column_name
if prefix:
columnkeyhash = f"{tables_and_models[prefix][0].name}_{column.key}"

if columnkeyhash in row._mapping:
child_item[foreign_key.from_fk_field_name(related, column_name)] = (
row._mapping[f"{column.table.key.replace('.', '_')}_{column.key}"]
row._mapping[columnkeyhash]
)
# Make sure we generate a temporary reduced model
# For the related fields. We simply chnage the structure of the model
Expand Down Expand Up @@ -190,13 +184,14 @@ async def from_sqla_row(
if column.key not in cls.meta.columns_to_field:
continue
# set if not of an foreign key with one column
elif (
column.key not in item
and f"{column.table.key.replace('.', '_')}_{column.key}" in row._mapping
):
item[column.key] = row._mapping[
f"{column.table.key.replace('.', '_')}_{column.key}"
]
if column.key in item:
continue
columnkeyhash = column.key
if prefix:
columnkeyhash = f"{tables_and_models[prefix][0].name}_{columnkeyhash}"

if columnkeyhash in row._mapping:
item[column.key] = row._mapping[columnkeyhash]
model: Model = (
cls.proxy_model(**item, __phase__="init_db") # type: ignore
if exclude_secrets or is_defer_fields or only_fields
Expand All @@ -208,24 +203,18 @@ async def from_sqla_row(
cls,
using_schema,
database=database,
table=tables_and_models[
cls.meta.tablename
if using_schema is None
else f"{using_schema}.{cls.meta.tablename}"
][0],
table=tables_and_models[prefix][0],
)

# Handle prefetch related fields.
await cls.__handle_prefetch_related(
row=row,
table=tables_and_models[
cls.meta.tablename
if using_schema is None
else f"{using_schema}.{cls.meta.tablename}"
][0],
model=model,
prefetch_related=prefetch_related,
)
if prefetch_related:
# Handle prefetch related fields.
await cls.__handle_prefetch_related(
row=row,
prefix=prefix,
model=model,
tables_and_models=tables_and_models,
prefetch_related=prefetch_related,
)
assert model.pk is not None, model
return model

Expand All @@ -243,27 +232,21 @@ def __should_ignore_related_name(
return False

@classmethod
def create_model_key_from_sqla_row(
cls,
row: "Row",
) -> tuple:
def create_model_key_from_sqla_row(cls, row: "Row", row_prefix: str = "") -> tuple:
"""
Build a cache key for the model.
"""
pk_key_list: list[Any] = [cls.__name__]
for attr in cls.pkcolumns:
try:
pk_key_list.append(str(row._mapping[getattr(cls.table.columns, attr)]))
except KeyError:
pk_key_list.append(str(row._mapping[attr]))
pk_key_list.append(str(row._mapping[f"{row_prefix}{attr}"]))
return tuple(pk_key_list)

@classmethod
async def __set_prefetch(
cls,
row: "Row",
table: "Table",
model: "Model",
row_prefix: str,
related: "Prefetch",
) -> None:
model_key = ()
Expand All @@ -273,48 +256,39 @@ async def __set_prefetch(
if model_key in related._baked_results:
setattr(model, related.to_attr, related._baked_results[model_key])
else:
clauses = []
for pkcol in cls.pkcolumns:
clauses.append(
getattr(table.columns, pkcol)
== row._mapping[f"{table.key.replace('.', '_')}_{pkcol}"]
crawl_result = crawl_relationship(
model.__class__, related.related_name, traverse_last=True
)
if crawl_result.reverse_path is False:
QuerySetError(
detail=("Creating a reverse path is not possible, unidirectional fields used.")
)
if crawl_result.cross_db_remainder:
raise NotImplementedError(
"Cannot prefetch from other db yet. Maybe in future this feature will be added."
)
queryset = related.queryset
if related._is_finished:
assert queryset is not None, "Queryset is not set but _is_finished flag"
else:
check_prefetch_collision(model, related)
crawl_result = crawl_relationship(
model.__class__, related.related_name, traverse_last=True
)
if queryset is None:
if crawl_result.reverse_path is False:
queryset = model.__class__.query.all()
else:
queryset = crawl_result.model_class.query.all()
queryset = crawl_result.model_class.query.all()

if queryset.model_class == model.__class__:
# queryset is of this model
queryset = queryset.select_related(related.related_name)
queryset.embed_parent = (related.related_name, "")
elif crawl_result.reverse_path is False:
QuerySetError(
detail=(
f"Creating a reverse path is not possible, unidirectional fields used."
f"You may want to use as queryset a queryset of model class {model!r}."
)
)
else:
# queryset is of the target model
queryset = queryset.select_related(crawl_result.reverse_path)
setattr(model, related.to_attr, await queryset.filter(*clauses))
queryset = queryset.select_related(cast(str, crawl_result.reverse_path))
clause = {
f"{crawl_result.reverse_path}__{pkcol}": row._mapping[f"{row_prefix}{pkcol}"]
for pkcol in cls.pkcolumns
}
setattr(model, related.to_attr, await queryset.filter(clause))

@classmethod
async def __handle_prefetch_related(
cls,
row: "Row",
table: "Table",
model: "Model",
prefix: str,
tables_and_models: dict[str, tuple["Table", type["BaseModelType"]]],
prefetch_related: Sequence["Prefetch"],
) -> None:
"""
Expand All @@ -331,6 +305,9 @@ async def __handle_prefetch_related(
# Check for conflicting names
# Check as early as possible
check_prefetch_collision(model=model, related=related)
queries.append(cls.__set_prefetch(row=row, table=table, model=model, related=related))
row_prefix = f"{tables_and_models[prefix].name}_" if prefix else ""
queries.append(
cls.__set_prefetch(row=row, row_prefix=row_prefix, model=model, related=related)
)
if queries:
await asyncio.gather(*queries)
Loading