Repository navigation
Conversation
11918b4 to
05a55da
Compare
|
Hey @Dev-iL 👋 |
| - 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
You already saw it, but linking just in case anyone else didn't see it: #652 (comment).
89151b4 to
dacfda9
Compare
e9427d8 to
00191c5
Compare
00191c5 to
a69fe9c
Compare
a69fe9c to
16f5a47
Compare
Please answer these questions before submitting your pull requests. Thanks!
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
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
compat.pyIS_VERSION_21flag for preserving backward compatibility with SQLA 1.4 and 2.0._ORMSelectCompileState(private in 2.1) and re-exported asORMSelectCompileState.base.pySnowflakeORMSelectCompileStatenow inherits fromcompat.ORMSelectCompileState.SnowflakeSelectState._setup_joins: ported theexplicit_leftpattern from SQLA 2.1'sSelectState._setup_joins.SnowflakeORMSelectCompileState._join_left_to_right: ported theexplicit_leftpattern from SQLA 2.1's_ORMSelectCompileState._join_left_to_rightusing the appropriate snowflake classes.util.py_joined_from_info/ string onclause code behindnot IS_VERSION_21(since removed in SQLA 2.1)augment_onclauselogic with SQLA 2.1's upstream bug fixNote
Medium Risk
Changes ORM/core select join compilation paths on SQLAlchemy 2.1+; mistakes could affect join SQL generation while 2.0 behavior stays gated behind version checks.
Overview
Adds bare-bones SQLAlchemy 2.1 support: the default Hatch dev environment now pulls SQLAlchemy ≥2.1.0rc2 (with pre-release installs enabled), and unreleased notes document the change.
In
base.py, anIS_VERSION_21gate selects the correct ORM compile-state base class (_ORMSelectCompileStateon 2.1 vs publicORMSelectCompileStateon 2.0).SnowflakeSelectState._setup_joinsandSnowflakeORMSelectCompileState._join_left_to_rightmirror SQLA 2.1’sexplicit_leftbehavior so joins with an explicit left but noONcan auto-derive the condition via_join_condition, while still allowingNoneonclause forLateraljoins (Snowflake BCR-1057).Reviewed by Cursor Bugbot for commit 16f5a47. Bugbot is set up for automated code reviews on this repo. Configure here.