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

Accessing a Query with a Subquery will break it for subsequent use #780

Closed
spacemanspiff2007 opened this issue May 28, 2021 · 9 comments
Closed

Comments

@spacemanspiff2007
Copy link
Contributor

spacemanspiff2007 commented May 28, 2021

Filtering relationships with sub queries does not work.

To Reproduce

from tortoise import fields, run_async, Tortoise
from tortoise.expressions import Subquery
from tortoise.fields import ManyToManyRelation, ManyToManyField
from tortoise.models import Model


class MyModel(Model):
    id = fields.IntField(pk=True)
    name = fields.TextField()
    properties: ManyToManyRelation["Property"]


class Property(Model):
    id = fields.IntField(pk=True)
    my_models: ManyToManyRelation["MyModel"] = ManyToManyField("models.MyModel", related_name='properties')


async def run():

    # Generate the schema
    await Tortoise.init({"connections": {"default_connection": {"engine": "tortoise.backends.sqlite", "credentials": {"file_path": ':memory:'},},}, "apps": {"models": {"models": ['__main__'], "default_connection": "default_connection"},},})
    await Tortoise.generate_schemas()

    m1 = await MyModel.create(id=1, name='Model1')
    m2 = await MyModel.create(id=2, name='Model2')
    p1 = await Property.create(id=1)
    p2 = await Property.create(id=2)

    await m1.properties.add(p1)
    await m2.properties.add(p1, p2)
    print('Properties Model1:', await m1.properties.all())    # Ouput: Model1: [<Property: 1>]
    print('Properties Model2:', await m2.properties.all())    # Ouput: Model2: [<Property: 1>, <Property: 2>]
    print()

    sub1 = Subquery(MyModel.filter(properties__id=1).values("id"))  # filter all relations with Property 1
    sub2 = Subquery(MyModel.filter(properties__id=2).values("id"))  # filter all relations with Property 2

    query = MyModel.filter(pk__in=sub1)  # This should select Model 1 and Model 2
    query = query.filter(pk__in=sub2)    # This should select Model 2

    # ------------------------------------------------------------------------------------------------------------------
    # commenting out one of these statements will change the result
    print('SQL1 : ', query.sql())

    print('Count: ', await query.count())  # Output: 0  but expected 1
    print('All  : ', await query.all())    # Output: [] but expected [<MyModel: 1>]

    # this will always show something different than the first query.sql()
    print('SQL2 : ', query.sql())


run_async(run())

Result from snippet. count and all is not working. Note that the SQL statement changed from SQL1 to SQL2.

SQL1 :  SELECT "id","name" FROM "mymodel" WHERE "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=1) AND "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=2)
Count:  0
All  :  []
SQL2 :  SELECT "id","name" FROM "mymodel" WHERE "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=1) AND "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=2)

Result with SQL1 commented out. Note that count is working now, but all still isn't!

Count:  1
All  :  []
SQL2 :  SELECT "name","id" FROM "mymodel" WHERE "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=1) AND "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=2)

Result with SQL1 + count commented out. Note that all is working now!

All  :  [<MyModel: 2>]
SQL2 :  SELECT "name","id" FROM "mymodel" WHERE "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=1) AND "id" IN (SELECT "id" "id" FROM "mymodel" WHERE "id"=2)
PS: The Generated SQL1 seems strange with the two left outer joins:
SELECT
    "name", "id" 
FROM
    "mymodel" 
WHERE
    "id" IN 
    (
        SELECT
            "mymodel"."id" "id" 
        FROM
            "mymodel" 
            LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id" = "property_mymodel"."mymodel_id" 
            LEFT OUTER JOIN "property" ON "property_mymodel"."property_id" = "property"."id" 
        WHERE
            "property"."id" = 1
    )
    AND "id" IN 
    (
        SELECT
            "mymodel"."id" "id" 
        FROM
            "mymodel" 
            LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id" = "property_mymodel"."mymodel_id" 
            LEFT OUTER JOIN "property" ON "property_mymodel"."property_id" = "property"."id" 
        WHERE
            "property"."id" = 2
    )
@spacemanspiff2007
Copy link
Contributor Author

@long2ice
Any idea what is causing the issue? Filtering on relationships does currently not work.

@spacemanspiff2007
Copy link
Contributor Author

@long2ice
This is still broken - are there any plans to fix it?
I am actually using the implementation of #756 successfully.
Maybe we can refactor this implementation a little bit and merge it.
What do you think?

@spacemanspiff2007
Copy link
Contributor Author

@long2ice
This issue is still there with 0.17.6. Is there anything I can do to help you reproduce the issue?

@spacemanspiff2007
Copy link
Contributor Author

Issue is still there with 0.17.8

@spacemanspiff2007
Copy link
Contributor Author

@abondar Can you reproduce this issue?

@abondar
Copy link
Member

abondar commented Jun 7, 2024

Hi!

Yes, it reproduces

Issue is that materializing query changes state of query, and it seems like some data could be lost if we call it several times

Simple adhoc fix for Subquery works

class Subquery(Term):  # type: ignore
    def __init__(self, query: "AwaitableQuery"):
        super().__init__()
        self.query = query

    def get_sql(self, **kwargs: Any) -> str:
        from copy import deepcopy
        query = deepcopy(self.query)
        return query.as_query().get_sql(**kwargs)

    def as_(self, alias: str) -> "Selectable":
        from copy import deepcopy
        query = deepcopy(self.query)
        return query.as_query().as_(alias)

After that your example gives identical queries

SQL1 :  SELECT "name","id" FROM "mymodel" WHERE "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=1) AND "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=2)
Count:  1
All  :  [<MyModel: 2>]
SQL2 :  SELECT "name","id" FROM "mymodel" WHERE "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=1) AND "id" IN (SELECT "mymodel"."id" "id" FROM "mymodel" LEFT OUTER JOIN "property_mymodel" ON "mymodel"."id"="property_mymodel"."mymodel_id" LEFT OUTER JOIN "property" ON "property_mymodel"."property_id"="property"."id" WHERE "property"."id"=2)

But not sure if it the way we should band-aid like this, or should investigate deeper what exactly breaks

Unfortunately I am mostly not available for coming several days, so don't have opportunity to investigate it, may be will find time next week

@abondar
Copy link
Member

abondar commented Jun 7, 2024

I think I found the issue - ValuesQuery and ValuesListQuery do not reset self._joined_tables on recalling make_query, which result into them thinking that tables were already joined and not joining related tables on second run

Should be not hard fix, but still can't do it earlier than next week
I'll also think if we may be can come up with kind of more robust immutable approach for queries, as such error can be sneaky in mutable queries

@spacemanspiff2007
Copy link
Contributor Author

@abondar: Do you already have an idea for a robust immutable approach or a fix?

@abondar
Copy link
Member

abondar commented Jul 3, 2024

@spacemanspiff2007 Hi!

Sorry, I got distracted and didn't release fix for it in time

I now released 0.21.4 that includes fix for this issue

@abondar abondar closed this as completed Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants