diff --git a/alembic/autogenerate/compare/comments.py b/alembic/autogenerate/compare/comments.py index 8db56606..8fb2e80b 100644 --- a/alembic/autogenerate/compare/comments.py +++ b/alembic/autogenerate/compare/comments.py @@ -34,8 +34,9 @@ def _compare_column_comment( if not autogen_context.dialect.supports_comments: return PriorityDispatchResult.CONTINUE - metadata_comment = metadata_col.comment - conn_col_comment = conn_col.comment + impl = autogen_context.migration_context.impl + metadata_comment = impl.normalize_comment(metadata_col.comment) + conn_col_comment = impl.normalize_comment(conn_col.comment) if conn_col_comment is None and metadata_comment is None: return PriorityDispatchResult.CONTINUE @@ -70,19 +71,23 @@ def _compare_table_comment( if conn_table.comment is None and metadata_table.comment is None: return PriorityDispatchResult.CONTINUE - if metadata_table.comment is None and conn_table.comment is not None: + impl = autogen_context.migration_context.impl + conn_comment = impl.normalize_comment(conn_table.comment) + metadata_comment = impl.normalize_comment(metadata_table.comment) + + if metadata_comment is None and conn_comment is not None: modify_table_ops.ops.append( ops.DropTableCommentOp( - tname, existing_comment=conn_table.comment, schema=schema + tname, existing_comment=conn_comment, schema=schema ) ) return PriorityDispatchResult.STOP - elif metadata_table.comment != conn_table.comment: + elif metadata_comment != conn_comment: modify_table_ops.ops.append( ops.CreateTableCommentOp( tname, - metadata_table.comment, - existing_comment=conn_table.comment, + metadata_comment, + existing_comment=conn_comment, schema=schema, ) ) diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py index 84e21bfa..06453838 100644 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@ -267,7 +267,7 @@ def _add_table(autogen_context: AutogenContext, op: ops.CreateTableOp) -> str: text += ",\nschema=%r" % _ident(op.schema) comment = table.comment - if comment: + if comment is not None: text += ",\ncomment=%r" % _ident(comment) info = table.info @@ -808,7 +808,7 @@ def _render_column( opts.append(("system", column.system)) comment = column.comment - if comment: + if comment is not None: opts.append(("comment", "%r" % comment)) # TODO: for non-ascii colname, assign a "key" diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py index be5dbb5a..e7e09f97 100644 --- a/alembic/ddl/impl.py +++ b/alembic/ddl/impl.py @@ -442,12 +442,12 @@ def create_table(self, table: Table, **kw: Any) -> None: self.dialect.supports_comments and not self.dialect.inline_comments ) comment = table.comment - if comment and with_comment: + if comment is not None and with_comment: self.create_table_comment(table) for column in table.columns: comment = column.comment - if comment and with_comment: + if comment is not None and with_comment: self.create_column_comment(column) def drop_table(self, table: Table, **kw: Any) -> None: @@ -632,6 +632,9 @@ def compare_server_default( ): return rendered_inspector_default != rendered_metadata_default + def normalize_comment(self, comment: str | None) -> str | None: + return comment + def correct_for_autogen_constraints( self, conn_uniques: set[UniqueConstraint], diff --git a/alembic/ddl/mysql.py b/alembic/ddl/mysql.py index 19ef3e82..a2b12716 100644 --- a/alembic/ddl/mysql.py +++ b/alembic/ddl/mysql.py @@ -552,7 +552,7 @@ def _mysql_colspec( spec += " AUTO_INCREMENT" if server_default is not False and server_default is not None: spec += " DEFAULT %s" % format_server_default(compiler, server_default) - if comment: + if comment is not None: spec += " COMMENT %s" % compiler.sql_compiler.render_literal_value( comment, sqltypes.String() ) diff --git a/alembic/ddl/oracle.py b/alembic/ddl/oracle.py index c93fa4b5..72df61c8 100644 --- a/alembic/ddl/oracle.py +++ b/alembic/ddl/oracle.py @@ -55,6 +55,11 @@ def _exec(self, construct: Any, *args, **kw) -> CursorResult | None: self.static_output(self.batch_separator) return result + def normalize_comment(self, comment: str | None) -> str | None: + if comment == "": + return None + return comment + def compare_server_default( self, inspector_column, diff --git a/alembic/operations/toimpl.py b/alembic/operations/toimpl.py index c4eef3d7..7dddbe0a 100644 --- a/alembic/operations/toimpl.py +++ b/alembic/operations/toimpl.py @@ -209,7 +209,7 @@ def add_column(operations: "Operations", operation: "ops.AddColumnOp") -> None: and not operations.impl.dialect.inline_comments ) comment = column.comment - if comment and with_comment: + if comment is not None and with_comment: operations.impl.create_column_comment(column) diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index 2d28b02d..fe7885f6 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -2682,3 +2682,78 @@ def include_name(name, type_, parent_names): diffs = self._fixture(m1, m2, name_filters=include_name) eq_(len(diffs), 1) eq_(diffs[0][0], "remove_fk") + + +class CompareCommentTest(AutogenFixtureTest, TestBase): + """Empty string comments are distinct from NULL, except on Oracle + where empty string equals NULL. See #1085.""" + + __backend__ = True + __requires__ = ("comments",) + + def test_column_empty_comment_no_change(self): + m1 = MetaData() + m2 = MetaData() + + Table("a", m1, Column("x", Integer, comment="")) + Table("a", m2, Column("x", Integer, comment="")) + + diffs = self._fixture(m1, m2) + eq_(diffs, []) + + def test_column_none_to_empty_comment(self): + m1 = MetaData() + m2 = MetaData() + + Table("a", m1, Column("x", Integer)) + Table("a", m2, Column("x", Integer, comment="")) + + diffs = self._fixture(m1, m2) + if config.db.name == "oracle": + eq_(diffs, []) + else: + eq_(len(diffs), 1) + eq_(diffs[0][0][0], "modify_comment") + eq_(diffs[0][0][5], None) + eq_(diffs[0][0][6], "") + + def test_table_empty_comment_no_change(self): + m1 = MetaData() + m2 = MetaData() + + Table( + "some_table", + m1, + Column("x", Integer, primary_key=True), + comment="", + ) + Table( + "some_table", + m2, + Column("x", Integer, primary_key=True), + comment="", + ) + + diffs = self._fixture(m1, m2) + eq_(diffs, []) + + def test_table_none_to_empty_comment(self): + m1 = MetaData() + m2 = MetaData() + + Table("some_table", m1, Column("x", Integer, primary_key=True)) + Table( + "some_table", + m2, + Column("x", Integer, primary_key=True), + comment="", + ) + + diffs = self._fixture(m1, m2) + if config.db.name == "oracle": + eq_(diffs, []) + else: + eq_(len(diffs), 1) + eq_(diffs[0][0], "add_table_comment") + eq_(diffs[0][1].comment, "") + eq_(diffs[0][2], None) diff --git a/tests/test_autogen_render.py b/tests/test_autogen_render.py index 7d966934..1d42e4fe 100644 --- a/tests/test_autogen_render.py +++ b/tests/test_autogen_render.py @@ -1400,6 +1400,17 @@ def test_render_col_with_comment(self): "comment='This is a comment')", ) + def test_render_col_with_empty_comment(self): + c = Column("some_key", Integer, comment="") + Table("some_table", MetaData(), c) + result = autogenerate.render._render_column(c, self.autogen_context) + eq_ignore_whitespace( + result, + "sa.Column('some_key', sa.Integer(), " + "nullable=True, " + "comment='')", + ) + def test_render_col_comment_with_quote(self): c = Column("some_key", Integer, comment="This is a john's comment") Table("some_table", MetaData(), c) @@ -2299,6 +2310,24 @@ def test_render_table_with_comment(self): ")", ) + def test_render_table_with_empty_comment(self): + m = MetaData() + t = Table( + "test", + m, + Column("id", Integer, primary_key=True, comment=""), + comment="", + ) + op_obj = ops.CreateTableOp.from_table(t) + eq_ignore_whitespace( + autogenerate.render_op_text(self.autogen_context, op_obj), + "op.create_table('test'," + "sa.Column('id', sa.Integer(), nullable=False, comment='')," + "sa.PrimaryKeyConstraint('id')," + "comment=''" + ")", + ) + def test_render_table_with_info(self): m = MetaData() t = Table( @@ -2330,6 +2359,14 @@ def test_render_add_column_with_comment(self): "nullable=True, comment='This is a Column'))", ) + def test_render_add_column_with_empty_comment(self): + op_obj = ops.AddColumnOp("foo", Column("x", Integer, comment="")) + eq_ignore_whitespace( + autogenerate.render_op_text(self.autogen_context, op_obj), + "op.add_column('foo', sa.Column('x', sa.Integer(), " + "nullable=True, comment=''))", + ) + def test_render_create_table_comment_op(self): op_obj = ops.CreateTableCommentOp("table_name", "comment") eq_ignore_whitespace(