Add support for SQLAlchemy 2.0 #488

amotl · 2022-12-21T01:41:27Z

Based on #485 and #498, this patch concludes compatibility work with SQLAlchemy 2.0 ¹² (GH-465) 🎉.

src/crate/client/sqlalchemy/dialect.py

amotl · 2022-12-21T22:45:06Z

src/crate/client/sqlalchemy/tests/bulk_test.py

-        self.assertEqual(expected_bulk_args, bulk_args)
+        self.assertSequenceEqual(expected_bulk_args, bulk_args)


With SA<2.0, bulk_args is a tuple, so expected_bulk_args was written as such. Now, with SA>=2.0, it is a list. Because this only concerns the call arguments to cursor.executemany(), which is invoked by SQLAlchemy, I think we do not need to put any kind of admonition notice about a breaking change into the changelog, as it looks like it will not concern the user of the library at all.

Please let me know if you think differently.

src/crate/client/sqlalchemy/tests/compiler_test.py

src/crate/client/sqlalchemy/compat/core20.py

src/crate/client/sqlalchemy/tests/dict_test.py

mfussenegger · 2023-01-09T06:59:31Z

src/crate/client/sqlalchemy/compat/core20.py

+        # [20] CrateDB patch end.
+
+        """
+        # TODO: Complete SA20 migration.


Shouldn't this TODO get addressed with this PR?

Hi. Thanks for spotting this leftover comment. I've spent another day on it, in order to mangle the parameter data into a shape that would be accepted by the new cast function, but wasn't able to make a cut.

So, I've just created GH-508 in order to track this issue, and eventually resolve it on behalf of a subsequent iteration, and to unblock this patch. Any help on this topic is very welcome.

mfussenegger · 2023-01-09T07:00:01Z

src/crate/client/sqlalchemy/compat/core20.py

+    #
+    #   sqlalchemy.exc.CompileError: Unconsumed column names: characters_name
+    #
+    # TODO: Investigate why this is actually happening and eventually mitigate


This is an issue we are carrying over since the initial implementation of the CrateDB SQLAlchemy dialect. I've also tried to remedy it a few times, but did not manage to improve it yet.

At least, I've created GH-509 now, in order to highlight this spot more prominently. I'd also recommend to investigate it on behalf on a subsequent iteration, in order not to block this patch. Any help on this will be well received.

References: - `sqlalchemy.sql.compiler.SQLCompiler.visit_update` - `sqlalchemy.sql.crud._get_crud_params` Vendored from SQLAlchemy 2.0.0b4, released on Dec 6, 2022.

This effectively concludes the support for SQLAlchemy 2.0, by implementing the same strategy as for the previous versions: After vendoring the vanilla dialect's compiler's `visit_update` and `_get_crud_params` methods, they are patched at a few spots to accommodate CrateDB's features. The changed behavior is mostly for running update statements on nested `OBJECT` or `ARRAY` data types.

Currently, this is 2.0.0b4.

Apparently, `PGCompiler` does not have a specific implementation for `returning_clause`, starting with SA20. On the other hand, the generic implementation changed its method signature, that's why CodeQL would croak otherwise.

mfussenegger · 2023-01-30T07:08:04Z

bootstrap.sh

@@ -70,7 +70,7 @@ function setup_package() {
      if [ "${SQLALCHEMY_VERSION}" = "latest" ]; then
        pip install "sqlalchemy" --upgrade
      else
-        pip install "sqlalchemy==${SQLALCHEMY_VERSION}"
+        pip install "sqlalchemy==${SQLALCHEMY_VERSION}" --pre


Is the --pre still required?

Thanks for spotting. I don't think so, so I've updated 71a7f0a correspondingly.

mfussenegger · 2023-01-30T07:09:58Z

src/crate/client/sqlalchemy/sa_version.py

@@ -25,3 +25,4 @@
 SA_VERSION = V(sa.__version__)

 SA_1_4 = V('1.4.0b1')
+SA_2_0 = V('2.0.0b1')


Suggested change

SA_2_0 = V('2.0.0b1')

SA_2_0 = V('2.0.0')

Thanks. See also 71a7f0a.

amotl force-pushed the amo/sa20 branch 2 times, most recently from c3ec061 to ad595a5 Compare December 21, 2022 01:49

amotl mentioned this pull request Dec 21, 2022

SA20: Fix SqlAlchemy tests to use "qmark" paramstyle again #490

Merged

amotl commented Dec 21, 2022

View reviewed changes

amotl force-pushed the amo/sa-mitigate-deprecation-warnings branch from 386e246 to 7ef37c1 Compare December 21, 2022 23:52

amotl force-pushed the amo/sa20 branch from ad595a5 to a8b5d68 Compare December 22, 2022 01:44

amotl force-pushed the amo/sa-mitigate-deprecation-warnings branch from 1429658 to a4454d5 Compare December 22, 2022 12:36

This was referenced Jun 16, 2024

SQLAlchemy: Maintenance support for SQLAlchemy 1.3 crate/sqlalchemy-cratedb#108

Closed

SQLAlchemy 2 RC is out. Help by testing on your project! sqlalchemy/sqlalchemy#8631

Closed

amotl force-pushed the amo/sa-mitigate-deprecation-warnings branch from fe32a5e to b177b1e Compare December 23, 2022 23:44

amotl force-pushed the amo/sa20 branch 3 times, most recently from 5b21d71 to caf6b19 Compare December 24, 2022 00:14

amotl force-pushed the amo/sa-mitigate-deprecation-warnings branch from b177b1e to be93172 Compare December 25, 2022 00:26

amotl mentioned this pull request Dec 25, 2022

Refactoring: Modularize CrateCompiler #498

Merged

amotl force-pushed the amo/sa20 branch from caf6b19 to efb0e90 Compare December 25, 2022 13:53

amotl changed the base branch from amo/sa-mitigate-deprecation-warnings to master December 25, 2022 13:54

amotl changed the title ~~Add support for SQLAlchemy 2.0~~ [STACK] Add support for SQLAlchemy 2.0 Dec 25, 2022

amotl force-pushed the amo/sa20 branch from efb0e90 to 7fbfc07 Compare December 25, 2022 13:54

github-advanced-security bot found potential problems Dec 25, 2022

View reviewed changes

src/crate/client/sqlalchemy/compat/core20.py Fixed Show fixed Hide fixed

src/crate/client/sqlalchemy/compat/core20.py Fixed Show fixed Hide fixed

amotl force-pushed the amo/sa20 branch from 7fbfc07 to a77a32c Compare December 27, 2022 11:38

amotl changed the base branch from master to amo/sa-mitigate-deprecation-warnings December 27, 2022 11:38

github-advanced-security bot found potential problems Dec 27, 2022

View reviewed changes

src/crate/client/sqlalchemy/compat/core20.py Fixed Show fixed Hide fixed

src/crate/client/sqlalchemy/compat/core20.py Fixed Show fixed Hide fixed

amotl force-pushed the amo/sa20 branch from a77a32c to c05d4a2 Compare December 27, 2022 12:03

amotl force-pushed the amo/sa-mitigate-deprecation-warnings branch from 639636f to 19dbf7d Compare December 29, 2022 13:09

Base automatically changed from amo/sa-mitigate-deprecation-warnings to master December 30, 2022 10:24

amotl force-pushed the amo/sa20 branch 2 times, most recently from 0bdf9f1 to f6f1bd3 Compare December 30, 2022 10:52

amotl changed the title ~~[STACK] Add support for SQLAlchemy 2.0~~ Add support for SQLAlchemy 2.0 Dec 30, 2022

amotl force-pushed the amo/sa20 branch from f6f1bd3 to b458220 Compare December 30, 2022 10:56

github-advanced-security bot found potential problems Dec 30, 2022

View reviewed changes

src/crate/client/sqlalchemy/tests/dict_test.py Outdated Show resolved Hide resolved

amotl force-pushed the amo/sa20 branch 3 times, most recently from 89d5d57 to 064d57f Compare January 2, 2023 17:56

amotl requested review from mfussenegger, hammerhead and seut January 2, 2023 18:07

amotl marked this pull request as ready for review January 2, 2023 18:07

amotl mentioned this pull request Jan 27, 2023

SA20: Evaluate compatibility with SQLAlchemy 2.0 crate/sqlalchemy-cratedb#109

Closed

mfussenegger reviewed Jan 9, 2023

View reviewed changes

amotl force-pushed the amo/sa20 branch from 064d57f to e8c633f Compare January 27, 2023 10:15

amotl mentioned this pull request Jan 27, 2023

SA20: AttributeError: can't set attribute 'returning' crate/sqlalchemy-cratedb#110

Closed

This comment was marked as outdated.

Sign in to view

amotl added 6 commits January 27, 2023 12:24

SA20: Add vanilla versions of SA20's visit_update and _get_crud_params

454ad9b

References: - `sqlalchemy.sql.compiler.SQLCompiler.visit_update` - `sqlalchemy.sql.crud._get_crud_params` Vendored from SQLAlchemy 2.0.0b4, released on Dec 6, 2022.

SA20: Adjust compatible SQLAlchemy versions: <2.1

34fe76b

SA20: Add SQLAlchemy 2.x to CI build matrix

4698002

Currently, this is 2.0.0b4.

SA20: Mitigate some leftover incompatibilities

ee6144d

amotl force-pushed the amo/sa20 branch from e8c633f to 2ee91d5 Compare January 27, 2023 11:27

This was referenced Jun 16, 2024

SQLAlchemy: Investigate CompileError: Unconsumed column names crate/sqlalchemy-cratedb#78

Open

SQLAlchemy 2: Type support for patch to visit_update crate/sqlalchemy-cratedb#84

Open

amotl requested a review from mfussenegger January 27, 2023 17:42

mfussenegger approved these changes Jan 30, 2023

View reviewed changes

amotl added 2 commits January 30, 2023 08:36

SA20: Update to SQLAlchemy 2.0.0

71a7f0a

SA20: Remove leftover backlog comment

2a78044

amotl force-pushed the amo/sa20 branch from 4fc4461 to 2a78044 Compare January 30, 2023 07:36

amotl merged commit 4327b97 into master Feb 1, 2023

amotl deleted the amo/sa20 branch February 1, 2023 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for SQLAlchemy 2.0 #488

Add support for SQLAlchemy 2.0 #488

amotl commented Dec 21, 2022 •

edited

Loading

amotl Dec 21, 2022 •

edited

Loading

mfussenegger Jan 9, 2023

amotl Jan 27, 2023 •

edited

Loading

mfussenegger Jan 9, 2023

amotl Jan 27, 2023 •

edited

Loading

This comment was marked as outdated.

mfussenegger Jan 30, 2023

amotl Jan 30, 2023

mfussenegger Jan 30, 2023

amotl Jan 30, 2023

		self.assertEqual(expected_bulk_args, bulk_args)
		self.assertSequenceEqual(expected_bulk_args, bulk_args)

Add support for SQLAlchemy 2.0 #488

Add support for SQLAlchemy 2.0 #488

Conversation

amotl commented Dec 21, 2022 • edited Loading

Footnotes

amotl Dec 21, 2022 • edited Loading

Choose a reason for hiding this comment

mfussenegger Jan 9, 2023

Choose a reason for hiding this comment

amotl Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

mfussenegger Jan 9, 2023

Choose a reason for hiding this comment

amotl Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

This comment was marked as outdated.

mfussenegger Jan 30, 2023

Choose a reason for hiding this comment

amotl Jan 30, 2023

Choose a reason for hiding this comment

mfussenegger Jan 30, 2023

Choose a reason for hiding this comment

amotl Jan 30, 2023

Choose a reason for hiding this comment

amotl commented Dec 21, 2022 •

edited

Loading

amotl Dec 21, 2022 •

edited

Loading

amotl Jan 27, 2023 •

edited

Loading

amotl Jan 27, 2023 •

edited

Loading