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

Bug: queries using cached argument values #120

Open
1 of 4 tasks
LonelyVikingMichael opened this issue Jan 16, 2024 · 2 comments · Fixed by #128
Open
1 of 4 tasks

Bug: queries using cached argument values #120

LonelyVikingMichael opened this issue Jan 16, 2024 · 2 comments · Fixed by #128
Labels
bug Something isn't working lambda-stmt Related to `lambda_stmt` in SQLAlchemy. Not necessarily an `advanced-alchemy` bug.

Comments

@LonelyVikingMichael
Copy link

LonelyVikingMichael commented Jan 16, 2024

Description

I've tried the following with both the get_one and get_one_or_none methods, in the context of a Litestar dev (local) environment.
The first query from my user table with an arbitrary kwarg results in fetching the correct result. Querying for a different user thereafter results in SQLAlchemy using a cached argument value. The log below might explain this better:

# initial query
# await user_repository.get_one(id_number="1234567890")

2024-01-16 09:38:45,451 INFO sqlalchemy.engine.Engine SELECT "user".name
FROM "user" 
WHERE "user".id_number = $1::VARCHAR
2024-01-16 09:38:45,453 INFO sqlalchemy.engine.Engine [generated in 0.00203s] ('1234567890',)
------
# second query
# await user_repository.get_one(id_number="789")

2024-01-16 09:41:39,580 INFO sqlalchemy.engine.Engine SELECT "user".name
FROM "user" 
WHERE "user".id_number = $1::VARCHAR
2024-01-16 09:41:39,581 INFO sqlalchemy.engine.Engine [cached since 174.1s ago] ('1234567890',)

This is across two separate requests with their own AsyncSession instances.

Overriding the method with my own select() resolves the issue.

I don't have a MCVE as this is all part of a greater project, I'll work on something

URL to code causing the issue

No response

MCVE

No response

Steps to reproduce

No response

Screenshots

No response

Logs

No response

Jolt Project Version

0.6.2

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Funding

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@LonelyVikingMichael LonelyVikingMichael added the bug Something isn't working label Jan 16, 2024
@cofin
Copy link
Member

cofin commented Jan 16, 2024

I have a feeling this is going to be related to the lambda_stmt

@LonelyVikingMichael
Copy link
Author

I've finally found the trigger. I've got a listens_for event handler that's modifying query statements - MCVE below

from uuid import UUID

from advanced_alchemy import SQLAlchemySyncRepository
from advanced_alchemy.base import UUIDBase
from sqlalchemy.orm import Mapped, Session, sessionmaker, declarative_mixin, with_loader_criteria, mapped_column
from sqlalchemy import create_engine, Boolean
from sqlalchemy.event import listens_for


@declarative_mixin
class SoftDeleteMixin:
    is_deleted: Mapped[bool] = mapped_column(Boolean())


class User(UUIDBase, SoftDeleteMixin):
    name: Mapped[str]


class UserRepository(SQLAlchemySyncRepository[User]):
    model_type = User


FOO_ID = UUID('ff111ee7-bbf1-4826-93d6-88153765e8b3')
BAR_ID = UUID('ba14ebd3-ac67-4781-ad14-3db91fece8fd')


engine = create_engine("sqlite:///:memory:")
session_factory: sessionmaker[Session] = sessionmaker(engine, expire_on_commit=False)

@listens_for(Session, "do_orm_execute")
def _do_orm_execute(orm_execute_state):
    """Modifies queries globally based on rulesets."""

    # this rule ensures that records won't load if they're soft-deleted
    # unless execution options are set otherwise
    if (
        orm_execute_state.is_select
        and not orm_execute_state.is_column_load
        and not orm_execute_state.is_relationship_load
        and not orm_execute_state.execution_options.get("include_deleted", False)
    ):
        orm_execute_state.statement = orm_execute_state.statement.options(
            with_loader_criteria(SoftDeleteMixin, lambda cls: cls.is_deleted.is_(False), include_aliases=True)
        )


with engine.begin() as conn:
    User.metadata.create_all(conn)

with session_factory() as session:
    repository = UserRepository(session=session)
    repository.add_many(
        [
            User(id=FOO_ID, name="foo", is_deleted=False),
            User(id=BAR_ID, name="bar", is_deleted=False)
        ]
    )
    session.commit()

    foo_user = repository.get_one(name="foo")
    assert foo_user.name == "foo"

    foo_user = repository.get(FOO_ID)
    assert foo_user.name == "foo"

    bar_user = repository.get_one(name="bar")
    assert bar_user.name == "bar"

    bar_user = repository.get(BAR_ID)
    assert bar_user.name == "bar"

@cofin cofin reopened this Feb 6, 2024
@cofin cofin added the lambda-stmt Related to `lambda_stmt` in SQLAlchemy. Not necessarily an `advanced-alchemy` bug. label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lambda-stmt Related to `lambda_stmt` in SQLAlchemy. Not necessarily an `advanced-alchemy` bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants