Skip to content

SNOW-3121122: Bare bones support for SQLA2.1#654

Open
Dev-iL wants to merge 15 commits into
snowflakedb:mainfrom
Dev-iL:2602/sqla_2.1_bb
Open

SNOW-3121122: Bare bones support for SQLA2.1#654
Dev-iL wants to merge 15 commits into
snowflakedb:mainfrom
Dev-iL:2602/sqla_2.1_bb

Conversation

@Dev-iL

@Dev-iL Dev-iL commented Feb 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes (?) SNOW-3121122: Meta: Support for SQLAlchemy 2.1 #652

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding new credentials
    • I am adding a new dependency

  3. Please describe how your code solves the related issue.

    • Added SQLA2.1 to CI.
    • compat.py
      • Added a IS_VERSION_21 flag for preserving backward compatibility with SQLA 1.4 and 2.0.
      • Import _ORMSelectCompileState (private in 2.1) and re-exported as ORMSelectCompileState.
    • base.py
      • SnowflakeORMSelectCompileState now inherits from compat.ORMSelectCompileState.
      • SnowflakeSelectState._setup_joins: ported the explicit_left pattern from SQLA 2.1's SelectState._setup_joins.
      • SnowflakeORMSelectCompileState._join_left_to_right: ported the explicit_left pattern from SQLA 2.1's _ORMSelectCompileState._join_left_to_right using the appropriate snowflake classes.
    • util.py
      • Guarded legacy _joined_from_info / string onclause code behind not IS_VERSION_21 (since removed in SQLA 2.1)
      • Synced augment_onclause logic with SQLA 2.1's upstream bug fix

@Dev-iL Dev-iL requested a review from a team as a code owner February 21, 2026 15:01
@Dev-iL Dev-iL force-pushed the 2602/sqla_2.1_bb branch 3 times, most recently from 11918b4 to 05a55da Compare February 22, 2026 07:51
@sfc-gh-jcieslak

sfc-gh-jcieslak commented Feb 24, 2026

Copy link
Copy Markdown
Member

Hey @Dev-iL 👋
Thanks again for the contribution. We’re currently focused on a few ongoing topics, but we should be able to review it within the next few days. Apologies for the delay.

@Dev-iL Dev-iL mentioned this pull request Feb 25, 2026
Draft
4 tasks
sfc-gh-jcieslak
sfc-gh-jcieslak reviewed Mar 10, 2026
View reviewed changes
Comment thread DESCRIPTION.md
- Bump `pandas` lower bound in `sa14` test environment from `<2.1` to `>=2.1.1,<2.2` to ensure pre-built wheels are available for Python 3.12
- Fix SQLAlchemy version parsing (SNOW-3066571)
- Document support for session parameters (like [QUERY_TAG](https://docs.snowflake.com/en/sql-reference/parameters#query-tag)), references: [#644](https://github.com/snowflakedb/snowflake-sqlalchemy/issues/495)
- Add bare bones support for SQLA2.1

@sfc-gh-jcieslak sfc-gh-jcieslak Mar 10, 2026

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.

High-level, the change looks okay, but I have to ask: were the changes to implement some of the features necessary for the tests to pass, or did you just add them along with the support change? If the latter, I would split this PR into two as they serve different purposes. Still, I would wait for @sfc-gh-mraba to look at the PR.

@Dev-iL Dev-iL Mar 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe the changes are indeed the minimum necessary for the tests to pass. My approach was very TDD: create a (failing) CI job, then fix incrementally.

Is there any particular change you're concerned about?

@sfc-gh-jcieslak sfc-gh-jcieslak Mar 11, 2026 •
edited
Loading

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.

You already saw it, but linking just in case anyone else didn't see it: #652 (comment).

@Dev-iL Dev-iL force-pushed the 2602/sqla_2.1_bb branch from 05a55da to b62ba40 Compare June 10, 2026 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Dev-iL @sfc-gh-jcieslak @sfc-gh-mraba