diff --git a/db2sql/domain/model/column.py b/db2sql/domain/model/column.py index 2444ea2..eaeaa2f 100644 --- a/db2sql/domain/model/column.py +++ b/db2sql/domain/model/column.py @@ -20,6 +20,7 @@ class Column: computed_definition: Optional[str] = None identity: bool = False constraint: Optional[str] = None + comment: Optional[str] = None @property def is_primary_key(self) -> bool: diff --git a/db2sql/domain/model/schema.py b/db2sql/domain/model/schema.py index df07e29..e6b48a8 100644 --- a/db2sql/domain/model/schema.py +++ b/db2sql/domain/model/schema.py @@ -16,6 +16,7 @@ class Schema: name: str tables: Dict[str, Table] = field(default_factory=dict) + comment: Optional[str] = None def add_table(self, table: Table) -> None: if table.name in self.tables: diff --git a/db2sql/domain/model/table.py b/db2sql/domain/model/table.py index 48deb37..6a05d26 100644 --- a/db2sql/domain/model/table.py +++ b/db2sql/domain/model/table.py @@ -20,6 +20,7 @@ class Table: indexes: Dict[str, List[str]] = field(default_factory=dict) foreign_keys: List[ForeignKey] = field(default_factory=list) source_query: Optional[str] = None + comment: Optional[str] = None def add_column(self, column: Column) -> None: if column.name in self.columns: diff --git a/db2sql/infrastructure/emit/mssql/emitter.py b/db2sql/infrastructure/emit/mssql/emitter.py index 949aeb4..fa7bb58 100644 --- a/db2sql/infrastructure/emit/mssql/emitter.py +++ b/db2sql/infrastructure/emit/mssql/emitter.py @@ -4,7 +4,7 @@ import re import warnings -from typing import Any, Dict, Iterable, Mapping, Optional +from typing import Any, Dict, Iterable, Mapping, Optional, Tuple from db2sql.application.ports import OutputSink from db2sql.domain.model import Column, Database, Schema, Table @@ -246,13 +246,34 @@ def emit_epilogue(self, sink: OutputSink) -> None: sink.write("COMMIT TRANSACTION;\n") sink.boundary() + @staticmethod + def _string_literal(value: str) -> str: + escaped = value.replace("'", "''") + return f"N'{escaped}'" + + def _description_arguments(self, comment: str, levels: Iterable[Tuple[str, str]]) -> str: + """Arguments of ``sp_addextendedproperty`` for one ``MS_Description``. + + The level names are the identifiers as created, not quoted: the + procedure takes them as plain strings. + """ + arguments = [ + "@name = N'MS_Description'", + f"@value = {self._string_literal(comment)}", + ] + for index, (level_type, level_name) in enumerate(levels): + arguments.append(f"@level{index}type = N'{level_type}'") + arguments.append(f"@level{index}name = {self._string_literal(level_name)}") + return ", ".join(arguments) + def emit_schemas(self, database: Database, sink: OutputSink) -> None: - emitted = set() + targets: Dict[str, Optional[str]] = {} for schema in database.schemas.values(): target = self._schema_mapping.get(schema.name, schema.name) - if target in emitted: - continue - emitted.add(target) + # Several source schemas can map to one target: the first comment wins. + if targets.get(target) is None: + targets[target] = schema.comment + for target, comment in targets.items(): quoted = self.quote_identifier(target) # CREATE SCHEMA must be the first statement in its batch — wrap in EXEC. literal = target.replace("'", "''") @@ -261,8 +282,24 @@ def emit_schemas(self, database: Database, sink: OutputSink) -> None: f" EXEC('CREATE SCHEMA {quoted}');\n" ) sink.boundary() + if comment is not None: + self._emit_schema_comment(target, comment, sink) sink.write("\n") + def _emit_schema_comment(self, target: str, comment: str, sink: OutputSink) -> None: + # The schema can exist before the script runs (``dbo`` always does) and + # already carry a description: sp_addextendedproperty then fails. + name = self._string_literal(self._normalize(target)) + arguments = self._description_arguments(comment, [("SCHEMA", self._normalize(target))]) + sink.write( + "IF EXISTS (SELECT 1 FROM sys.fn_listextendedproperty(" + f"N'MS_Description', N'SCHEMA', {name}, NULL, NULL, NULL, NULL))\n" + f" EXEC sys.sp_updateextendedproperty {arguments};\n" + "ELSE\n" + f" EXEC sys.sp_addextendedproperty {arguments};\n" + ) + sink.boundary() + def emit_drops(self, database: Database, sink: OutputSink) -> None: # Native ``DROP TABLE IF EXISTS`` syntax — requires SQL Server 2016+. for schema_name, table_name in drop_order(database): @@ -309,6 +346,30 @@ def emit_tables(self, database: Database, sink: OutputSink) -> None: sink.write(",\n".join(column_lines)) sink.write("\n);\n\n") sink.boundary() + self._emit_table_comments(schema, table, sink) + + def _emit_table_comments(self, schema: Schema, table: Table, sink: OutputSink) -> None: + # The table is created by this script, so it has no description yet. + levels = [ + ("SCHEMA", self._normalize(self._schema_mapping.get(schema.name, schema.name))), + ("TABLE", self._normalize(table.name)), + ] + emitted = False + if table.comment is not None: + arguments = self._description_arguments(table.comment, levels) + sink.write(f"EXEC sys.sp_addextendedproperty {arguments};\n") + sink.boundary() + emitted = True + for column in table.columns.values(): + if column.comment is None: + continue + column_levels = levels + [("COLUMN", self._normalize(column.name))] + arguments = self._description_arguments(column.comment, column_levels) + sink.write(f"EXEC sys.sp_addextendedproperty {arguments};\n") + sink.boundary() + emitted = True + if emitted: + sink.write("\n") def emit_foreign_keys(self, database: Database, sink: OutputSink) -> None: for schema in database.schemas.values(): diff --git a/db2sql/infrastructure/emit/postgres/emitter.py b/db2sql/infrastructure/emit/postgres/emitter.py index 2d4d3b1..7281ef2 100644 --- a/db2sql/infrastructure/emit/postgres/emitter.py +++ b/db2sql/infrastructure/emit/postgres/emitter.py @@ -359,15 +359,29 @@ def emit_epilogue(self, sink: OutputSink) -> None: sink.write("COMMIT;\n") sink.boundary() + @staticmethod + def _string_literal(value: str) -> str: + escaped = value.replace("'", "''") + return f"'{escaped}'" + + def _emit_comment(self, sink: OutputSink, target: str, comment: Optional[str]) -> None: + if comment is None: + return + sink.write(f"COMMENT ON {target} IS {self._string_literal(comment)};\n") + sink.boundary() + def emit_schemas(self, database: Database, sink: OutputSink) -> None: - emitted = set() + targets: Dict[str, Optional[str]] = {} for schema in database.schemas.values(): target = self._schema_mapping.get(schema.name, schema.name) - if target in emitted: - continue - emitted.add(target) - sink.write(f"CREATE SCHEMA IF NOT EXISTS {self.quote_identifier(target)};\n") + # Several source schemas can map to one target: the first comment wins. + if targets.get(target) is None: + targets[target] = schema.comment + for target, comment in targets.items(): + quoted = self.quote_identifier(target) + sink.write(f"CREATE SCHEMA IF NOT EXISTS {quoted};\n") sink.boundary() + self._emit_comment(sink, f"SCHEMA {quoted}", comment) sink.write("\n") def emit_drops(self, database: Database, sink: OutputSink) -> None: @@ -414,6 +428,17 @@ def emit_tables(self, database: Database, sink: OutputSink) -> None: sink.write(",\n".join(column_lines)) sink.write("\n);\n\n") sink.boundary() + self._emit_table_comments(schema, table, sink) + + def _emit_table_comments(self, schema: Schema, table: Table, sink: OutputSink) -> None: + qualified = self.table_name(schema, table) + self._emit_comment(sink, f"TABLE {qualified}", table.comment) + commented = [col for col in table.columns.values() if col.comment is not None] + for column in commented: + target = f"COLUMN {qualified}.{self.quote_identifier(column.name)}" + self._emit_comment(sink, target, column.comment) + if table.comment is not None or commented: + sink.write("\n") def emit_foreign_keys(self, database: Database, sink: OutputSink) -> None: for schema in database.schemas.values(): diff --git a/db2sql/infrastructure/persistence/mssql/reader.py b/db2sql/infrastructure/persistence/mssql/reader.py index 4ca82b0..6628a0e 100644 --- a/db2sql/infrastructure/persistence/mssql/reader.py +++ b/db2sql/infrastructure/persistence/mssql/reader.py @@ -59,6 +59,8 @@ def collect_metadata(self) -> Database: self._read_foreign_keys(database) self._logger.info("reading indexes") self._read_indexes(database) + self._logger.info("reading comments") + self._read_comments(database) except Exception as exception: self._logger.error(str(exception)) raise SourceReaderError("failed to collect database information") from exception @@ -276,3 +278,48 @@ def _read_indexes(self, database: Database) -> None: table = database.get_table(row.TABLE_SCHEMA, row.TABLE_NAME) if table: table.add_index(row.INDEX_NAME, row.COLUMN_NAME) + + def _read_comments(self, database: Database) -> None: + # SQL Server has no COMMENT ON. SSMS and most tools store the comment + # as the MS_Description extended property: class 3 on a schema, class 1 + # on a table (minor_id 0) or on one of its columns (minor_id = column_id). + r: engine.Result[Any] = self._ensure_session().execute(text(""" +SELECT s.name AS TABLE_SCHEMA, + NULL AS TABLE_NAME, + NULL AS COLUMN_NAME, + CAST(ep.value AS NVARCHAR(MAX)) AS COMMENT +FROM sys.extended_properties ep +INNER JOIN sys.schemas s + ON ep.major_id = s.schema_id +WHERE ep.class = 3 + AND ep.name = 'MS_Description' +UNION ALL +SELECT s.name, t.name, c.name, CAST(ep.value AS NVARCHAR(MAX)) +FROM sys.extended_properties ep +INNER JOIN sys.tables t + ON ep.major_id = t.object_id +INNER JOIN sys.schemas s + ON s.schema_id = t.schema_id +LEFT JOIN sys.columns c + ON c.object_id = t.object_id + AND c.column_id = ep.minor_id +WHERE ep.class = 1 + AND ep.name = 'MS_Description' + AND (ep.minor_id = 0 OR c.column_id IS NOT NULL) + """)) + + for row in r: + if row.TABLE_NAME is None: + schema = database.get_schema(row.TABLE_SCHEMA) + if schema is not None: + schema.comment = row.COMMENT + continue + table = database.get_table(row.TABLE_SCHEMA, row.TABLE_NAME) + if table is None: + continue + if row.COLUMN_NAME is None: + table.comment = row.COMMENT + continue + column = table.get_column(row.COLUMN_NAME) + if column is not None: + column.comment = row.COMMENT diff --git a/db2sql/infrastructure/persistence/mysql/reader.py b/db2sql/infrastructure/persistence/mysql/reader.py index c90fa4f..54df2f9 100644 --- a/db2sql/infrastructure/persistence/mysql/reader.py +++ b/db2sql/infrastructure/persistence/mysql/reader.py @@ -69,21 +69,24 @@ def collect_metadata(self) -> Database: def _read_tables(self, database: Database) -> None: rows = self._ensure_session().execute( text( - "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES " + "SELECT TABLE_NAME, TABLE_COMMENT FROM INFORMATION_SCHEMA.TABLES " "WHERE TABLE_SCHEMA = :schema AND TABLE_TYPE = 'BASE TABLE' " "ORDER BY TABLE_NAME" ), {"schema": self._database_name}, ) for row in rows: - database.add_table(self._database_name, Table(row.TABLE_NAME)) + database.add_table( + self._database_name, + Table(row.TABLE_NAME, comment=row.TABLE_COMMENT or None), + ) def _read_columns(self, database: Database) -> None: rows = self._ensure_session().execute( text( "SELECT TABLE_NAME, COLUMN_NAME, COLUMN_DEFAULT, IS_NULLABLE, " "DATA_TYPE, CHARACTER_MAXIMUM_LENGTH, NUMERIC_PRECISION, " - "NUMERIC_SCALE, EXTRA " + "NUMERIC_SCALE, EXTRA, COLUMN_COMMENT " "FROM INFORMATION_SCHEMA.COLUMNS " "WHERE TABLE_SCHEMA = :schema " "ORDER BY TABLE_NAME, ORDINAL_POSITION" @@ -102,6 +105,8 @@ def _read_columns(self, database: Database) -> None: char_length=row.CHARACTER_MAXIMUM_LENGTH or -1, precision=row.NUMERIC_PRECISION, scale=row.NUMERIC_SCALE, + # MySQL reports "no comment" as an empty string, not NULL. + comment=row.COLUMN_COMMENT or None, ) if row.EXTRA and "auto_increment" in row.EXTRA.lower(): column.identity = True diff --git a/db2sql/infrastructure/persistence/oracle/reader.py b/db2sql/infrastructure/persistence/oracle/reader.py index fc54b05..2e9c080 100644 --- a/db2sql/infrastructure/persistence/oracle/reader.py +++ b/db2sql/infrastructure/persistence/oracle/reader.py @@ -139,6 +139,8 @@ def collect_metadata(self) -> Database: self._read_indexes(database) self._logger.info("reading identity columns ...") self._read_identity_columns(database) + self._logger.info("reading comments ...") + self._read_comments(database) except SourceReaderError: raise except Exception as exc: @@ -361,6 +363,45 @@ def _read_identity_columns(self, database: Database) -> None: if column is not None: column.identity = True + def _read_comments(self, database: Database) -> None: + """Oracle has no comment on a schema, only on tables and columns.""" + owner = self._schema_filter + params: Dict[str, Any] = {} + if owner: + owner_clause = "OWNER = :owner" + params["owner"] = owner + else: + owner_clause = self._excluded_schemas_sql("OWNER") + session = self._ensure_session() + tables = session.execute( + text( + "SELECT OWNER, TABLE_NAME, COMMENTS " + "FROM ALL_TAB_COMMENTS " + f"WHERE {owner_clause} " + " AND TABLE_TYPE = 'TABLE' AND COMMENTS IS NOT NULL" + ), + params, + ) + for row in tables: + table = database.get_table(row.owner, row.table_name) + if table is not None: + table.comment = row.comments + columns = session.execute( + text( + "SELECT OWNER, TABLE_NAME, COLUMN_NAME, COMMENTS " + "FROM ALL_COL_COMMENTS " + f"WHERE {owner_clause} AND COMMENTS IS NOT NULL" + ), + params, + ) + for row in columns: + table = database.get_table(row.owner, row.table_name) + if table is None: + continue + column = table.get_column(row.column_name) + if column is not None: + column.comment = row.comments + def iter_rows(self, schema: str, table: Table, limit: int = -1) -> Iterator[Tuple[Any, ...]]: session = self._ensure_session() columns = ", ".join(f'"{name}"' for name in table.columns) diff --git a/db2sql/infrastructure/persistence/postgres/reader.py b/db2sql/infrastructure/persistence/postgres/reader.py index 1c48b53..1631b19 100644 --- a/db2sql/infrastructure/persistence/postgres/reader.py +++ b/db2sql/infrastructure/persistence/postgres/reader.py @@ -19,6 +19,7 @@ from db2sql.infrastructure.url import build_url, redact_url _SYSTEM_SCHEMAS = ("pg_catalog", "information_schema", "pg_toast") +_PUBLIC_SCHEMA_DEFAULT_COMMENT = ("public", "standard public schema") class PostgresSourceReader: @@ -50,6 +51,7 @@ def collect_metadata(self) -> Database: self._read_constraints(database) self._read_foreign_keys(database) self._read_indexes(database) + self._read_comments(database) except Exception as exc: self._logger.error(str(exc)) raise SourceReaderError("failed to collect database information") from exc @@ -175,6 +177,49 @@ def _read_indexes(self, database: Database) -> None: if table: table.add_index(row.index_name, row.column_name) + def _read_comments(self, database: Database) -> None: + # One row per COMMENT ON: objsubid 0 is the object itself, a positive + # objsubid is the attnum of a column of that table. + rows = self._ensure_session().execute( + text( + "SELECT n.nspname AS schema_name, NULL AS table_name, " + " 0 AS column_number, NULL AS column_name, d.description " + "FROM pg_description d " + "JOIN pg_namespace n ON n.oid = d.objoid " + "WHERE d.classoid = 'pg_namespace'::regclass " + f" AND n.nspname NOT IN {_SYSTEM_SCHEMAS} " + "UNION ALL " + "SELECT n.nspname, c.relname, d.objsubid, a.attname, d.description " + "FROM pg_description d " + "JOIN pg_class c ON c.oid = d.objoid " + "JOIN pg_namespace n ON n.oid = c.relnamespace " + "LEFT JOIN pg_attribute a " + " ON a.attrelid = c.oid AND a.attnum = d.objsubid " + "WHERE d.classoid = 'pg_class'::regclass " + " AND c.relkind IN ('r', 'p') " + f" AND n.nspname NOT IN {_SYSTEM_SCHEMAS}" + ) + ) + for row in rows: + if row.table_name is None: + schema = database.get_schema(row.schema_name) + # PG ships ``public`` with this comment. Copying it would make + # the dump COMMENT ON a schema that the target role usually + # does not own, which fails the whole transaction. + builtin = (row.schema_name, row.description) == _PUBLIC_SCHEMA_DEFAULT_COMMENT + if schema is not None and not builtin: + schema.comment = row.description + continue + table = database.get_table(row.schema_name, row.table_name) + if table is None: + continue + if row.column_number == 0: + table.comment = row.description + continue + column = table.get_column(row.column_name) if row.column_name else None + if column is not None: + column.comment = row.description + def iter_rows(self, schema: str, table: Table, limit: int = -1) -> Iterator[Tuple[Any, ...]]: session = self._ensure_session() columns = ", ".join(f'"{name}"' for name in table.columns) diff --git a/docs/dialect-mapping.rst b/docs/dialect-mapping.rst index 4177f91..257a3fb 100644 --- a/docs/dialect-mapping.rst +++ b/docs/dialect-mapping.rst @@ -302,6 +302,58 @@ the literal. ANSI-compatible keywords (``CURRENT_USER``, ``SESSION_USER``, Anything not in the table is passed through unchanged. +Comments +-------- + +The comments of the source schemas, tables and columns are copied to the +target, in dump mode and in migrate mode. With ``--on-existing truncate``, no +DDL is emitted, so no comment is copied. + +.. list-table:: + :header-rows: 1 + :widths: 20 25 25 30 + + * - Source + - Schema + - Table + - Column + * - PostgreSQL + - ``COMMENT ON SCHEMA`` + - ``COMMENT ON TABLE`` + - ``COMMENT ON COLUMN`` + * - MSSQL + - ``MS_Description`` property + - ``MS_Description`` property + - ``MS_Description`` property + * - MySQL / MariaDB + - not read + - ``TABLE_COMMENT`` + - ``COLUMN_COMMENT`` + * - Oracle + - none in Oracle + - ``ALL_TAB_COMMENTS`` + - ``ALL_COL_COMMENTS`` + * - SQLite + - none in SQLite + - none in SQLite + - none in SQLite + +The PostgreSQL target receives ``COMMENT ON`` statements. The MSSQL target +receives ``sys.sp_addextendedproperty`` calls for the ``MS_Description`` +property, which SSMS shows as the description. A schema can exist before the +script runs, so its description is updated if it exists and added if not. + +Two rules apply: + +* The comment ``standard public schema``, which PostgreSQL puts on ``public``, + is not copied. ``COMMENT ON SCHEMA`` needs the owner of the schema, and the + role that replays the dump is usually not the owner of ``public``. +* When ``schema_mapping`` sends several source schemas to one target schema, + the comment of the first source schema that has a comment is used. + +A table synthesized from a view export has no comment. + + Overriding the mapping ---------------------- diff --git a/tests/unit/infrastructure/emit/test_mssql_emitter.py b/tests/unit/infrastructure/emit/test_mssql_emitter.py index c8b349f..dfb01f1 100644 --- a/tests/unit/infrastructure/emit/test_mssql_emitter.py +++ b/tests/unit/infrastructure/emit/test_mssql_emitter.py @@ -366,3 +366,66 @@ def test_copy_warning_is_emitted_only_once(self) -> None: emitter.emit_data_copy(schema, table, [(1, "a")], _Sink()) emitter.emit_data_copy(schema, table, [(2, "b")], _Sink()) assert sum(1 for w in caught if "COPY is Postgres-only" in str(w.message)) == 1 + + +class TestComments: + def _db(self) -> Database: + db = Database(name="main") + sales = Schema(name="Sales", comment="Orders and customers") + customer = Table(name="Customer", comment="People who buy") + customer.add_column(Column(name="Id", type="int", constraint="PRIMARY KEY")) + customer.add_column(Column(name="Name", type="text", comment="Name, as it's written")) + sales.add_table(customer) + db.add_schema(sales) + return db + + def test_schema_comment_is_updated_or_added(self) -> None: + emitter = MssqlSqlEmitter(preserve_case=True) + sink = _Sink() + emitter.emit_schemas(self._db(), sink) + arguments = ( + "@name = N'MS_Description', @value = N'Orders and customers', " + "@level0type = N'SCHEMA', @level0name = N'Sales'" + ) + assert sink.text.endswith( + "IF EXISTS (SELECT 1 FROM sys.fn_listextendedproperty(" + "N'MS_Description', N'SCHEMA', N'Sales', NULL, NULL, NULL, NULL))\n" + f" EXEC sys.sp_updateextendedproperty {arguments};\n" + "ELSE\n" + f" EXEC sys.sp_addextendedproperty {arguments};\n" + "\n" + ) + assert sink.boundaries == 2 + + def test_table_and_column_comments_follow_create_table(self) -> None: + emitter = MssqlSqlEmitter(preserve_case=False, schema_mapping={"Sales": "dbo"}) + sink = _Sink() + emitter.emit_tables(self._db(), sink) + comments = sink.text.split(");\n\n", 1)[1] + assert comments == ( + "EXEC sys.sp_addextendedproperty @name = N'MS_Description', " + "@value = N'People who buy', " + "@level0type = N'SCHEMA', @level0name = N'dbo', " + "@level1type = N'TABLE', @level1name = N'customer';\n" + "EXEC sys.sp_addextendedproperty @name = N'MS_Description', " + "@value = N'Name, as it''s written', " + "@level0type = N'SCHEMA', @level0name = N'dbo', " + "@level1type = N'TABLE', @level1name = N'customer', " + "@level2type = N'COLUMN', @level2name = N'name';\n" + "\n" + ) + assert sink.boundaries == 3 + + def test_nothing_is_emitted_without_comments(self) -> None: + emitter = MssqlSqlEmitter(preserve_case=True) + db = Database(name="main") + schema = Schema(name="dbo") + table = Table(name="t") + table.add_column(Column(name="id", type="int")) + schema.add_table(table) + db.add_schema(schema) + sink = _Sink() + emitter.emit_schemas(db, sink) + emitter.emit_tables(db, sink) + assert "extendedproperty" not in sink.text + assert sink.text.endswith(");\n\n") diff --git a/tests/unit/infrastructure/emit/test_postgres_emitter.py b/tests/unit/infrastructure/emit/test_postgres_emitter.py index 3899e69..7eee4aa 100644 --- a/tests/unit/infrastructure/emit/test_postgres_emitter.py +++ b/tests/unit/infrastructure/emit/test_postgres_emitter.py @@ -640,3 +640,75 @@ def test_column_definition_keeps_its_one_argument_form(self) -> None: emitter = PostgresSqlEmitter(preserve_case=True) column = Column(name="client_id", type="varchar", char_length=36, nullable=True) assert emitter.column_definition(column) == '"client_id" varchar(36)' + + +class TestComments: + def _db(self) -> Database: + db = Database(name="main") + sales = Schema(name="Sales", comment="Orders and customers") + customer = Table(name="Customer", comment="People who buy") + customer.add_column(Column(name="Id", type="int", constraint="PRIMARY KEY")) + customer.add_column(Column(name="Name", type="text", comment="Name, as it's written")) + sales.add_table(customer) + db.add_schema(sales) + return db + + def test_schema_comment_follows_create_schema(self) -> None: + emitter = PostgresSqlEmitter(preserve_case=True) + sink = _Sink() + emitter.emit_schemas(self._db(), sink) + assert sink.text == ( + 'CREATE SCHEMA IF NOT EXISTS "Sales";\n' + "COMMENT ON SCHEMA \"Sales\" IS 'Orders and customers';\n" + "\n" + ) + assert sink.boundaries == 2 + + def test_table_and_column_comments_follow_create_table(self) -> None: + emitter = PostgresSqlEmitter(preserve_case=False) + sink = _Sink() + emitter.emit_tables(self._db(), sink) + comments = sink.text.split(");\n\n", 1)[1] + assert comments == ( + "COMMENT ON TABLE \"sales\".\"customer\" IS 'People who buy';\n" + "COMMENT ON COLUMN \"sales\".\"customer\".\"name\" IS 'Name, as it''s written';\n" + "\n" + ) + assert sink.boundaries == 3 + + def test_mapped_schema_carries_the_comment(self) -> None: + emitter = PostgresSqlEmitter(preserve_case=True, schema_mapping={"Sales": "public"}) + sink = _Sink() + db = self._db() + emitter.emit_schemas(db, sink) + emitter.emit_tables(db, sink) + assert "COMMENT ON SCHEMA \"public\" IS 'Orders and customers';" in sink.text + assert 'COMMENT ON TABLE "public"."Customer"' in sink.text + + def test_first_comment_wins_when_two_schemas_share_a_target(self) -> None: + emitter = PostgresSqlEmitter( + preserve_case=True, schema_mapping={"a": "common", "b": "common", "c": "common"} + ) + db = Database(name="main") + db.add_schema(Schema(name="a")) + db.add_schema(Schema(name="b", comment="from b")) + db.add_schema(Schema(name="c", comment="from c")) + sink = _Sink() + emitter.emit_schemas(db, sink) + assert sink.text.count("CREATE SCHEMA") == 1 + assert sink.text.count("COMMENT ON") == 1 + assert "IS 'from b'" in sink.text + + def test_nothing_is_emitted_without_comments(self) -> None: + emitter = PostgresSqlEmitter(preserve_case=True) + db = Database(name="main") + schema = Schema(name="public") + table = Table(name="t") + table.add_column(Column(name="id", type="int")) + schema.add_table(table) + db.add_schema(schema) + sink = _Sink() + emitter.emit_schemas(db, sink) + emitter.emit_tables(db, sink) + assert "COMMENT" not in sink.text + assert sink.text.endswith(");\n\n") diff --git a/tests/unit/infrastructure/persistence/test_mssql_reader.py b/tests/unit/infrastructure/persistence/test_mssql_reader.py index 2670e57..fef79e2 100644 --- a/tests/unit/infrastructure/persistence/test_mssql_reader.py +++ b/tests/unit/infrastructure/persistence/test_mssql_reader.py @@ -324,3 +324,32 @@ def test_ensure_session_is_cached() -> None: def test_build_reader_returns_instance() -> None: assert isinstance(build_reader(_config(dbname="d"), _StubLogger()), MSSQLSourceReader) + + +def test_collect_metadata_reads_ms_description_properties() -> None: + session = _populated_session() + session.add( + "sys.extended_properties", + [ + FakeRow(TABLE_SCHEMA="audit", TABLE_NAME=None, COLUMN_NAME=None, COMMENT="Audit trail"), + FakeRow(TABLE_SCHEMA="sales", TABLE_NAME=None, COLUMN_NAME=None, COMMENT="no table"), + FakeRow( + TABLE_SCHEMA="dbo", TABLE_NAME="Customer", COLUMN_NAME=None, COMMENT="Customers" + ), + FakeRow(TABLE_SCHEMA="dbo", TABLE_NAME="Customer", COLUMN_NAME="Id", COMMENT="Key"), + FakeRow(TABLE_SCHEMA="dbo", TABLE_NAME="Customer", COLUMN_NAME="Gone", COMMENT="x"), + FakeRow(TABLE_SCHEMA="dbo", TABLE_NAME="Ghost", COLUMN_NAME=None, COMMENT="x"), + ], + ) + reader = MSSQLSourceReader(_config(dbname="d"), _StubLogger()) + install_fake_session(reader, session) + + db = reader.collect_metadata() + + assert db.schemas["audit"].comment == "Audit trail" + assert db.schemas["dbo"].comment is None + assert "sales" not in db.schemas + customer = db.schemas["dbo"].tables["Customer"] + assert customer.comment == "Customers" + assert customer.columns["Id"].comment == "Key" + assert db.schemas["dbo"].tables["Order"].comment is None diff --git a/tests/unit/infrastructure/persistence/test_mysql_reader.py b/tests/unit/infrastructure/persistence/test_mysql_reader.py index 5177470..27bf217 100644 --- a/tests/unit/infrastructure/persistence/test_mysql_reader.py +++ b/tests/unit/infrastructure/persistence/test_mysql_reader.py @@ -40,7 +40,10 @@ def _full_plan() -> FakeSession: session = FakeSession() session.add( "FROM INFORMATION_SCHEMA.TABLES", - [FakeRow(TABLE_NAME="author"), FakeRow(TABLE_NAME="book")], + [ + FakeRow(TABLE_NAME="author", TABLE_COMMENT="Book authors"), + FakeRow(TABLE_NAME="book", TABLE_COMMENT=""), + ], ) session.add( "FROM INFORMATION_SCHEMA.COLUMNS", @@ -55,6 +58,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=10, NUMERIC_SCALE=0, EXTRA="auto_increment", + COLUMN_COMMENT="", ), FakeRow( TABLE_NAME="author", @@ -66,6 +70,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=None, NUMERIC_SCALE=None, EXTRA="", + COLUMN_COMMENT="Full name", ), FakeRow( TABLE_NAME="book", @@ -77,6 +82,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=10, NUMERIC_SCALE=0, EXTRA="", + COLUMN_COMMENT="", ), FakeRow( TABLE_NAME="author", @@ -88,6 +94,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=None, NUMERIC_SCALE=None, EXTRA="", + COLUMN_COMMENT="", ), FakeRow( TABLE_NAME="book", @@ -99,6 +106,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=None, NUMERIC_SCALE=None, EXTRA="", + COLUMN_COMMENT="", ), # Belongs to a missing table — should be ignored gracefully FakeRow( @@ -111,6 +119,7 @@ def _full_plan() -> FakeSession: NUMERIC_PRECISION=10, NUMERIC_SCALE=0, EXTRA="", + COLUMN_COMMENT="", ), ], ) @@ -223,6 +232,18 @@ def test_collect_metadata_populates_schema_tables_columns_indexes_fks() -> None: assert book.indexes == {"idx_book_author": ["author_id"]} +def test_collect_metadata_keeps_comments_and_reads_empty_as_none() -> None: + reader = MySQLSourceReader(_config(dbname="main"), _SilentLogger()) + install_fake_session(reader, _full_plan()) + + schema = reader.collect_metadata().schemas["main"] + + assert schema.tables["author"].comment == "Book authors" + assert schema.tables["author"].columns["name"].comment == "Full name" + assert schema.tables["book"].comment is None + assert schema.tables["author"].columns["id"].comment is None + + def test_collect_metadata_wraps_unexpected_exception() -> None: reader = MySQLSourceReader(_config(dbname="main"), _SilentLogger()) diff --git a/tests/unit/infrastructure/persistence/test_oracle_reader.py b/tests/unit/infrastructure/persistence/test_oracle_reader.py index e706be2..2c84075 100644 --- a/tests/unit/infrastructure/persistence/test_oracle_reader.py +++ b/tests/unit/infrastructure/persistence/test_oracle_reader.py @@ -487,3 +487,32 @@ def _execute(statement, params=None): with pytest.raises(SourceReaderError) as exc: reader.collect_metadata() assert "failed to collect database information" in str(exc.value) + + +def test_collect_metadata_reads_table_and_column_comments() -> None: + reader = _build_reader() + session = _full_plan() + session.add( + "FROM ALL_TAB_COMMENTS", + [ + FakeRow(owner="HR", table_name="EMP", comments="Employees"), + FakeRow(owner="HR", table_name="GHOST", comments="not collected"), + ], + ) + session.add( + "FROM ALL_COL_COMMENTS", + [ + FakeRow(owner="HR", table_name="EMP", column_name="NAME", comments="Full name"), + FakeRow(owner="HR", table_name="EMP", column_name="GONE", comments="x"), + ], + ) + install_fake_session(reader, session) + + table = reader.collect_metadata().get_table("HR", "EMP") + + assert table is not None + assert table.comment == "Employees" + assert table.columns["NAME"].comment == "Full name" + assert table.columns["ID"].comment is None + queries = " ".join(query for query, _ in session.executed) + assert "TABLE_TYPE = 'TABLE'" in queries diff --git a/tests/unit/infrastructure/persistence/test_postgres_reader.py b/tests/unit/infrastructure/persistence/test_postgres_reader.py index ae0027f..d4a574e 100644 --- a/tests/unit/infrastructure/persistence/test_postgres_reader.py +++ b/tests/unit/infrastructure/persistence/test_postgres_reader.py @@ -290,3 +290,101 @@ def test_ensure_session_is_cached() -> None: def test_build_reader_returns_instance() -> None: assert isinstance(build_reader(_config(dbname="d"), _StubLogger()), PostgresSourceReader) + + +def test_collect_metadata_reads_schema_table_and_column_comments() -> None: + session = _populated_session() + session.add( + "FROM pg_description", + [ + FakeRow( + schema_name="other", + table_name=None, + column_number=0, + column_name=None, + description="Things that are not books", + ), + FakeRow( + schema_name="public", + table_name="author", + column_number=0, + column_name=None, + description="Book authors", + ), + FakeRow( + schema_name="public", + table_name="author", + column_number=2, + column_name="state", + description="It's an 'A' or an 'I'", + ), + # A comment left on a dropped column: no attribute matches it. + FakeRow( + schema_name="public", + table_name="book", + column_number=7, + column_name=None, + description="orphan", + ), + FakeRow( + schema_name="ghost", + table_name="x", + column_number=0, + column_name=None, + description="table not collected", + ), + ], + ) + reader = PostgresSourceReader(_config(dbname="d"), _StubLogger()) + install_fake_session(reader, session) + + db = reader.collect_metadata() + + assert db.schemas["other"].comment == "Things that are not books" + author = db.schemas["public"].tables["author"] + assert author.comment == "Book authors" + assert author.columns["state"].comment == "It's an 'A' or an 'I'" + assert author.columns["id"].comment is None + book = db.schemas["public"].tables["book"] + assert book.comment is None + assert all(column.comment is None for column in book.columns.values()) + + +def test_collect_metadata_skips_the_builtin_comment_of_public() -> None: + session = _populated_session() + session.add( + "FROM pg_description", + [ + FakeRow( + schema_name="public", + table_name=None, + column_number=0, + column_name=None, + description="standard public schema", + ), + ], + ) + reader = PostgresSourceReader(_config(dbname="d"), _StubLogger()) + install_fake_session(reader, session) + + assert reader.collect_metadata().schemas["public"].comment is None + + +def test_collect_metadata_keeps_the_builtin_text_on_another_schema() -> None: + session = _populated_session() + session.add( + "FROM pg_description", + [ + FakeRow( + schema_name="other", + table_name=None, + column_number=0, + column_name=None, + description="standard public schema", + ), + ], + ) + reader = PostgresSourceReader(_config(dbname="d"), _StubLogger()) + install_fake_session(reader, session) + + assert reader.collect_metadata().schemas["other"].comment == "standard public schema"