From: Federico Caselli <cfederico87@gmail.com>
Date: Fri, 7 Apr 2023 18:35:37 +0000 (+0200)
Subject: Fix reflection of long expressions in postgresql
X-Git-Tag: rel_2_0_10~26^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7e285f2234010c241a357ae1d6d77a6fc43177bb;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Fix reflection of long expressions in postgresql

Fixed issue that prevented reflection of expression based indexes
with long expressions in PostgreSQL. The expression where erroneously
truncated to the identifier length (that's 63 bytes by default).

Fixes: #9615
Change-Id: I50727b0699e08fa25f10f3c94dcf8b79534bfb75
---

diff --git a/doc/build/changelog/unreleased_20/9615.rst b/doc/build/changelog/unreleased_20/9615.rst
new file mode 100644
index 0000000000..ccc37577ef
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9615.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, postgresql
+    :tickets: 9615
+
+    Fixed issue that prevented reflection of expression based indexes
+    with long expressions in PostgreSQL. The expression where erroneously
+    truncated to the identifier length (that's 63 bytes by default).
diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py
index 18f31ce470..4d299b9183 100644
--- a/lib/sqlalchemy/dialects/postgresql/base.py
+++ b/lib/sqlalchemy/dialects/postgresql/base.py
@@ -3915,6 +3915,8 @@ class PGDialect(default.DefaultDialect):
             select(
                 pg_catalog.pg_class.c.relname,
                 pg_catalog.pg_constraint.c.conname,
+                # NOTE: avoid calling pg_get_constraintdef when not needed
+                # to speed up the query
                 sql.case(
                     (
                         pg_catalog.pg_constraint.c.oid.is_not(None),
@@ -4121,7 +4123,10 @@ class PGDialect(default.DefaultDialect):
                             idx_sq.c.indexrelid, idx_sq.c.ord + 1, True
                         ),
                     ),
-                    else_=pg_catalog.pg_attribute.c.attname,
+                    # NOTE: need to cast this since attname is of type "name"
+                    # that's limited to 63 bytes, while pg_get_indexdef
+                    # returns "text" so it may get cut
+                    else_=sql.cast(pg_catalog.pg_attribute.c.attname, TEXT()),
                 ).label("element"),
                 (idx_sq.c.attnum == 0).label("is_expr"),
             )
@@ -4169,9 +4174,9 @@ class PGDialect(default.DefaultDialect):
                 pg_catalog.pg_index.c.indoption,
                 pg_class_index.c.reloptions,
                 pg_catalog.pg_am.c.amname,
+                # NOTE: pg_get_expr is very fast so this case has almost no
+                # performance impact
                 sql.case(
-                    # pg_get_expr is very fast so this case has almost no
-                    # performance impact
                     (
                         pg_catalog.pg_index.c.indpred.is_not(None),
                         pg_catalog.pg_get_expr(
@@ -4179,7 +4184,7 @@ class PGDialect(default.DefaultDialect):
                             pg_catalog.pg_index.c.indrelid,
                         ),
                     ),
-                    else_=sql.null(),
+                    else_=None,
                 ).label("filter_definition"),
                 indnkeyatts,
                 cols_sq.c.elements,
@@ -4455,6 +4460,8 @@ class PGDialect(default.DefaultDialect):
             select(
                 pg_catalog.pg_class.c.relname,
                 pg_catalog.pg_constraint.c.conname,
+                # NOTE: avoid calling pg_get_constraintdef when not needed
+                # to speed up the query
                 sql.case(
                     (
                         pg_catalog.pg_constraint.c.oid.is_not(None),
diff --git a/lib/sqlalchemy/testing/suite/test_reflection.py b/lib/sqlalchemy/testing/suite/test_reflection.py
index 8b7cb8cbc8..5927df065f 100644
--- a/lib/sqlalchemy/testing/suite/test_reflection.py
+++ b/lib/sqlalchemy/testing/suite/test_reflection.py
@@ -2397,7 +2397,8 @@ class ComponentReflectionTestExtra(ComparesIndexes, fixtures.TestBase):
         )
 
         Index("t_idx", func.lower(t.c.x), t.c.z, func.lower(t.c.y))
-
+        long_str = "long string " * 100
+        Index("t_idx_long", func.coalesce(t.c.x, long_str))
         Index("t_idx_2", t.c.x)
 
         metadata.create_all(connection)
@@ -2424,24 +2425,41 @@ class ComponentReflectionTestExtra(ComparesIndexes, fixtures.TestBase):
 
         completeIndex(expected[0])
 
-        class filtering_str(str):
+        class lower_index_str(str):
             def __eq__(self, other):
                 # test that lower and x or y are in the string
                 return "lower" in other and ("x" in other or "y" in other)
 
+        class coalesce_index_str(str):
+            def __eq__(self, other):
+                # test that coalesce and the string is in other
+                return "coalesce" in other.lower() and long_str in other
+
         if testing.requires.reflect_indexes_with_expressions.enabled:
             expr_index = {
                 "name": "t_idx",
                 "column_names": [None, "z", None],
                 "expressions": [
-                    filtering_str("lower(x)"),
+                    lower_index_str("lower(x)"),
                     "z",
-                    filtering_str("lower(y)"),
+                    lower_index_str("lower(y)"),
                 ],
                 "unique": False,
             }
             completeIndex(expr_index)
             expected.insert(0, expr_index)
+
+            expr_index_long = {
+                "name": "t_idx_long",
+                "column_names": [None],
+                "expressions": [
+                    coalesce_index_str(f"coalesce(x, '{long_str}')")
+                ],
+                "unique": False,
+            }
+            completeIndex(expr_index_long)
+            expected.append(expr_index_long)
+
             eq_(insp.get_indexes("t"), expected)
             m2 = MetaData()
             t2 = Table("t", m2, autoload_with=connection)