Skip to content

Remove Legacy Query Object Usage from airflow-models - #52325

Merged
uranusjr merged 36 commits into
apache:mainfrom
Prab-27:remove-session-query-from-airflow-models
Sep 2, 2025
Merged

uranusjr merged 36 commits into
apache:mainfrom
Prab-27:remove-session-query-from-airflow-models

Conversation

@Prab-27

@Prab-27 Prab-27 commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

Related discussion: here

This PR is part of the migration to SQLAlchemy 2.0

Replaced instances of session.query() with select() constructs
Updated methods like XComModel.get_many to return Select objects instead of Query objects
Adjusted dependent code to reflect the updated return types


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@jason810496 jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Could you help to make the CI green?

@Prab-27

Prab-27 commented Jun 30, 2025

Copy link
Copy Markdown
Contributor Author

Converting into draft until green checks

@Prab-27
Prab-27 marked this pull request as draft June 30, 2025 14:39
@Prab-27
Prab-27 force-pushed the remove-session-query-from-airflow-models branch from 67e0627 to bc302b0 Compare June 30, 2025 14:42
@Prab-27
Prab-27 force-pushed the remove-session-query-from-airflow-models branch from 4910c67 to bd99c84 Compare July 5, 2025 17:41
@Prab-27
Prab-27 force-pushed the remove-session-query-from-airflow-models branch 2 times, most recently from 335c872 to 34b3863 Compare July 21, 2025 17:10
@Prab-27
Prab-27 force-pushed the remove-session-query-from-airflow-models branch 4 times, most recently from 3fe6087 to 00158d5 Compare August 3, 2025 03:49
@Prab-27
Prab-27 force-pushed the remove-session-query-from-airflow-models branch from 98ed7a3 to 432672c Compare September 2, 2025 12:24
@uranusjr
uranusjr merged commit 2357f96 into apache:main Sep 2, 2025
1 check passed
@jason810496

Copy link
Copy Markdown
Member

#protm

Thank you again for your consistent work @Prab-27.

@Prab-27

Prab-27 commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

Thanks a lot for reviewing this PR—I'm working on the next step and excited to contribute more!

@Prab-27

Prab-27 commented Sep 3, 2025 •

Copy link
Copy Markdown
Contributor Author

One remaining Query-related section—I'll address it soon

pierrejeambrun added a commit to astronomer/airflow that referenced this pull request Sep 8, 2026
Two more callers of ``session.scalars(...).first()`` /
``session.execute(select(...)).first()`` were still missing an explicit
``.limit(1)``. Because SQLAlchemy 2.0's ``Result.first()`` does not add
one (unlike the legacy ``Query.first``), each call sends every matching
row to the Python process and discards all but the first. The favorite
existence check is bounded to a single row per (dag_id, user_id) by the
schema, but the previous-TI lookup scans every earlier matching TI.

Introduced independently of apache#52325's migration: get_previous_task_instance
came in apache#59712, unfavorite_dag's existence probe in apache#51264. Same
anti-pattern as the four sites apache#72554 fixes; the tests reuse the
``capture_orm_selects`` helper that PR introduces.
pierrejeambrun added a commit to astronomer/airflow that referenced this pull request Sep 8, 2026
Two more callers of ``session.scalars(...).first()`` /
``session.execute(select(...)).first()`` were still missing an explicit
``.limit(1)``. Because SQLAlchemy 2.0's ``Result.first()`` does not add
one (unlike the legacy ``Query.first``), each call sends every matching
row to the Python process and discards all but the first. The favorite
existence check is bounded to a single row per (dag_id, user_id) by the
schema, but the previous-TI lookup scans every earlier matching TI.

Introduced independently of apache#52325's migration: get_previous_task_instance
came in apache#59712, unfavorite_dag's existence probe in apache#51264. Same
anti-pattern as the four sites apache#72554 fixes; the tests reuse the
``capture_orm_selects`` helper that PR introduces.
pierrejeambrun added a commit to astronomer/airflow that referenced this pull request Sep 9, 2026
Two more callers of ``session.scalars(...).first()`` /
``session.execute(select(...)).first()`` were still missing an explicit
``.limit(1)``. Because SQLAlchemy 2.0's ``Result.first()`` does not add
one (unlike the legacy ``Query.first``), each call sends every matching
row to the Python process and discards all but the first. The favorite
existence check is bounded to a single row per (dag_id, user_id) by the
schema, but the previous-TI lookup scans every earlier matching TI.

Introduced independently of apache#52325's migration: get_previous_task_instance
came in apache#59712, unfavorite_dag's existence probe in apache#51264. Same
anti-pattern as the four sites apache#72554 fixes; the tests reuse the
``capture_orm_selects`` helper that PR introduces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants