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

Multi ForeignKey not working #47

Closed
EspenAlbert opened this issue Nov 20, 2020 · 3 comments
Closed

Multi ForeignKey not working #47

EspenAlbert opened this issue Nov 20, 2020 · 3 comments
Labels
question Further information is requested

Comments

@EspenAlbert
Copy link
Contributor

Thanks for the quick fix and the elaboration on the previous issue
I hope to contribute some code at some point.

I think I found one more bug:

import databases
import pytest
import sqlalchemy
from ormar import ModelMeta, Model, Integer, Boolean, Float, String, ForeignKey

from typing import Optional

from sqlalchemy import create_engine

DATABASE_URL = "sqlite:///test.db"
database = databases.Database(DATABASE_URL)
metadata = sqlalchemy.MetaData()


class MainMeta(ModelMeta):
    metadata = metadata
    database = database


class PositionOrm(Model):
    class Meta(MainMeta):
        pass

    id: int = Integer(primary_key=True, autoincrement=True)
    name: str = String(max_length=50, unique=True, index=True)
    x: float = Float()
    y: float = Float()
    degrees: float = Float()
    is_charging_station: bool = Boolean()
    is_parallell: bool = Boolean()


class ChargingStationOrm(Model):
    class Meta(MainMeta):
        pass

    id: int = Integer(primary_key=True, autoincrement=True)


class ChargingPad(Model):
    class Meta(MainMeta):
        pass

    id: int = Integer(primary_key=True, autoincrement=True)
    wheel_type: int = Integer()
    position: Optional[PositionOrm] = ForeignKey(PositionOrm)
    charging_station: Optional[ChargingStationOrm] = ForeignKey(ChargingStationOrm)


@pytest.fixture(autouse=True, scope="module")
def create_test_database():
    engine = create_engine(DATABASE_URL)
    metadata.create_all(engine)
    yield
    metadata.drop_all(engine)


@pytest.fixture(scope="function")
async def cleanup():
    yield
    async with database:
        await PositionOrm.objects.delete(each=True)
        await ChargingStationOrm.objects.delete(each=True)
        await ChargingPad.objects.delete(each=True)


@pytest.fixture()
async def db():
    async with database:
        yield


charging_station = ChargingStationOrm(
    id=1,
)
charging_pads = [
    ChargingPad(id=id, wheel_type=id, charging_station=1, position=id)
    for id in [1, 2, 3, 4]
]
charging_positions = [
    PositionOrm(
        name=f"n{i}",
        x=i * 1.1,
        y=i * 2.2,
        degrees=i * 3.3,
        is_charging_station=True,
        is_parallell=True,
    )
    for i in [1, 2, 3, 4]
]


@pytest.mark.asyncio()
async def test_create_charging_station(cleanup, db):
    await charging_station.save()
    await PositionOrm.objects.bulk_create(charging_positions)
    for charging_pad in charging_pads:
        await charging_pad.save()

    pan_ids_db = await charging_station.chargingpads.all()
    assert len(pan_ids_db) == 4

Stacktrace:

tests_ormar/test_issue_multi_foreign_key.py:92 (test_create_charging_station)
test_issue_multi_foreign_key.py:100: in test_create_charging_station
    pan_ids_db = await charging_station.chargingpads.all()
venv/lib/python3.8/site-packages/ormar/relations/relation_proxy.py:27: in __getattr__
    self._initialize_queryset()
venv/lib/python3.8/site-packages/ormar/relations/relation_proxy.py:32: in _initialize_queryset
    self.queryset_proxy.queryset = self._set_queryset()
venv/lib/python3.8/site-packages/ormar/relations/relation_proxy.py:50: in _set_queryset
    ormar.QuerySet(model_cls=self.relation.to)
venv/lib/python3.8/site-packages/ormar/queryset/queryset.py:133: in filter
    filter_clauses, select_related = qryclause.filter(**kwargs)
venv/lib/python3.8/site-packages/ormar/queryset/clause.py:50: in filter
    filter_clauses, select_related = self._populate_filter_clauses(**kwargs)
venv/lib/python3.8/site-packages/ormar/queryset/clause.py:77: in _populate_filter_clauses
    ) = self._determine_filter_target_table(
venv/lib/python3.8/site-packages/ormar/queryset/clause.py:136: in _determine_filter_target_table
    if issubclass(model_cls.Meta.model_fields[part], ManyToManyField):
E   KeyError: 'chargingstationorms'
@collerek
Copy link
Owner

collerek commented Nov 20, 2020

Ok, so this is how you go from parent model to child model in ForeigKey relation:

# you need to go through QuerySet and select related model 'chargingpads' and select all()
# will get you a list of stations (and you created only one) so yours is at index 0
# note that you CAN use instance here (charging_station.objects...) but better be explicit as this can be changed in future releases
station = await ChargingStationOrm.objects.select_related('chargingpads').all()
assert len(station[0].chargingpads) == 4

# the same but get by pk (or any other filter combination that will return one row) so no list and no need for index
# note that if you do not provide filter parameters for now the query is limited to 2 rows - plan to change that in the future
# so if you provide just get() at the end there will be only 2 pads - it's a feature not a bug as empty get should not be issued with select_related
station2 = await ChargingStationOrm.objects.select_related('chargingpads').get(pk=1)
assert len(station2.chargingpads) == 4

This is actually the only not yet implemented load (you can load child from parent: so charging_pad.charging_station.load() and child of ManyToMany with Model.many_to_many_name.all()).

You can find some explanation and discussion in #27.
Probably it will be added but still this produces additional query and this is probably not something you want performance wide -> select related composes a (sometimes complex) join in only one db query -> nested models are later extracted from result in python.

@collerek
Copy link
Owner

collerek commented Nov 20, 2020

Oh and if you want the other way around use:

# only pads
pads = ChargingPad.object.all()
assert len(pads) == 4

# only pads and only pk field -> will complain if you try to exclude required fields
pads = ChargingPad.object.fields("id").all()
assert len(pads) == 4

# with station
pads = ChargingPad.object.select_related("charging_station").all()
assert len(pads) == 4

Note that ormar never returns you raw data. So even if you limit your query to one field (pk as it cannot be excluded - pk is always returned - note that regardless of the real name you can always set it and get it by pk alias) you will get a model with only one field populated (rest will be None). That's why if you want to exclude fields from result those fields needs to be nullable=True as otherwise pydantic will raise ValidationError. (more on this in fields() and exclude_fields() docs in queries)

So even in the first query above you can still go pads[0].charging_station.load() even if you did not provide select_related(), as the foreignkey database column is filled and ormar provides you with nested CharingStationOrm model with pk only populated (this is a special case where only pk can be set and no ValidationError is raised. If you explicitly set select_related stating that you want a nested model returned from a database and try to exclude required fields it will complain).

If you think that the documentation is not clear in this regard let me know. (Or you can issue a PR - it's always easier to start contributing with the docs). If that answers your question please close the issue :)

@collerek collerek added the question Further information is requested label Nov 20, 2020
@EspenAlbert
Copy link
Contributor Author

Awesome. Thank you so much for the detailed how-to :)

I feel encouraged to contribute. Let's see what happens ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants