Complete migration away from legacy SQLAlchemy Query API#156
Complete migration away from legacy SQLAlchemy Query API#156winwinashwin merged 3 commits intomainfrom
Conversation
|
@ashish16052 The How should we go about this? Related thread in sqlalchemy-continuum - sqlalchemy-continuum/sqlalchemy-continuum#332 |
one way to go about this is making lazy strategy configurable. and add a deprecation warning. this will: |
winwinashwin
force-pushed
the
ashwin/migrate-query
branch
from
98950f5 to
7d37edc
Compare
Move away from the query API (which is deemed legacy in 2.x) and use the newer calling form. This will also help us in the long run to support async sessions as async sessions do not work with the Query API. See: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#migration-orm-usage See: sqlalchemy/sqlalchemy#7971 (comment)
winwinashwin
force-pushed
the
ashwin/migrate-query
branch
from
7d37edc to
276ba06
Compare
In sqla 2.0, lazy='dynamic' is superseded by lazy='write_only' strategy. The relationship builder did not handle write_only strategy correctly and these relationships in the version objects were eagerly loaded from the DB. 1. Implement a minimal adapter that quacks like sa.orm.writeonly.WriteOnlyCollection and expose a select(...) method. 2. Refactor the builder to use the sqla select clauses instead of legacy Query API 3. Maintain backward compatibility for lazy=dynamic as it still depends on the Query object internally in sqlalchemy. Add a deprecation warning for users to migrate to write_only from dynamic 4. Add test cases for write_only relationships
code changes by cline, manually reviewed.
winwinashwin
force-pushed
the
ashwin/migrate-query
branch
from
276ba06 to
c4b0708
Compare
|
Let's keep the lazy strategy for versions to be |
| if self.property.uselist is False: | ||
| return query.first() | ||
| return query.all() | ||
| return session.scalars(query.limit(1)).first() |
There was a problem hiding this comment.
not clear on why was this limit required?
There was a problem hiding this comment.
session.scalars(query).first() would not work?
There was a problem hiding this comment.
I referred to the migration guide.
The query object used to apply a limit on the statement internally which the new form doesn't do.
There will be a performance regression in the newer form if it is not explicitly applied as all the rows will be returned from the DB but will be discarded in ORM layer.
Refer to the note for more details
2 participants
This PR removes all remaining references to the legacy SQLAlchemy Query API and standardizes on 2.0-style select()-based querying across code, tests, and documentation.
Majority of the query patterns are based off of SQLAlchemy migration guide.
This will also help us long term if we need to add support for async sessions as async extension cannot work with the Query object and related constructs.
Summary of changes
Fixes: #155
See: https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#orm-query-unified-with-core-select