diff --git a/CHANGES.txt b/CHANGES.txt index eae65d86..b738c22d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,6 +8,9 @@ Unreleased - SQLAlchemy: Rename leftover occurrences of ``Object``. The new symbol to represent CrateDB's ``OBJECT`` column type is now ``ObjectType``. +- SQLAlchemy DQL: Use CrateDB's native ``ILIKE`` operator instead of using SA's + generic implementation ``lower() LIKE lower()``. Thanks, @hlcianfagna. + 2023/07/06 0.32.0 ================= diff --git a/src/crate/client/sqlalchemy/compiler.py b/src/crate/client/sqlalchemy/compiler.py index 3965c9e1..3ae7a7cb 100644 --- a/src/crate/client/sqlalchemy/compiler.py +++ b/src/crate/client/sqlalchemy/compiler.py @@ -244,6 +244,49 @@ def visit_any(self, element, **kw): self.process(element.right, **kw) ) + def visit_ilike_case_insensitive_operand(self, element, **kw): + """ + Use native `ILIKE` operator, like PostgreSQL's `PGCompiler`. + """ + if self.dialect.has_ilike_operator(): + return element.element._compiler_dispatch(self, **kw) + else: + return super().visit_ilike_case_insensitive_operand(element, **kw) + + def visit_ilike_op_binary(self, binary, operator, **kw): + """ + Use native `ILIKE` operator, like PostgreSQL's `PGCompiler`. + + Do not implement the `ESCAPE` functionality, because it is not + supported by CrateDB. + """ + if binary.modifiers.get("escape", None) is not None: + raise NotImplementedError("Unsupported feature: ESCAPE is not supported") + if self.dialect.has_ilike_operator(): + return "%s ILIKE %s" % ( + self.process(binary.left, **kw), + self.process(binary.right, **kw), + ) + else: + return super().visit_ilike_op_binary(binary, operator, **kw) + + def visit_not_ilike_op_binary(self, binary, operator, **kw): + """ + Use native `ILIKE` operator, like PostgreSQL's `PGCompiler`. + + Do not implement the `ESCAPE` functionality, because it is not + supported by CrateDB. + """ + if binary.modifiers.get("escape", None) is not None: + raise NotImplementedError("Unsupported feature: ESCAPE is not supported") + if self.dialect.has_ilike_operator(): + return "%s NOT ILIKE %s" % ( + self.process(binary.left, **kw), + self.process(binary.right, **kw), + ) + else: + return super().visit_not_ilike_op_binary(binary, operator, **kw) + def limit_clause(self, select, **kw): """ Generate OFFSET / LIMIT clause, PostgreSQL-compatible. diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py index 74a8aff9..3f1197df 100644 --- a/src/crate/client/sqlalchemy/dialect.py +++ b/src/crate/client/sqlalchemy/dialect.py @@ -356,6 +356,13 @@ def _create_column_info(self, row): def _resolve_type(self, type_): return TYPES_MAP.get(type_, sqltypes.UserDefinedType) + def has_ilike_operator(self): + """ + Only CrateDB 4.1.0 and higher implements the `ILIKE` operator. + """ + server_version_info = self.server_version_info + return server_version_info is not None and server_version_info >= (4, 1, 0) + class DateTrunc(functions.GenericFunction): name = "date_trunc" diff --git a/src/crate/client/sqlalchemy/tests/__init__.py b/src/crate/client/sqlalchemy/tests/__init__.py index 3c032ebb..6102cb5a 100644 --- a/src/crate/client/sqlalchemy/tests/__init__.py +++ b/src/crate/client/sqlalchemy/tests/__init__.py @@ -2,6 +2,7 @@ from ..compat.api13 import monkeypatch_amend_select_sa14, monkeypatch_add_connectionfairy_driver_connection from ..sa_version import SA_1_4, SA_VERSION +from ...test_util import ParametrizedTestCase # `sql.select()` of SQLAlchemy 1.3 uses old calling semantics, # but the test cases already need the modern ones. @@ -32,6 +33,9 @@ def test_suite_unit(): tests.addTest(makeSuite(SqlAlchemyDictTypeTest)) tests.addTest(makeSuite(SqlAlchemyDateAndDateTimeTest)) tests.addTest(makeSuite(SqlAlchemyCompilerTest)) + tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": None})) + tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": (4, 0, 12)})) + tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": (4, 1, 10)})) tests.addTest(makeSuite(SqlAlchemyUpdateTest)) tests.addTest(makeSuite(SqlAlchemyMatchTest)) tests.addTest(makeSuite(SqlAlchemyCreateTableTest)) diff --git a/src/crate/client/sqlalchemy/tests/compiler_test.py b/src/crate/client/sqlalchemy/tests/compiler_test.py index 17612232..5d5cc89e 100644 --- a/src/crate/client/sqlalchemy/tests/compiler_test.py +++ b/src/crate/client/sqlalchemy/tests/compiler_test.py @@ -18,8 +18,8 @@ # However, if you have executed another commercial license agreement # with Crate these terms will supersede the license and you may use the # software solely pursuant to the terms of the relevant commercial agreement. - -from unittest import mock, TestCase, skipIf +from textwrap import dedent +from unittest import mock, skipIf from crate.client.sqlalchemy.compiler import crate_before_execute @@ -28,12 +28,16 @@ from crate.client.sqlalchemy.sa_version import SA_VERSION, SA_1_4, SA_2_0 from crate.client.sqlalchemy.types import ObjectType +from crate.client.test_util import ParametrizedTestCase -class SqlAlchemyCompilerTest(TestCase): +class SqlAlchemyCompilerTest(ParametrizedTestCase): def setUp(self): self.crate_engine = sa.create_engine('crate://') + if isinstance(self.param, dict) and "server_version_info" in self.param: + server_version_info = self.param["server_version_info"] + self.crate_engine.dialect.server_version_info = server_version_info self.sqlite_engine = sa.create_engine('sqlite://') self.metadata = sa.MetaData() self.mytable = sa.Table('mytable', self.metadata, @@ -71,6 +75,65 @@ def test_bulk_update_on_builtin_type(self): self.assertFalse(hasattr(clauseelement, '_crate_specific')) + def test_select_with_ilike_no_escape(self): + """ + Verify the compiler uses CrateDB's native `ILIKE` method. + """ + selectable = self.mytable.select().where(self.mytable.c.name.ilike("%foo%")) + statement = str(selectable.compile(bind=self.crate_engine)) + if self.crate_engine.dialect.has_ilike_operator(): + self.assertEqual(statement, dedent(""" + SELECT mytable.name, mytable.data + FROM mytable + WHERE mytable.name ILIKE ? + """).strip()) # noqa: W291 + else: + self.assertEqual(statement, dedent(""" + SELECT mytable.name, mytable.data + FROM mytable + WHERE lower(mytable.name) LIKE lower(?) + """).strip()) # noqa: W291 + + def test_select_with_not_ilike_no_escape(self): + """ + Verify the compiler uses CrateDB's native `ILIKE` method. + """ + selectable = self.mytable.select().where(self.mytable.c.name.notilike("%foo%")) + statement = str(selectable.compile(bind=self.crate_engine)) + if SA_VERSION < SA_1_4 or not self.crate_engine.dialect.has_ilike_operator(): + self.assertEqual(statement, dedent(""" + SELECT mytable.name, mytable.data + FROM mytable + WHERE lower(mytable.name) NOT LIKE lower(?) + """).strip()) # noqa: W291 + else: + self.assertEqual(statement, dedent(""" + SELECT mytable.name, mytable.data + FROM mytable + WHERE mytable.name NOT ILIKE ? + """).strip()) # noqa: W291 + + def test_select_with_ilike_and_escape(self): + """ + Verify the compiler fails when using CrateDB's native `ILIKE` method together with `ESCAPE`. + """ + + selectable = self.mytable.select().where(self.mytable.c.name.ilike("%foo%", escape='\\')) + with self.assertRaises(NotImplementedError) as cmex: + selectable.compile(bind=self.crate_engine) + self.assertEqual(str(cmex.exception), "Unsupported feature: ESCAPE is not supported") + + @skipIf(SA_VERSION < SA_1_4, "SQLAlchemy 1.3 and earlier do not support native `NOT ILIKE` compilation") + def test_select_with_not_ilike_and_escape(self): + """ + Verify the compiler fails when using CrateDB's native `ILIKE` method together with `ESCAPE`. + """ + + selectable = self.mytable.select().where(self.mytable.c.name.notilike("%foo%", escape='\\')) + with self.assertRaises(NotImplementedError) as cmex: + selectable.compile(bind=self.crate_engine) + self.assertEqual(str(cmex.exception), "Unsupported feature: ESCAPE is not supported") + def test_select_with_offset(self): """ Verify the `CrateCompiler.limit_clause` method, with offset. diff --git a/src/crate/client/test_util.py b/src/crate/client/test_util.py index 90379a79..823a44e3 100644 --- a/src/crate/client/test_util.py +++ b/src/crate/client/test_util.py @@ -18,6 +18,7 @@ # However, if you have executed another commercial license agreement # with Crate these terms will supersede the license and you may use the # software solely pursuant to the terms of the relevant commercial agreement. +import unittest class ClientMocked(object): @@ -42,3 +43,27 @@ def set_next_server_infos(self, server, server_name, version): def close(self): pass + + +class ParametrizedTestCase(unittest.TestCase): + """ + TestCase classes that want to be parametrized should + inherit from this class. + + https://eli.thegreenplace.net/2011/08/02/python-unit-testing-parametrized-test-cases + """ + def __init__(self, methodName="runTest", param=None): + super(ParametrizedTestCase, self).__init__(methodName) + self.param = param + + @staticmethod + def parametrize(testcase_klass, param=None): + """ Create a suite containing all tests taken from the given + subclass, passing them the parameter 'param'. + """ + testloader = unittest.TestLoader() + testnames = testloader.getTestCaseNames(testcase_klass) + suite = unittest.TestSuite() + for name in testnames: + suite.addTest(testcase_klass(name, param=param)) + return suite