From d64ecc957f4c949a84072ea9189ef5ab3876eef5 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Mon, 16 Nov 2020 15:15:27 +0100 Subject: [PATCH 01/19] SA14/CI: Add SQLAlchemy 1.4 to the test matrix --- .github/workflows/tests.yml | 2 +- DEVELOP.rst | 7 ++++++- docs/sqlalchemy.rst | 2 +- setup.py | 2 +- tox.ini | 3 ++- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 621d6850..0512a7c9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,7 +19,7 @@ jobs: os: [ubuntu-latest, macos-latest] python-version: ['3.7', '3.8', '3.9', '3.10'] cratedb-version: ['4.8.0'] - sqla-version: ['1.3.24'] + sqla-version: ['1.3.24', '1.4.36'] fail-fast: true steps: diff --git a/DEVELOP.rst b/DEVELOP.rst index cdd64551..d3d7efd4 100644 --- a/DEVELOP.rst +++ b/DEVELOP.rst @@ -33,7 +33,12 @@ Run all tests:: Run specific tests:: - # Ignore all tests below src/crate/testing + ./bin/test -vvvv -t SqlAlchemyCompilerTest + ./bin/test -vvvv -t test_score + ./bin/test -vvvv -t sqlalchemy + +Ignore specific test directories:: + ./bin/test -vvvv --ignore_dir=testing You can run the tests against multiple Python interpreters with tox_:: diff --git a/docs/sqlalchemy.rst b/docs/sqlalchemy.rst index ee0be079..9ee38cf0 100644 --- a/docs/sqlalchemy.rst +++ b/docs/sqlalchemy.rst @@ -11,7 +11,7 @@ The CrateDB Python client library provides support for SQLAlchemy. A CrateDB configuration. The CrateDB Python client library is validated to work with SQLAlchemy versions -``1.3``. +``1.3`` and ``1.4``. .. NOTE:: diff --git a/setup.py b/setup.py index 6bb237de..28eea262 100644 --- a/setup.py +++ b/setup.py @@ -67,7 +67,7 @@ def read(path): test=['zope.testing>=4,<5', 'zc.customdoctests>=1.0.1,<2', 'stopit>=1.1.2,<2'], - sqlalchemy=['sqlalchemy>=1.0,<1.4', 'geojson>=2.5.0'] + sqlalchemy=['sqlalchemy>=1.0,<1.5', 'geojson>=2.5.0'] ), python_requires='>=3.4', install_requires=requirements, diff --git a/tox.ini b/tox.ini index 7a5ba805..fa7995bc 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{py3,35,36,37,38,39}-sa_{1_0,1_1,1_2,1_3} +envlist = py{py3,35,36,37,38,39}-sa_{1_0,1_1,1_2,1_3,1_4} [testenv] usedevelop = True @@ -12,6 +12,7 @@ deps = sa_1_1: sqlalchemy>=1.1,<1.2 sa_1_2: sqlalchemy>=1.2,<1.3 sa_1_3: sqlalchemy>=1.3,<1.4 + sa_1_4: sqlalchemy>=1.4,<1.5 mock urllib3 commands = From 1d9e151aea8c5c62a61f02fe816e5967d2e27d14 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 18 Dec 2020 01:18:57 +0100 Subject: [PATCH 02/19] SA14: Add `get_view_names` method to SQLAlchemy dialect --- src/crate/client/sqlalchemy/dialect.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 42eb4961..46e3b7ad 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -237,6 +237,15 @@ def get_table_names(self, connection, schema=None, **kw): ) return [row[0] for row in cursor.fetchall()] + @reflection.cache + def get_view_names(self, connection, schema=None, **kw): + cursor = connection.execute( + "SELECT table_name FROM information_schema.views " + "ORDER BY table_name ASC, {0} ASC".format(self.schema_column), + [schema or self.default_schema_name] + ) + return [row[0] for row in cursor.fetchall()] + @reflection.cache def get_columns(self, connection, table_name, schema=None, **kw): query = "SELECT column_name, data_type " \ From 62fd1827e55001684c7500db61fca93ebba33fbd Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Mon, 16 Nov 2020 20:29:12 +0100 Subject: [PATCH 03/19] SA14: Add SQLAlchemy 1.4 compatibility for CrateCompiler - Code reuse was aimed at, but for the SA <1.4 vs. SA >=1.4 split, two functions, `visit_update_14` and `_get_crud_params_14`, have been vendored separately to accompany `crate.client.sqlalchemy.compiler.CrateCompiler`. All adjustments have now been marked inline with `CrateDB amendment`. - The main query rewriting function for UPDATE statements, `rewrite_update`, needed adjustments to account for a different wrapping/nesting of in/out parameters. - The `cresultproxy` module was temporarily taken out of the equation because it raised some runtime error. --- CHANGES.txt | 2 + src/crate/client/sqlalchemy/compiler.py | 317 +++++++++++++++++++++- src/crate/client/sqlalchemy/dialect.py | 5 + src/crate/client/sqlalchemy/monkey.py | 44 +++ src/crate/client/sqlalchemy/sa_version.py | 2 + 5 files changed, 367 insertions(+), 3 deletions(-) create mode 100644 src/crate/client/sqlalchemy/monkey.py diff --git a/CHANGES.txt b/CHANGES.txt index 7d754247..06fb7616 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -22,6 +22,8 @@ Unreleased - Added support for enabling SSL using SQLAlchemy DB URI with parameter ``?ssl=true``. +- Add support for SQLAlchemy 1.4 + 2020/09/28 0.26.0 ================= diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 03347a27..62c92037 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -23,9 +23,10 @@ from collections import defaultdict import sqlalchemy as sa -from sqlalchemy.sql import crud +from sqlalchemy.sql import crud, selectable from sqlalchemy.sql import compiler from .types import MutableDict +from .sa_version import SA_VERSION, SA_1_4 def rewrite_update(clauseelement, multiparams, params): @@ -73,7 +74,16 @@ def rewrite_update(clauseelement, multiparams, params): def crate_before_execute(conn, clauseelement, multiparams, params): is_crate = type(conn.dialect).__name__ == 'CrateDialect' if is_crate and isinstance(clauseelement, sa.sql.expression.Update): - return rewrite_update(clauseelement, multiparams, params) + if SA_VERSION >= SA_1_4: + multiparams = ([params],) + params = {} + + clauseelement, multiparams, params = rewrite_update(clauseelement, multiparams, params) + + if SA_VERSION >= SA_1_4: + params = multiparams[0] + multiparams = [] + return clauseelement, multiparams, params @@ -189,6 +199,9 @@ def visit_update(self, update_stmt, **kw): Parts are taken from the SQLCompiler base class. """ + if SA_VERSION >= SA_1_4: + return self.visit_update_14(update_stmt, **kw) + if not update_stmt.parameters and \ not hasattr(update_stmt, '_crate_specific'): return super(CrateCompiler, self).visit_update(update_stmt, **kw) @@ -212,11 +225,14 @@ def visit_update(self, update_stmt, **kw): update_stmt, table_text ) + # CrateDB amendment. crud_params = self._get_crud_params(update_stmt, **kw) text += table_text text += ' SET ' + + # CrateDB amendment begin. include_table = extra_froms and \ self.render_table_with_column_in_update_from @@ -234,6 +250,7 @@ def visit_update(self, update_stmt, **kw): set_clauses.append(k + ' = ' + self.process(bindparam)) text += ', '.join(set_clauses) + # CrateDB amendment end. if self.returning or update_stmt._returning: if not self.returning: @@ -269,7 +286,6 @@ def visit_update(self, update_stmt, **kw): def _get_crud_params(compiler, stmt, **kw): """ extract values from crud parameters - taken from SQLAlchemy's crud module (since 1.0.x) and adapted for Crate dialect""" @@ -325,3 +341,298 @@ def _get_crud_params(compiler, stmt, **kw): values, kw) return values + + def visit_update_14(self, update_stmt, **kw): + + compile_state = update_stmt._compile_state_factory( + update_stmt, self, **kw + ) + update_stmt = compile_state.statement + + toplevel = not self.stack + if toplevel: + self.isupdate = True + if not self.compile_state: + self.compile_state = compile_state + + extra_froms = compile_state._extra_froms + is_multitable = bool(extra_froms) + + if is_multitable: + # main table might be a JOIN + main_froms = set(selectable._from_objects(update_stmt.table)) + render_extra_froms = [ + f for f in extra_froms if f not in main_froms + ] + correlate_froms = main_froms.union(extra_froms) + else: + render_extra_froms = [] + correlate_froms = {update_stmt.table} + + self.stack.append( + { + "correlate_froms": correlate_froms, + "asfrom_froms": correlate_froms, + "selectable": update_stmt, + } + ) + + text = "UPDATE " + + if update_stmt._prefixes: + text += self._generate_prefixes( + update_stmt, update_stmt._prefixes, **kw + ) + + table_text = self.update_tables_clause( + update_stmt, update_stmt.table, render_extra_froms, **kw + ) + + # CrateDB amendment. + crud_params = _get_crud_params_14( + self, update_stmt, compile_state, **kw + ) + + if update_stmt._hints: + dialect_hints, table_text = self._setup_crud_hints( + update_stmt, table_text + ) + else: + dialect_hints = None + + text += table_text + + text += " SET " + + # CrateDB amendment begin. + include_table = extra_froms and \ + self.render_table_with_column_in_update_from + + set_clauses = [] + + for c, expr, value in crud_params: + key = c._compiler_dispatch(self, include_table=include_table) + clause = key + ' = ' + value + set_clauses.append(clause) + + for k, v in compile_state._dict_parameters.items(): + if isinstance(k, str) and '[' in k: + bindparam = sa.sql.bindparam(k, v) + clause = k + ' = ' + self.process(bindparam) + set_clauses.append(clause) + + text += ', '.join(set_clauses) + # CrateDB amendment end. + + if self.returning or update_stmt._returning: + if self.returning_precedes_values: + text += " " + self.returning_clause( + update_stmt, self.returning or update_stmt._returning + ) + + if extra_froms: + extra_from_text = self.update_from_clause( + update_stmt, + update_stmt.table, + render_extra_froms, + dialect_hints, + **kw + ) + if extra_from_text: + text += " " + extra_from_text + + if update_stmt._where_criteria: + t = self._generate_delimited_and_list( + update_stmt._where_criteria, **kw + ) + if t: + text += " WHERE " + t + + limit_clause = self.update_limit_clause(update_stmt) + if limit_clause: + text += " " + limit_clause + + if ( + self.returning or update_stmt._returning + ) and not self.returning_precedes_values: + text += " " + self.returning_clause( + update_stmt, self.returning or update_stmt._returning + ) + + if self.ctes and toplevel: + text = self._render_cte_clause() + text + + self.stack.pop(-1) + + return text + + +def _get_crud_params_14(compiler, stmt, compile_state, **kw): + """create a set of tuples representing column/string pairs for use + in an INSERT or UPDATE statement. + + Also generates the Compiled object's postfetch, prefetch, and + returning column collections, used for default handling and ultimately + populating the CursorResult's prefetch_cols() and postfetch_cols() + collections. + + """ + from sqlalchemy.sql.crud import _key_getters_for_crud_column + from sqlalchemy.sql.crud import _create_bind_param + from sqlalchemy.sql.crud import REQUIRED + from sqlalchemy.sql.crud import _get_stmt_parameter_tuples_params + from sqlalchemy.sql.crud import _get_multitable_params + from sqlalchemy.sql.crud import _scan_insert_from_select_cols + from sqlalchemy.sql.crud import _scan_cols + from sqlalchemy import exc # noqa: F401 + from sqlalchemy.sql.crud import _extend_values_for_multiparams + + compiler.postfetch = [] + compiler.insert_prefetch = [] + compiler.update_prefetch = [] + compiler.returning = [] + + # getters - these are normally just column.key, + # but in the case of mysql multi-table update, the rules for + # .key must conditionally take tablename into account + ( + _column_as_key, + _getattr_col_key, + _col_bind_name, + ) = getters = _key_getters_for_crud_column(compiler, stmt, compile_state) + + compiler._key_getters_for_crud_column = getters + + # no parameters in the statement, no parameters in the + # compiled params - return binds for all columns + if compiler.column_keys is None and compile_state._no_parameters: + return [ + ( + c, + compiler.preparer.format_column(c), + _create_bind_param(compiler, c, None, required=True), + ) + for c in stmt.table.columns + ] + + if compile_state._has_multi_parameters: + spd = compile_state._multi_parameters[0] + stmt_parameter_tuples = list(spd.items()) + elif compile_state._ordered_values: + spd = compile_state._dict_parameters + stmt_parameter_tuples = compile_state._ordered_values + elif compile_state._dict_parameters: + spd = compile_state._dict_parameters + stmt_parameter_tuples = list(spd.items()) + else: + stmt_parameter_tuples = spd = None + + # if we have statement parameters - set defaults in the + # compiled params + if compiler.column_keys is None: + parameters = {} + elif stmt_parameter_tuples: + parameters = dict( + (_column_as_key(key), REQUIRED) + for key in compiler.column_keys + if key not in spd + ) + else: + parameters = dict( + (_column_as_key(key), REQUIRED) for key in compiler.column_keys + ) + + # create a list of column assignment clauses as tuples + values = [] + + if stmt_parameter_tuples is not None: + _get_stmt_parameter_tuples_params( + compiler, + compile_state, + parameters, + stmt_parameter_tuples, + _column_as_key, + values, + kw, + ) + + check_columns = {} + + # special logic that only occurs for multi-table UPDATE + # statements + if compile_state.isupdate and compile_state.is_multitable: + _get_multitable_params( + compiler, + stmt, + compile_state, + stmt_parameter_tuples, + check_columns, + _col_bind_name, + _getattr_col_key, + values, + kw, + ) + + if compile_state.isinsert and stmt._select_names: + _scan_insert_from_select_cols( + compiler, + stmt, + compile_state, + parameters, + _getattr_col_key, + _column_as_key, + _col_bind_name, + check_columns, + values, + kw, + ) + else: + _scan_cols( + compiler, + stmt, + compile_state, + parameters, + _getattr_col_key, + _column_as_key, + _col_bind_name, + check_columns, + values, + kw, + ) + + # CrateDB amendment. + # The rewriting logic in `rewrite_update` and `visit_update` needs + # adjustments here in order to prevent `sqlalchemy.exc.CompileError: + # Unconsumed column names: characters_name, data['nested']` + """ + if parameters and stmt_parameter_tuples: + check = ( + set(parameters) + .intersection(_column_as_key(k) for k, v in stmt_parameter_tuples) + .difference(check_columns) + ) + if check: + raise exc.CompileError( + "Unconsumed column names: %s" + % (", ".join("%s" % (c,) for c in check)) + ) + """ + + if compile_state._has_multi_parameters: + values = _extend_values_for_multiparams( + compiler, stmt, compile_state, values, kw + ) + elif not values and compiler.for_executemany: + # convert an "INSERT DEFAULT VALUES" + # into INSERT (firstcol) VALUES (DEFAULT) which can be turned + # into an in-place multi values. This supports + # insert_executemany_returning mode :) + values = [ + ( + stmt.table.columns[0], + compiler.preparer.format_column(stmt.table.columns[0]), + "DEFAULT", + ) + ] + + return values diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 46e3b7ad..1a2017fa 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -22,6 +22,11 @@ import logging from datetime import datetime, date +# FIXME: Workaround to be able to use SQLAlchemy 1.4. +# Caveat: This purges the ``cresultproxy`` extension +# at runtime, so it will impose a speed bump. +import crate.client.sqlalchemy.monkey # noqa:F401 + from sqlalchemy import types as sqltypes from sqlalchemy.engine import default, reflection from sqlalchemy.sql import functions diff --git a/src/crate/client/sqlalchemy/monkey.py b/src/crate/client/sqlalchemy/monkey.py new file mode 100644 index 00000000..653713bb --- /dev/null +++ b/src/crate/client/sqlalchemy/monkey.py @@ -0,0 +1,44 @@ +from crate.client.sqlalchemy.sa_version import SA_VERSION, SA_1_4 + + +def sqlalchemy_without_cresultproxy(): + """ + When running on SQLAlchemy 1.4 with the overhauled ``cresultproxy`` + C-implementation, UPDATE statements will raise:: + + RuntimeError: number of values in row (3) differ from number of column processors (2) + + The problem can be reproduced by running:: + + ./bin/test --test test_assign_to_craty_type_after_commit + + This function is a workaround to purge the ``cresultproxy`` + extension at runtime by using monkeypatching. + + The reason might be the CrateDB-specific amendments within + ``visit_update_14`` or even general problems with the new + architecture of SQLAlchemy 1.4 vs. the needs of CrateDB. + + See also: + - https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#rowproxy-is-no-longer-a-proxy-is-now-called-row-and-behaves-like-an-enhanced-named-tuple + - https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4_0b1/lib/sqlalchemy/cextension/resultproxy.c + """ + import sys + + # https://gist.github.com/schipiga/482de016fa749bc08c7b36cf5323fd1b + to_uncache = [] + for mod in sys.modules: + if mod.startswith("sqlalchemy"): + to_uncache.append(mod) + for mod in to_uncache: + del sys.modules[mod] + + # Don't allow SQLAlchemy to bring in this extension again. + sys.modules["sqlalchemy.cresultproxy"] = None + + +# FIXME: Workaround to be able to use SQLAlchemy 1.4. +# Caveat: This purges the ``cresultproxy`` extension +# at runtime, so it will impose a speed bump. +if SA_VERSION >= SA_1_4: + sqlalchemy_without_cresultproxy() diff --git a/src/crate/client/sqlalchemy/sa_version.py b/src/crate/client/sqlalchemy/sa_version.py index 0973a14a..502e5228 100644 --- a/src/crate/client/sqlalchemy/sa_version.py +++ b/src/crate/client/sqlalchemy/sa_version.py @@ -23,3 +23,5 @@ from distutils.version import StrictVersion as V SA_VERSION = V(sa.__version__) + +SA_1_4 = V('1.4.0b1') From bcf9fffa8056568eeaf05e2a6804162f6f2ddd09 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 11 May 2022 22:18:02 +0200 Subject: [PATCH 04/19] SA14: Adjust CrateDB dialect compiler patch to SqlAlchemy 1.4.36 The original code for `visit_update` and `_get_crud_params` from SQLAlchemy 1.4.0b1 has been vendored into the CrateDB dialect the other day, in order to amend it due to dialect-specific purposes. This patch reflects the changes from SA 1.4.0b1 to SA 1.4.36 on this code. --- lgtm.yml | 7 ++++++ src/crate/client/sqlalchemy/compiler.py | 29 ++++++++++++++++++------- src/crate/client/sqlalchemy/dialect.py | 2 +- src/crate/client/sqlalchemy/types.py | 2 +- 4 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 lgtm.yml diff --git a/lgtm.yml b/lgtm.yml new file mode 100644 index 00000000..f25f90ad --- /dev/null +++ b/lgtm.yml @@ -0,0 +1,7 @@ +queries: + + # Suppress some LGTM warnings. + + # A module is imported with the "import" and "import from" statements. + # https://lgtm.com/rules/1818040193/ + - exclude: py/import-and-import-from diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 62c92037..7eb5eb0c 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -23,8 +23,7 @@ from collections import defaultdict import sqlalchemy as sa -from sqlalchemy.sql import crud, selectable -from sqlalchemy.sql import compiler +from sqlalchemy.sql import compiler, crud, selectable from .types import MutableDict from .sa_version import SA_VERSION, SA_1_4 @@ -400,6 +399,10 @@ def visit_update_14(self, update_stmt, **kw): else: dialect_hints = None + if update_stmt._independent_ctes: + for cte in update_stmt._independent_ctes: + cte._compiler_dispatch(self, **kw) + text += table_text text += " SET " @@ -459,8 +462,9 @@ def visit_update_14(self, update_stmt, **kw): update_stmt, self.returning or update_stmt._returning ) - if self.ctes and toplevel: - text = self._render_cte_clause() + text + if self.ctes: + nesting_level = len(self.stack) if not toplevel else None + text = self._render_cte_clause(nesting_level=nesting_level) + text self.stack.pop(-1) @@ -481,7 +485,7 @@ def _get_crud_params_14(compiler, stmt, compile_state, **kw): from sqlalchemy.sql.crud import _create_bind_param from sqlalchemy.sql.crud import REQUIRED from sqlalchemy.sql.crud import _get_stmt_parameter_tuples_params - from sqlalchemy.sql.crud import _get_multitable_params + from sqlalchemy.sql.crud import _get_update_multitable_params from sqlalchemy.sql.crud import _scan_insert_from_select_cols from sqlalchemy.sql.crud import _scan_cols from sqlalchemy import exc # noqa: F401 @@ -561,7 +565,7 @@ def _get_crud_params_14(compiler, stmt, compile_state, **kw): # special logic that only occurs for multi-table UPDATE # statements if compile_state.isupdate and compile_state.is_multitable: - _get_multitable_params( + _get_update_multitable_params( compiler, stmt, compile_state, @@ -620,9 +624,18 @@ def _get_crud_params_14(compiler, stmt, compile_state, **kw): if compile_state._has_multi_parameters: values = _extend_values_for_multiparams( - compiler, stmt, compile_state, values, kw + compiler, + stmt, + compile_state, + values, + _column_as_key, + kw, ) - elif not values and compiler.for_executemany: + elif ( + not values + and compiler.for_executemany # noqa: W503 + and compiler.dialect.supports_default_metavalue # noqa: W503 + ): # convert an "INSERT DEFAULT VALUES" # into INSERT (firstcol) VALUES (DEFAULT) which can be turned # into an in-place multi values. This supports diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 1a2017fa..15373127 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -25,7 +25,7 @@ # FIXME: Workaround to be able to use SQLAlchemy 1.4. # Caveat: This purges the ``cresultproxy`` extension # at runtime, so it will impose a speed bump. -import crate.client.sqlalchemy.monkey # noqa:F401 +import crate.client.sqlalchemy.monkey # noqa:F401, lgtm[py/unused-import] from sqlalchemy import types as sqltypes from sqlalchemy.engine import default, reflection diff --git a/src/crate/client/sqlalchemy/types.py b/src/crate/client/sqlalchemy/types.py index a343d5e3..a01a68d1 100644 --- a/src/crate/client/sqlalchemy/types.py +++ b/src/crate/client/sqlalchemy/types.py @@ -168,7 +168,7 @@ class Any(expression.ColumnElement): def __init__(self, left, right, operator=operators.eq): self.type = sqltypes.Boolean() - self.left = expression._literal_as_binds(left) + self.left = expression.literal(left) self.right = right self.operator = operator From 936915824c2befcee422ca8487aea977e7f07123 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 12 May 2022 01:18:43 +0200 Subject: [PATCH 05/19] SA14: Adjust `rewrite_update` parameter mangling This patch resembles the missing bits, where previously things have not been in order. It is about the correct wrapping/unwrapping of data returned to and from `rewrite_update`, which deviates between SA < 1.4 an SA >= 1.4. --- src/crate/client/sqlalchemy/compiler.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 7eb5eb0c..8e189cf4 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -74,13 +74,19 @@ def crate_before_execute(conn, clauseelement, multiparams, params): is_crate = type(conn.dialect).__name__ == 'CrateDialect' if is_crate and isinstance(clauseelement, sa.sql.expression.Update): if SA_VERSION >= SA_1_4: - multiparams = ([params],) + if params is None: + multiparams = ([],) + else: + multiparams = ([params],) params = {} clauseelement, multiparams, params = rewrite_update(clauseelement, multiparams, params) if SA_VERSION >= SA_1_4: - params = multiparams[0] + if multiparams[0]: + params = multiparams[0][0] + else: + params = multiparams[0] multiparams = [] return clauseelement, multiparams, params From 867a4c0e14b9b3e1ca2bd2727b5b220b6fd94920 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 12 May 2022 13:11:01 +0200 Subject: [PATCH 06/19] SA14/Tests: Fix call signature to match `reflection.Inspector.get_table_names` Croaks otherwise:: TypeError: get_table_names() takes from 1 to 2 positional arguments but 3 were given --- src/crate/client/sqlalchemy/tests/dialect_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crate/client/sqlalchemy/tests/dialect_test.py b/src/crate/client/sqlalchemy/tests/dialect_test.py index d3811861..0b77a22f 100644 --- a/src/crate/client/sqlalchemy/tests/dialect_test.py +++ b/src/crate/client/sqlalchemy/tests/dialect_test.py @@ -92,6 +92,6 @@ def test_get_table_names(self): insp = inspect(self.character.metadata.bind) self.engine.dialect.server_version_info = (2, 0, 0) - eq_(insp.get_table_names(self.connection, "doc"), + eq_(insp.get_table_names(schema="doc"), ['t1', 't2']) in_("WHERE table_schema = ? AND table_type = 'BASE TABLE' ORDER BY", self.executed_statement) From 9ecb35d144dafddaad527e32715738e96d430690 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 12 May 2022 14:37:42 +0200 Subject: [PATCH 07/19] SA14/Tests: Do not pass `params=None` to `crate_before_execute` Specifically, for the non-bulk case. In case of validating bulk updates, it is still left to `None`. Croaks otherwise:: File "/Users/amo/dev/crate/sources/crate-python/src/crate/client/sqlalchemy/tests/compiler_test.py", line 58, in test_crate_update_rewritten assert hasattr(clauseelement, '_crate_specific') is True AssertionError --- src/crate/client/sqlalchemy/tests/compiler_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crate/client/sqlalchemy/tests/compiler_test.py b/src/crate/client/sqlalchemy/tests/compiler_test.py index 67b3bca8..cd3c4ef3 100644 --- a/src/crate/client/sqlalchemy/tests/compiler_test.py +++ b/src/crate/client/sqlalchemy/tests/compiler_test.py @@ -45,14 +45,14 @@ def setUp(self): def test_sqlite_update_not_rewritten(self): clauseelement, multiparams, params = crate_before_execute( - self.sqlite_engine, self.update, self.values, None + self.sqlite_engine, self.update, self.values, {} ) assert hasattr(clauseelement, '_crate_specific') is False def test_crate_update_rewritten(self): clauseelement, multiparams, params = crate_before_execute( - self.crate_engine, self.update, self.values, None + self.crate_engine, self.update, self.values, {} ) assert hasattr(clauseelement, '_crate_specific') is True From 071beacdc9b9f08fc77e5923f14192d87307a705 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 12 May 2022 15:02:32 +0200 Subject: [PATCH 08/19] SA14/Tests: Declare column expressions for `_score` as `literal_column` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the original code, SA14 would croak like:: sqlalchemy.exc.ArgumentError: Textual column expression '_score' should be explicitly declared with text('_score'), or use column('_score') for more specificity However, when using the `sa.text` type, as suggested, older SQLAlchemy versions will croak in this manner:: sqlalchemy.exc.InvalidRequestError: SQL expression, column, or mapped entity expected - got '' The `sa.column` type also will not work, because it would get rendered with quotes as `"_score"`. > `literal_column()` is similar to `column()`, except that it is more > often used as a “standalone” column expression that renders exactly > as stated. > > -- https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.literal_column :: - SELECT characters.name AS characters_name, _score FROM characters WHERE match(characters.name, ?) + SELECT characters.name AS characters_name, "_score" FROM characters WHERE match(characters.name, ?) Settling with the `sa.literal_column` type resolved any woes. --- CHANGES.txt | 34 +++++++++++++++++++ src/crate/client/doctests/sqlalchemy.txt | 5 ++- .../client/sqlalchemy/tests/match_test.py | 3 +- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 06fb7616..669c9a20 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,6 +24,40 @@ Unreleased - Add support for SQLAlchemy 1.4 + +Breaking changes +---------------- + +Textual column expressions +'''''''''''''''''''''''''' + +On the update to SQLAlchemy 1.4, some test cases had to be adjusted in order +to compensate for apparent additional strictness of SQLAlchemy on some details. + +Where it was ok to use a textual column expression in plain text beforehand, +a `SQLAlchemy literal_column`_ type should be used now. Otherwise, for example +when accessing `CrateDB system columns`_ like ``_score``, the engine might +complain like:: + + sqlalchemy.exc.ArgumentError: Textual column expression '_score' should be + explicitly declared with text('_score'), or use column('_score') for more + specificity + +The changes to be made look like this:: + + old: session.query(Character.name, '_score') + new: session.query(Character.name, sa.literal_column('_score')) + +:: + + old: .order_by(sa.desc(sa.text('_score'))) + new: .order_by(sa.desc(sa.literal_column('_score'))) + + +.. _SQLAlchemy literal_column: https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.literal_column +.. _CrateDB system columns: https://crate.io/docs/crate/reference/en/4.8/general/ddl/system-columns.html + + 2020/09/28 0.26.0 ================= diff --git a/src/crate/client/doctests/sqlalchemy.txt b/src/crate/client/doctests/sqlalchemy.txt index 12e187a4..d1648550 100644 --- a/src/crate/client/doctests/sqlalchemy.txt +++ b/src/crate/client/doctests/sqlalchemy.txt @@ -289,7 +289,7 @@ the other rows. The higher the score value, the more relevant the row. In most cases ``_score`` is not part of the SQLAlchemy Table definition, so it must be passed as a string:: - >>> session.query(Character.name, '_score') \ + >>> session.query(Character.name, sa.literal_column('_score')) \ ... .filter(match(Character.quote_ft, 'space')) \ ... .all() [('Tricia McMillan', ...)] @@ -298,11 +298,10 @@ To search on multiple columns you have to pass a dictionary with columns and ``boost`` attached. ``boost`` is a factor that increases the relevance of a column in respect to the other columns:: - >>> from sqlalchemy.sql import text >>> session.query(Character.name) \ ... .filter(match({Character.name_ft: 1.5, Character.quote_ft: 0.1}, ... 'Arthur')) \ - ... .order_by(sa.desc(text('_score'))) \ + ... .order_by(sa.desc(sa.literal_column('_score'))) \ ... .all() [('Arthur Dent',), ('Tricia McMillan',)] diff --git a/src/crate/client/sqlalchemy/tests/match_test.py b/src/crate/client/sqlalchemy/tests/match_test.py index e716ffcc..71b79d0d 100644 --- a/src/crate/client/sqlalchemy/tests/match_test.py +++ b/src/crate/client/sqlalchemy/tests/match_test.py @@ -109,7 +109,8 @@ def test_match_type_options(self): ) def test_score(self): - query = self.session.query(self.Character.name, '_score') \ + query = self.session.query(self.Character.name, + sa.literal_column('_score')) \ .filter(match(self.Character.name, 'Trillian')) self.assertSQL( "SELECT characters.name AS characters_name, _score " + From e58a6577366a4d0c6d2d6c0e8919a4fba033741a Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 13 May 2022 11:57:11 +0200 Subject: [PATCH 09/19] SA13: Suppress specific LGTM.com warning --- lgtm.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lgtm.yml b/lgtm.yml index f25f90ad..55db2cfa 100644 --- a/lgtm.yml +++ b/lgtm.yml @@ -5,3 +5,8 @@ queries: # A module is imported with the "import" and "import from" statements. # https://lgtm.com/rules/1818040193/ - exclude: py/import-and-import-from + + # Disable rule to compensate parameter naming in `CrateCompiler._get_crud_params`. + # Using an alternative name for the first parameter of an instance method makes code more difficult to read. + # https://lgtm.com/rules/910082/ + - exclude: py/not-named-self From 40b697e46dd894c7f63241efd298b645a4c67701 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Tue, 17 May 2022 17:11:05 +0200 Subject: [PATCH 10/19] SA14: Add test cases for new dialect method `get_view_names` In order to test cases when views are actually _present_, some would have to be created. This is out of scope for this patch. - https://github.com/sqlalchemy/sqlalchemy/wiki/Views - https://github.com/jklukas/sqlalchemy-views - https://stackoverflow.com/questions/9766940/how-to-create-an-sql-view-with-sqlalchemy --- .../client/sqlalchemy/doctests/reflection.txt | 8 ++++++++ src/crate/client/sqlalchemy/tests/dialect_test.py | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/crate/client/sqlalchemy/doctests/reflection.txt b/src/crate/client/sqlalchemy/doctests/reflection.txt index 489081a6..c87c24fe 100644 --- a/src/crate/client/sqlalchemy/doctests/reflection.txt +++ b/src/crate/client/sqlalchemy/doctests/reflection.txt @@ -17,6 +17,14 @@ List all tables:: >>> set(['checks', 'cluster', 'jobs', 'jobs_log']).issubset(inspector.get_table_names(schema='sys')) True +List all views:: + + >>> inspector.get_view_names() + [] + + >>> inspector.get_view_names(schema='sys') + [] + Get default schema name:: >>> inspector.default_schema_name diff --git a/src/crate/client/sqlalchemy/tests/dialect_test.py b/src/crate/client/sqlalchemy/tests/dialect_test.py index 0b77a22f..7505b0e4 100644 --- a/src/crate/client/sqlalchemy/tests/dialect_test.py +++ b/src/crate/client/sqlalchemy/tests/dialect_test.py @@ -83,7 +83,6 @@ def test_primary_keys(self): in_("information_schema.key_column_usage", self.executed_statement) def test_get_table_names(self): - self.fake_cursor.rowcount = 1 self.fake_cursor.description = ( ('foo', None, None, None, None, None, None), @@ -95,3 +94,17 @@ def test_get_table_names(self): eq_(insp.get_table_names(schema="doc"), ['t1', 't2']) in_("WHERE table_schema = ? AND table_type = 'BASE TABLE' ORDER BY", self.executed_statement) + + def test_get_view_names(self): + self.fake_cursor.rowcount = 1 + self.fake_cursor.description = ( + ('foo', None, None, None, None, None, None), + ) + self.fake_cursor.fetchall = MagicMock(return_value=[["v1"], ["v2"]]) + + insp = inspect(self.character.metadata.bind) + self.engine.dialect.server_version_info = (2, 0, 0) + eq_(insp.get_view_names(schema="doc"), + ['v1', 'v2']) + eq_(self.executed_statement, "SELECT table_name FROM information_schema.views " + "ORDER BY table_name ASC, table_schema ASC") From 0e5b785c35d6ed93a8314ce9c6362a5957cf0ede Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Tue, 17 May 2022 18:08:01 +0200 Subject: [PATCH 11/19] SA14: Enable SQL compilation caching for CrateDialect and friends This patch follows the documentation at [1] and adds the attributes `supports_statement_cache`, `cache_ok` and `inherit_cache` to the corresponding classes of `CrateDialect`, `UserDefinedType` and `ColumnElement`. The test suite thoroughly accepted this change of setting all attributes to `True`, effectively fully supporting SQL compilation caching. [1] https://docs.sqlalchemy.org/en/14/faq/performance.html --- src/crate/client/sqlalchemy/dialect.py | 1 + src/crate/client/sqlalchemy/predicates/__init__.py | 1 + src/crate/client/sqlalchemy/types.py | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 15373127..2d95e1c3 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -167,6 +167,7 @@ class CrateDialect(default.DefaultDialect): ddl_compiler = CrateDDLCompiler type_compiler = CrateTypeCompiler supports_native_boolean = True + supports_statement_cache = True colspecs = colspecs implicit_returning = True diff --git a/src/crate/client/sqlalchemy/predicates/__init__.py b/src/crate/client/sqlalchemy/predicates/__init__.py index 1c0c2bda..4f974f92 100644 --- a/src/crate/client/sqlalchemy/predicates/__init__.py +++ b/src/crate/client/sqlalchemy/predicates/__init__.py @@ -24,6 +24,7 @@ class Match(ColumnElement): + inherit_cache = True def __init__(self, column, term, match_type=None, options=None): super(Match, self).__init__() diff --git a/src/crate/client/sqlalchemy/types.py b/src/crate/client/sqlalchemy/types.py index a01a68d1..1a3d7a06 100644 --- a/src/crate/client/sqlalchemy/types.py +++ b/src/crate/client/sqlalchemy/types.py @@ -132,6 +132,7 @@ def __eq__(self, other): class _Craty(sqltypes.UserDefinedType): + cache_ok = True class Comparator(sqltypes.TypeEngine.Comparator): @@ -165,6 +166,7 @@ class Any(expression.ColumnElement): """ __visit_name__ = 'any' + inherit_cache = True def __init__(self, left, right, operator=operators.eq): self.type = sqltypes.Boolean() @@ -174,6 +176,7 @@ def __init__(self, left, right, operator=operators.eq): class _ObjectArray(sqltypes.UserDefinedType): + cache_ok = True class Comparator(sqltypes.TypeEngine.Comparator): def __getitem__(self, key): @@ -222,6 +225,7 @@ def get_col_spec(self, **kws): class Geopoint(sqltypes.UserDefinedType): + cache_ok = True class Comparator(sqltypes.TypeEngine.Comparator): @@ -247,6 +251,7 @@ def result_processor(self, dialect, coltype): class Geoshape(sqltypes.UserDefinedType): + cache_ok = True class Comparator(sqltypes.TypeEngine.Comparator): From ca33c73b0f82dede610cbe723ef2e500e0817eb6 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 18 May 2022 11:09:15 +0200 Subject: [PATCH 12/19] SA14/Tests: Remove masking the resultproxy and fix setup of fake cursors Beforehand, after observing errors like:: RuntimeError: number of values in row (1) differ from number of column processors (0) we decided to take the `resultproxy` out of the equation in order to work on the real gist of SQLAlchemy 1.4 compatibility. When looking at this again, it turned out that the root cause have been inaccuracies when setting up fake cursors within the test cases. SQLAlchemy <1.4 might have been more forgiving on that, but >=1.4 isn't. --- src/crate/client/sqlalchemy/dialect.py | 5 --- src/crate/client/sqlalchemy/monkey.py | 44 ------------------- .../client/sqlalchemy/tests/dict_test.py | 2 +- 3 files changed, 1 insertion(+), 50 deletions(-) delete mode 100644 src/crate/client/sqlalchemy/monkey.py diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 2d95e1c3..637a8f92 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -22,11 +22,6 @@ import logging from datetime import datetime, date -# FIXME: Workaround to be able to use SQLAlchemy 1.4. -# Caveat: This purges the ``cresultproxy`` extension -# at runtime, so it will impose a speed bump. -import crate.client.sqlalchemy.monkey # noqa:F401, lgtm[py/unused-import] - from sqlalchemy import types as sqltypes from sqlalchemy.engine import default, reflection from sqlalchemy.sql import functions diff --git a/src/crate/client/sqlalchemy/monkey.py b/src/crate/client/sqlalchemy/monkey.py deleted file mode 100644 index 653713bb..00000000 --- a/src/crate/client/sqlalchemy/monkey.py +++ /dev/null @@ -1,44 +0,0 @@ -from crate.client.sqlalchemy.sa_version import SA_VERSION, SA_1_4 - - -def sqlalchemy_without_cresultproxy(): - """ - When running on SQLAlchemy 1.4 with the overhauled ``cresultproxy`` - C-implementation, UPDATE statements will raise:: - - RuntimeError: number of values in row (3) differ from number of column processors (2) - - The problem can be reproduced by running:: - - ./bin/test --test test_assign_to_craty_type_after_commit - - This function is a workaround to purge the ``cresultproxy`` - extension at runtime by using monkeypatching. - - The reason might be the CrateDB-specific amendments within - ``visit_update_14`` or even general problems with the new - architecture of SQLAlchemy 1.4 vs. the needs of CrateDB. - - See also: - - https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#rowproxy-is-no-longer-a-proxy-is-now-called-row-and-behaves-like-an-enhanced-named-tuple - - https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4_0b1/lib/sqlalchemy/cextension/resultproxy.c - """ - import sys - - # https://gist.github.com/schipiga/482de016fa749bc08c7b36cf5323fd1b - to_uncache = [] - for mod in sys.modules: - if mod.startswith("sqlalchemy"): - to_uncache.append(mod) - for mod in to_uncache: - del sys.modules[mod] - - # Don't allow SQLAlchemy to bring in this extension again. - sys.modules["sqlalchemy.cresultproxy"] = None - - -# FIXME: Workaround to be able to use SQLAlchemy 1.4. -# Caveat: This purges the ``cresultproxy`` extension -# at runtime, so it will impose a speed bump. -if SA_VERSION >= SA_1_4: - sqlalchemy_without_cresultproxy() diff --git a/src/crate/client/sqlalchemy/tests/dict_test.py b/src/crate/client/sqlalchemy/tests/dict_test.py index d9a1cc3f..2efc6d81 100644 --- a/src/crate/client/sqlalchemy/tests/dict_test.py +++ b/src/crate/client/sqlalchemy/tests/dict_test.py @@ -138,7 +138,7 @@ def test_assign_null_to_object_array(self): @patch('crate.client.connection.Cursor', FakeCursor) def test_assign_to_craty_type_after_commit(self): session, Character = self.set_up_character_and_cursor( - return_value=[('Trillian', None, None)] + return_value=[('Trillian', None)] ) char = Character(name='Trillian') session.add(char) From a48c0939ca3648c925888ca53460a50f0bbec7de Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 18 May 2022 13:17:03 +0200 Subject: [PATCH 13/19] SA14: Fix SQLAlchemy deprecation warning on `before_execute` event hdlr SADeprecationWarning: The argument signature for the `ConnectionEvents.before_execute` event listener has changed as of version 1.4, and conversion for the old argument signature will be removed in a future release. The new signature is `def before_execute(conn, clauseelement, multiparams, params, execution_options)`. --- src/crate/client/sqlalchemy/compiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 8e189cf4..1aeeb971 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -70,7 +70,7 @@ def rewrite_update(clauseelement, multiparams, params): @sa.event.listens_for(sa.engine.Engine, "before_execute", retval=True) -def crate_before_execute(conn, clauseelement, multiparams, params): +def crate_before_execute(conn, clauseelement, multiparams, params, *args, **kwargs): is_crate = type(conn.dialect).__name__ == 'CrateDialect' if is_crate and isinstance(clauseelement, sa.sql.expression.Update): if SA_VERSION >= SA_1_4: From 40cabe3a80c01fd359a1f2df0ab8a480323f4343 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 18 May 2022 14:54:19 +0200 Subject: [PATCH 14/19] SA14: Final adjustments to the changelog --- CHANGES.txt | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 669c9a20..ef84b976 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -22,7 +22,13 @@ Unreleased - Added support for enabling SSL using SQLAlchemy DB URI with parameter ``?ssl=true``. -- Add support for SQLAlchemy 1.4 +- Added support for SQLAlchemy 1.4 + +.. note:: + + For learning about the transition to SQLAlchemy 1.4, we recommend the + corresponding documentation `What’s New in SQLAlchemy 1.4?`_. + Breaking changes @@ -31,31 +37,26 @@ Breaking changes Textual column expressions '''''''''''''''''''''''''' -On the update to SQLAlchemy 1.4, some test cases had to be adjusted in order -to compensate for apparent additional strictness of SQLAlchemy on some details. +SQLAlchemy 1.4 became stricter on some details. It requires to wrap `CrateDB +system columns`_ like ``_score`` in a `SQLAlchemy literal_column`_ type. +Before, it was possible to use a query like this:: -Where it was ok to use a textual column expression in plain text beforehand, -a `SQLAlchemy literal_column`_ type should be used now. Otherwise, for example -when accessing `CrateDB system columns`_ like ``_score``, the engine might -complain like:: - - sqlalchemy.exc.ArgumentError: Textual column expression '_score' should be - explicitly declared with text('_score'), or use column('_score') for more - specificity + session.query(Character.name, '_score') -The changes to be made look like this:: +It must now be written like:: - old: session.query(Character.name, '_score') - new: session.query(Character.name, sa.literal_column('_score')) + session.query(Character.name, sa.literal_column('_score')) -:: +Otherwise, SQLAlchemy will complain like:: - old: .order_by(sa.desc(sa.text('_score'))) - new: .order_by(sa.desc(sa.literal_column('_score'))) + sqlalchemy.exc.ArgumentError: Textual column expression '_score' should be + explicitly declared with text('_score'), or use column('_score') for more + specificity -.. _SQLAlchemy literal_column: https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.literal_column .. _CrateDB system columns: https://crate.io/docs/crate/reference/en/4.8/general/ddl/system-columns.html +.. _SQLAlchemy literal_column: https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.literal_column +.. _What’s New in SQLAlchemy 1.4?: https://docs.sqlalchemy.org/en/14/changelog/migration_14.html 2020/09/28 0.26.0 From abc6a6a1bd75fbfa5e250dc25e1b43a302fc01a5 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Tue, 24 May 2022 11:36:07 +0200 Subject: [PATCH 15/19] SA14: Add test fixture for testing VIEWs --- src/crate/client/sqlalchemy/doctests/reflection.txt | 5 +---- src/crate/client/tests.py | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/crate/client/sqlalchemy/doctests/reflection.txt b/src/crate/client/sqlalchemy/doctests/reflection.txt index c87c24fe..ae089e31 100644 --- a/src/crate/client/sqlalchemy/doctests/reflection.txt +++ b/src/crate/client/sqlalchemy/doctests/reflection.txt @@ -20,10 +20,7 @@ List all tables:: List all views:: >>> inspector.get_view_names() - [] - - >>> inspector.get_view_names(schema='sys') - [] + ['characters_view'] Get default schema name:: diff --git a/src/crate/client/tests.py b/src/crate/client/tests.py index 7b19a7fb..b63da6d0 100644 --- a/src/crate/client/tests.py +++ b/src/crate/client/tests.py @@ -195,7 +195,8 @@ def setUpCrateLayerAndSqlAlchemy(test): more_details array(object), INDEX name_ft using fulltext(name) with (analyzer = 'english'), INDEX quote_ft using fulltext(quote) with (analyzer = 'english') - ) """) + )""") + cursor.execute("CREATE VIEW characters_view AS SELECT * FROM characters") with connect(crate_host) as conn: cursor = conn.cursor() @@ -342,6 +343,7 @@ def tearDownWithCrateLayer(test): for stmt in ["DROP TABLE locations", "DROP BLOB TABLE myfiles", "DROP TABLE characters", + "DROP VIEW characters_view", "DROP TABLE cities", "DROP USER me", "DROP USER trusted_me", From 35aa8905f034624b4cd9517ee9ece7c1482198ce Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 25 May 2022 17:00:08 +0200 Subject: [PATCH 16/19] Documentation: Minor edit to fix link checker --- README.rst | 2 +- docs/getting-started.rst | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 8931a6c3..4c99e2cf 100644 --- a/README.rst +++ b/README.rst @@ -52,7 +52,7 @@ Prerequisites ============= Recent versions of this library are validated on Python 3 (>= 3.7). -It might also work on earlier versions of Python. +It may also work on earlier versions of Python. Installation diff --git a/docs/getting-started.rst b/docs/getting-started.rst index c220c2ac..99d67e0d 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -15,9 +15,8 @@ Learn how to install and get started the :ref:`CrateDB Python client library Prerequisites ============= -Python 3.7 is recommended. - -However, :ref:`older versions of Python ` can still be used. +Recent versions of this library are validated on Python 3 (>= 3.7). +It may also work on earlier versions of Python. `Pip`_ should be installed on your system. From 14479e5f31f5d8ed61958c4725970f7d95c7d155 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 1 Jun 2022 12:15:17 +0200 Subject: [PATCH 17/19] SA14: Improve inline documentation of CrateDB dialect compiler --- src/crate/client/sqlalchemy/compiler.py | 33 +++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 1aeeb971..38c94dba 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -230,14 +230,14 @@ def visit_update(self, update_stmt, **kw): update_stmt, table_text ) - # CrateDB amendment. + # CrateDB patch. crud_params = self._get_crud_params(update_stmt, **kw) text += table_text text += ' SET ' - # CrateDB amendment begin. + # CrateDB patch begin. include_table = extra_froms and \ self.render_table_with_column_in_update_from @@ -255,7 +255,7 @@ def visit_update(self, update_stmt, **kw): set_clauses.append(k + ' = ' + self.process(bindparam)) text += ', '.join(set_clauses) - # CrateDB amendment end. + # CrateDB patch end. if self.returning or update_stmt._returning: if not self.returning: @@ -393,7 +393,7 @@ def visit_update_14(self, update_stmt, **kw): update_stmt, update_stmt.table, render_extra_froms, **kw ) - # CrateDB amendment. + # CrateDB patch. crud_params = _get_crud_params_14( self, update_stmt, compile_state, **kw ) @@ -413,7 +413,7 @@ def visit_update_14(self, update_stmt, **kw): text += " SET " - # CrateDB amendment begin. + # CrateDB patch begin. include_table = extra_froms and \ self.render_table_with_column_in_update_from @@ -431,7 +431,7 @@ def visit_update_14(self, update_stmt, **kw): set_clauses.append(clause) text += ', '.join(set_clauses) - # CrateDB amendment end. + # CrateDB patch end. if self.returning or update_stmt._returning: if self.returning_precedes_values: @@ -610,10 +610,23 @@ def _get_crud_params_14(compiler, stmt, compile_state, **kw): kw, ) - # CrateDB amendment. - # The rewriting logic in `rewrite_update` and `visit_update` needs - # adjustments here in order to prevent `sqlalchemy.exc.CompileError: - # Unconsumed column names: characters_name, data['nested']` + # CrateDB patch. + # + # This sanity check performed by SQLAlchemy currently needs to be + # deactivated in order to satisfy the rewriting logic of the CrateDB + # dialect in `rewrite_update` and `visit_update`. + # + # It can be quickly reproduced by activating this section and running the + # test cases:: + # + # ./bin/test -vvvv -t dict_test + # + # That croaks like:: + # + # sqlalchemy.exc.CompileError: Unconsumed column names: characters_name, data['nested'] + # + # TODO: Investigate why this is actually happening and eventually mitigate + # the root cause. """ if parameters and stmt_parameter_tuples: check = ( From d93dfce0b09625e915a51edbbd745caa67cbe8d2 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 1 Jun 2022 12:35:08 +0200 Subject: [PATCH 18/19] SA14/CI: Update to SQLAlchemy 1.4.37 --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0512a7c9..e8ba181e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,7 +19,7 @@ jobs: os: [ubuntu-latest, macos-latest] python-version: ['3.7', '3.8', '3.9', '3.10'] cratedb-version: ['4.8.0'] - sqla-version: ['1.3.24', '1.4.36'] + sqla-version: ['1.3.24', '1.4.37'] fail-fast: true steps: From aad1458ce0fd34d391fe9c209110412aa396cc0d Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Wed, 1 Jun 2022 14:20:45 +0200 Subject: [PATCH 19/19] SA14/CI: Suppress alert from lgtm.com lgtm.com issued an alert like:: Module 'sqlalchemy' is imported with both 'import' and 'import from' See: https://lgtm.com/projects/g/crate/crate-python/rev/pr-8ea3b257a334a41cdb531d681072752e662eee56. The corresponding rule in lgtm.yml introduced by bcf9fffa8 does not seem to work. --- src/crate/client/sqlalchemy/compiler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 38c94dba..d5c29e83 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -22,8 +22,8 @@ import string from collections import defaultdict -import sqlalchemy as sa -from sqlalchemy.sql import compiler, crud, selectable +import sqlalchemy as sa # lgtm[py/import-and-import-from] +from sqlalchemy.sql import compiler, crud, selectable # lgtm[py/import-and-import-from] from .types import MutableDict from .sa_version import SA_VERSION, SA_1_4