adjust concat precedence to match that of string comparison

Adjusted the operator precedence for the string concatenation operator to
be equal to that of string matching operators, such as
:meth:`.ColumnElement.like`, :meth:`.ColumnElement.regexp_match`,
:meth:`.ColumnElement.match`, etc., as well as plain ``==`` which has the
same precedence as string comparison operators, so that parenthesis will be
applied to a string concatenation expression that follows a string match
operator. This provides for backends such as PostgreSQL where the "regexp
match" operator is apparently of higher precedence than the string
concatenation operator.

Fixes: #9610
Change-Id: I73640e40e445375177340e1ed8f45b5da98d6dfb

diff --git a/doc/build/changelog/unreleased_20/9610.rst b/doc/build/changelog/unreleased_20/9610.rst
new file mode 100644 (file)
index 0000000..db39086
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9610.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 9610
+
+    Adjusted the operator precedence for the string concatenation operator to
+    be equal to that of string matching operators, such as
+    :meth:`.ColumnElement.like`, :meth:`.ColumnElement.regexp_match`,
+    :meth:`.ColumnElement.match`, etc., as well as plain ``==`` which has the
+    same precedence as string comparison operators, so that parenthesis will be
+    applied to a string concatenation expression that follows a string match
+    operator. This provides for backends such as PostgreSQL where the "regexp
+    match" operator is apparently of higher precedence than the string
+    concatenation operator.

diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py
index dbd593e9fbf418a579b7fa6098dcdcb1f6d91687..6ec150424b8d6de4f89027bb5a26fa00f61a9f5f 100644 (file)
--- a/lib/sqlalchemy/sql/operators.py
+++ b/lib/sqlalchemy/sql/operators.py
@@ -2533,8 +2533,8 @@ _PRECEDENCE: Dict[OperatorType, int] = {
     bitwise_and_op: 7,
     bitwise_lshift_op: 7,
     bitwise_rshift_op: 7,
-    concat_op: 6,
     filter_op: 6,
+    concat_op: 5,
     match_op: 5,
     not_match_op: 5,
     regexp_match_op: 5,

diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py
index 2de35a9a1e8a56085a7e565aea4e90e2deaad94f..d6b886be151b1dc246f3306160940b08cfbdd1ab 100644 (file)
--- a/test/orm/test_relationships.py
+++ b/test/orm/test_relationships.py
@@ -531,12 +531,12 @@ class DirectSelfRefFKTest(fixtures.MappedTest, AssertsCompiledSQL):
         Entity = self.classes.Entity
         self.assert_compile(
             Entity.descendants.property.strategy._lazywhere,
-            "entity.path LIKE :param_1 || :path_1",
+            "entity.path LIKE (:param_1 || :path_1)",
         )
 
         self.assert_compile(
             Entity.descendants.property.strategy._rev_lazywhere,
-            ":param_1 LIKE entity.path || :path_1",
+            ":param_1 LIKE (entity.path || :path_1)",
         )
 
     def test_ancestors_lazyload_clause(self):
@@ -545,12 +545,12 @@ class DirectSelfRefFKTest(fixtures.MappedTest, AssertsCompiledSQL):
         # :param_1 LIKE (:param_1 || :path_1)
         self.assert_compile(
             Entity.anscestors.property.strategy._lazywhere,
-            ":param_1 LIKE entity.path || :path_1",
+            ":param_1 LIKE (entity.path || :path_1)",
         )
 
         self.assert_compile(
             Entity.anscestors.property.strategy._rev_lazywhere,
-            "entity.path LIKE :param_1 || :path_1",
+            "entity.path LIKE (:param_1 || :path_1)",
         )
 
     def test_descendants_lazyload(self):
@@ -636,7 +636,7 @@ class DirectSelfRefFKTest(fixtures.MappedTest, AssertsCompiledSQL):
         self.assert_compile(
             sess.query(Entity).join(Entity.descendants.of_type(da)),
             "SELECT entity.path AS entity_path FROM entity JOIN entity AS "
-            "entity_1 ON entity_1.path LIKE entity.path || :path_1",
+            "entity_1 ON entity_1.path LIKE (entity.path || :path_1)",
         )
 
 
@@ -6529,8 +6529,8 @@ class SecondaryIncludesLocalColsTest(fixtures.MappedTest):
             CompiledSQL(
                 "SELECT a.id AS a_id, b.id AS b_id FROM a JOIN "
                 "(SELECT a.id AS "
-                "aid, b.id AS id FROM a JOIN b ON a.b_ids LIKE :id_1 || "
-                "b.id || :param_1) AS anon_1 ON a.id = anon_1.aid "
+                "aid, b.id AS id FROM a JOIN b ON a.b_ids LIKE (:id_1 || "
+                "b.id || :param_1)) AS anon_1 ON a.id = anon_1.aid "
                 "JOIN b ON b.id = anon_1.id ORDER BY a.id, b.id"
             )
         )
@@ -6551,7 +6551,7 @@ class SecondaryIncludesLocalColsTest(fixtures.MappedTest):
             CompiledSQL(
                 "SELECT a.id AS a_id, a.b_ids AS a_b_ids, b_1.id AS b_1_id "
                 "FROM a LEFT OUTER JOIN ((SELECT a.id AS aid, b.id AS id "
-                "FROM a JOIN b ON a.b_ids LIKE :id_1 || b.id || :param_1) "
+                "FROM a JOIN b ON a.b_ids LIKE (:id_1 || b.id || :param_1)) "
                 "AS anon_1 JOIN b AS b_1 ON b_1.id = anon_1.id) "
                 "ON a.id = anon_1.aid WHERE a.id = :id_2",
                 params=[{"id_1": "%", "param_1": "%", "id_2": 2}],
@@ -6570,7 +6570,7 @@ class SecondaryIncludesLocalColsTest(fixtures.MappedTest):
             CompiledSQL(
                 "SELECT a.id AS a_id FROM a WHERE "
                 "EXISTS (SELECT 1 FROM b, (SELECT a.id AS aid, b.id AS id "
-                "FROM a JOIN b ON a.b_ids LIKE :id_1 || b.id || :param_1) "
+                "FROM a JOIN b ON a.b_ids LIKE (:id_1 || b.id || :param_1)) "
                 "AS anon_1 WHERE a.id = anon_1.aid AND b.id = anon_1.id)",
                 params=[],
             )
@@ -6600,7 +6600,7 @@ class SecondaryIncludesLocalColsTest(fixtures.MappedTest):
             CompiledSQL(
                 "SELECT a_1.id AS a_1_id, b.id AS b_id FROM a AS a_1 JOIN "
                 "(SELECT a.id AS aid, b.id AS id FROM a JOIN b ON a.b_ids "
-                "LIKE :id_1 || b.id || :param_1) AS anon_1 "
+                "LIKE (:id_1 || b.id || :param_1)) AS anon_1 "
                 "ON a_1.id = anon_1.aid JOIN b ON b.id = anon_1.id "
                 "WHERE a_1.id IN (__[POSTCOMPILE_primary_keys])",
                 params=[{"id_1": "%", "param_1": "%", "primary_keys": [2]}],

diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py
index 9ae827e3776e92507dc4d8d4b5ca426240569eea..eca45c5d5d30ddceb36c85ffa0fdeb64f930757b 100644 (file)
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -3235,6 +3235,28 @@ class RegexpTestStrCompiler(fixtures.TestBase, testing.AssertsCompiledSQL):
             "<regexp replace>(mytable.myid, :myid_1, :myid_2))",
         )
 
+    @testing.combinations(
+        (lambda c, r: c.regexp_match(r), "<regexp>"),
+        (lambda c, r: c.like(r), "LIKE"),
+        (lambda c, r: ~c.regexp_match(r), "<not regexp>"),
+        (lambda c, r: ~c.match(r), "NOT %s MATCH"),
+        (lambda c, r: c.match(r), "MATCH"),
+    )
+    def test_all_match_precedence_against_concat(self, expr, expected):
+        expr = testing.resolve_lambda(
+            expr, c=self.table.c.myid, r=self.table.c.name + "some text"
+        )
+
+        if "%s" in expected:
+            self.assert_compile(
+                expr,
+                f"{expected % ('mytable.myid', )} (mytable.name || :name_1)",
+            )
+        else:
+            self.assert_compile(
+                expr, f"mytable.myid {expected} (mytable.name || :name_1)"
+            )
+
 
 class ComposedLikeOperatorsTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     __dialect__ = "default"

diff --git a/test/sql/test_update.py b/test/sql/test_update.py
index 72980ab55ab5ae0b575487db7ff133960a288d65..febbf4345e999dceade507bf81f60acf0763a24b 100644 (file)
--- a/test/sql/test_update.py
+++ b/test/sql/test_update.py
@@ -615,7 +615,7 @@ class UpdateTest(_UpdateFromTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             "name=(mytable.name || :name_1) "
             "WHERE "
             "mytable.myid = hoho(:hoho_1) AND "
-            "mytable.name = :param_2 || mytable.name || :param_3",
+            "mytable.name = (:param_2 || mytable.name || :param_3)",
         )
 
     def test_unconsumed_names_kwargs(self):
@@ -676,7 +676,7 @@ class UpdateTest(_UpdateFromTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             "myid=do_stuff(mytable.myid, :param_1) "
             "WHERE "
             "mytable.myid = hoho(:hoho_1) AND "
-            "mytable.name = :param_2 || mytable.name || :param_3",
+            "mytable.name = (:param_2 || mytable.name || :param_3)",
         )
 
     def test_update_ordered_parameters_newstyle_2(self):
@@ -706,7 +706,7 @@ class UpdateTest(_UpdateFromTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             "myid=do_stuff(mytable.myid, :param_1) "
             "WHERE "
             "mytable.myid = hoho(:hoho_1) AND "
-            "mytable.name = :param_2 || mytable.name || :param_3",
+            "mytable.name = (:param_2 || mytable.name || :param_3)",
         )
 
     def test_update_ordered_parameters_multiple(self):
@@ -776,7 +776,7 @@ class UpdateTest(_UpdateFromTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             "name=(mytable.name || :name_1) "
             "WHERE "
             "mytable.myid = hoho(:hoho_1) AND "
-            "mytable.name = :param_2 || mytable.name || :param_3",
+            "mytable.name = (:param_2 || mytable.name || :param_3)",
         )
 
     def test_where_empty(self):