Skip to content

Commit

Permalink
apply PG ddl paren rules to index expressions
Browse files Browse the repository at this point in the history
Fixed autogen render issue where expressions inside of indexes for PG need
to be double-parenthesized, meaning a single parens must be present within
the generated ``text()`` construct.

Change-Id: Iaf88ce28612707994a4753ee57384a8bb26f1133
Fixes: #1322
  • Loading branch information
zzzeek committed Oct 12, 2023
1 parent bd8e465 commit 5a95c35
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 2 deletions.
6 changes: 4 additions & 2 deletions alembic/autogenerate/render.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,10 @@ def _ident(name: Optional[Union[quoted_name, str]]) -> Optional[str]:
def _render_potential_expr(
value: Any,
autogen_context: AutogenContext,
*,
wrap_in_text: bool = True,
is_server_default: bool = False,
is_index: bool = False,
) -> str:
if isinstance(value, sql.ClauseElement):
if wrap_in_text:
Expand All @@ -555,7 +557,7 @@ def _render_potential_expr(
return template % {
"prefix": _sqlalchemy_autogenerate_prefix(autogen_context),
"sql": autogen_context.migration_context.impl.render_ddl_sql_expr(
value, is_server_default=is_server_default
value, is_server_default=is_server_default, is_index=is_index
),
}

Expand All @@ -569,7 +571,7 @@ def _get_index_rendered_expressions(
return [
repr(_ident(getattr(exp, "name", None)))
if isinstance(exp, sa_schema.Column)
else _render_potential_expr(exp, autogen_context)
else _render_potential_expr(exp, autogen_context, is_index=True)
for exp in idx.expressions
]

Expand Down
23 changes: 23 additions & 0 deletions alembic/ddl/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,29 @@ def _compile_element(self, element: ClauseElement) -> str:
compile_kwargs={"literal_binds": True, "include_table": False},
).string

def render_ddl_sql_expr(
self,
expr: ClauseElement,
is_server_default: bool = False,
is_index: bool = False,
**kw: Any,
) -> str:
"""Render a SQL expression that is typically a server default,
index expression, etc.
"""

# apply self_group to index expressions;
# see https://github.com/sqlalchemy/sqlalchemy/blob/
# 82fa95cfce070fab401d020c6e6e4a6a96cc2578/
# lib/sqlalchemy/dialects/postgresql/base.py#L2261
if is_index and not isinstance(expr, ColumnClause):
expr = expr.self_group()

return super().render_ddl_sql_expr(
expr, is_server_default=is_server_default, is_index=is_index, **kw
)

def render_type(
self, type_: TypeEngine, autogen_context: AutogenContext
) -> Union[str, Literal[False]]:
Expand Down
7 changes: 7 additions & 0 deletions docs/build/unreleased/1322.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.. change::
:tags: bug, postgresql
:tickets: 1322

Fixed autogen render issue where expressions inside of indexes for PG need
to be double-parenthesized, meaning a single parens must be present within
the generated ``text()`` construct.
16 changes: 16 additions & 0 deletions tests/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,22 @@ def test_jsonb_type(self):
"postgresql.JSONB(astext_type=sa.Text())",
)

def test_jsonb_expression_in_index(self):
"""test #1322"""

m = MetaData()
t = Table("tbl", m, Column("c", JSONB()))
idx = Index("my_idx", t.c.c["foo"].astext)

eq_ignore_whitespace(
autogenerate.render.render_op_text(
self.autogen_context,
ops.CreateIndexOp.from_index(idx),
),
"op.create_index('my_idx', 'tbl', "
"[sa.text(\"(c ->> 'foo')\")], unique=False)",
)

@config.requirements.nulls_not_distinct_sa
def test_render_unique_nulls_not_distinct_constraint(self):
m = MetaData()
Expand Down

0 comments on commit 5a95c35

Please sign in to comment.