From b5afb7d726888068b972361193294041dd930262 Mon Sep 17 00:00:00 2001 From: Dotan Mor <52997876+dotan-mor@users.noreply.github.com> Date: Mon, 9 Sep 2024 23:49:57 +0300 Subject: [PATCH] add column comment to get_columns method (#253) * add column comment to get_columns method --------- Co-authored-by: Gord Thompson --- sqlalchemy_cockroachdb/base.py | 8 +- test/test_column_reflect.py | 5 + test/test_suite_sqlalchemy.py | 244 ++++++++++++++++++--------------- 3 files changed, 141 insertions(+), 116 deletions(-) diff --git a/sqlalchemy_cockroachdb/base.py b/sqlalchemy_cockroachdb/base.py index acb433d..19f8a97 100644 --- a/sqlalchemy_cockroachdb/base.py +++ b/sqlalchemy_cockroachdb/base.py @@ -182,9 +182,10 @@ def get_columns(self, conn, table_name, schema=None, **kw): if not self._is_v191plus: # v2.x does not have is_generated or generation_expression sql = ( - "SELECT column_name, data_type, is_nullable::bool, column_default, " + "SELECT column_name, data_type, is_nullable::bool, column_default," "numeric_precision, numeric_scale, character_maximum_length, " - "NULL AS is_generated, NULL AS generation_expression, is_hidden::bool " + "NULL AS is_generated, NULL AS generation_expression, is_hidden::bool," + "column_comment AS comment " "FROM information_schema.columns " "WHERE table_schema = :table_schema AND table_name = :table_name " ) @@ -200,7 +201,7 @@ def get_columns(self, conn, table_name, schema=None, **kw): "numeric_precision, numeric_scale, character_maximum_length, " "CASE is_generated WHEN 'ALWAYS' THEN true WHEN 'NEVER' THEN false " "ELSE is_generated::bool END AS is_generated, " - "generation_expression, is_hidden::bool, crdb_sql_type " + "generation_expression, is_hidden::bool, crdb_sql_type, column_comment AS comment " "FROM information_schema.columns " "WHERE table_schema = :table_schema AND table_name = :table_name " ) @@ -281,6 +282,7 @@ def get_columns(self, conn, table_name, schema=None, **kw): default=default, autoincrement=autoincrement, is_hidden=row.is_hidden, + comment=row.comment, ) if computed is not None: column_info["computed"] = computed diff --git a/test/test_column_reflect.py b/test/test_column_reflect.py index 032b3f7..4853cdb 100644 --- a/test/test_column_reflect.py +++ b/test/test_column_reflect.py @@ -44,6 +44,7 @@ def test_reflect_hidden_columns(self): "default": "unique_rowid()", "autoincrement": True, "is_hidden": False, + "comment": None, }, { "name": "txt", @@ -52,6 +53,7 @@ def test_reflect_hidden_columns(self): "default": None, "autoincrement": False, "is_hidden": False, + "comment": None, }, ], ) @@ -66,6 +68,7 @@ def test_reflect_hidden_columns(self): "default": None, "autoincrement": False, "is_hidden": False, + "comment": None, }, ], ) @@ -80,6 +83,7 @@ def test_reflect_hidden_columns(self): "default": None, "autoincrement": False, "is_hidden": False, + "comment": None, }, { "name": "rowid", @@ -88,6 +92,7 @@ def test_reflect_hidden_columns(self): "default": "unique_rowid()", "autoincrement": True, "is_hidden": True, + "comment": None, }, ], ) diff --git a/test/test_suite_sqlalchemy.py b/test/test_suite_sqlalchemy.py index e8e00ec..213f02a 100644 --- a/test/test_suite_sqlalchemy.py +++ b/test/test_suite_sqlalchemy.py @@ -4,14 +4,20 @@ from sqlalchemy.testing.suite import ( BizarroCharacterFKResolutionTest as _BizarroCharacterFKResolutionTest, ) -from sqlalchemy.testing.suite import ComponentReflectionTest as _ComponentReflectionTest +from sqlalchemy.testing.suite import ( + ComponentReflectionTest as _ComponentReflectionTest, +) from sqlalchemy.testing.suite import HasIndexTest as _HasIndexTest from sqlalchemy.testing.suite import HasTableTest as _HasTableTest from sqlalchemy.testing.suite import InsertBehaviorTest as _InsertBehaviorTest from sqlalchemy.testing.suite import IsolationLevelTest as _IsolationLevelTest -from sqlalchemy.testing.suite import LongNameBlowoutTest as _LongNameBlowoutTest +from sqlalchemy.testing.suite import ( + LongNameBlowoutTest as _LongNameBlowoutTest, +) from sqlalchemy.testing.suite import NumericTest as _NumericTest -from sqlalchemy.testing.suite import QuotedNameArgumentTest as _QuotedNameArgumentTest +from sqlalchemy.testing.suite import ( + QuotedNameArgumentTest as _QuotedNameArgumentTest, +) from sqlalchemy.testing.suite import TrueDivTest as _TrueDivTest from sqlalchemy.testing.suite import UnicodeSchemaTest as _UnicodeSchemaTest @@ -27,17 +33,13 @@ class BizarroCharacterFKResolutionTest(_BizarroCharacterFKResolutionTest): argnames="tablename", ) def test_fk_ref(self, connection, metadata, use_composite, tablename, columnname): - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_fk_ref(connection, metadata, use_composite, tablename, columnname) class ComponentReflectionTest(_ComponentReflectionTest): def test_get_indexes(self, connection): - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_get_indexes(connection, None, None) @skip("cockroachdb") @@ -49,176 +51,202 @@ def test_get_multi_columns(self): insp = inspect(config.db) actual = insp.get_multi_columns() expected = { - (None, "comment_test"): [ + (None, "users"): [ { - "autoincrement": True, + "name": "user_id", + "type": INTEGER(), + "nullable": False, "default": "unique_rowid()", + "autoincrement": True, "is_hidden": False, - "name": "id", - "nullable": False, - "type": INTEGER(), + "comment": None, }, { - "autoincrement": False, + "name": "test1", + "type": VARCHAR(length=5), + "nullable": False, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "data", - "nullable": True, - "type": VARCHAR(length=20), + "comment": None, }, { - "autoincrement": False, + "name": "test2", + "type": FLOAT(), + "nullable": False, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "d2", - "nullable": True, - "type": VARCHAR(length=20), + "comment": None, }, { - "autoincrement": False, + "name": "parent_user_id", + "type": INTEGER(), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "d3", - "nullable": True, - "type": VARCHAR(length=42), + "comment": None, }, ], - (None, "dingalings"): [ + (None, "comment_test"): [ { - "autoincrement": True, + "name": "id", + "type": INTEGER(), + "nullable": False, "default": "unique_rowid()", + "autoincrement": True, "is_hidden": False, - "name": "dingaling_id", - "nullable": False, - "type": INTEGER(), + "comment": "id comment", }, { - "autoincrement": False, - "default": None, - "is_hidden": False, - "name": "address_id", + "name": "data", + "type": VARCHAR(length=20), "nullable": True, - "type": INTEGER(), - }, - { - "autoincrement": False, "default": None, - "is_hidden": False, - "name": "id_user", - "nullable": True, - "type": INTEGER(), - }, - { "autoincrement": False, - "default": None, - "is_hidden": False, - "name": "data", - "nullable": True, - "type": VARCHAR(length=30), - }, - ], - (None, "email_addresses"): [ - { - "autoincrement": True, - "default": "unique_rowid()", "is_hidden": False, - "name": "address_id", - "nullable": False, - "type": INTEGER(), + "comment": "data % comment", }, { - "autoincrement": False, + "name": "d2", + "type": VARCHAR(length=20), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "remote_user_id", - "nullable": True, - "type": INTEGER(), + "comment": "Comment types type speedily ' \" \\ '' Fun!", }, { - "autoincrement": False, + "name": "d3", + "type": VARCHAR(length=42), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "email_address", - "nullable": True, - "type": VARCHAR(length=20), + "comment": "Comment\nwith\rescapes", }, ], (None, "no_constraints"): [ { - "autoincrement": False, - "default": None, - "is_hidden": False, "name": "data", - "nullable": True, "type": VARCHAR(length=20), + "nullable": True, + "default": None, + "autoincrement": False, + "is_hidden": False, + "comment": None, } ], (None, "noncol_idx_test_nopk"): [ { - "autoincrement": False, - "default": None, - "is_hidden": False, "name": "q", - "nullable": True, "type": VARCHAR(length=5), + "nullable": True, + "default": None, + "autoincrement": False, + "is_hidden": False, + "comment": None, } ], (None, "noncol_idx_test_pk"): [ { - "autoincrement": True, - "default": "unique_rowid()", - "is_hidden": False, "name": "id", - "nullable": False, "type": INTEGER(), + "nullable": False, + "default": "unique_rowid()", + "autoincrement": True, + "is_hidden": False, + "comment": None, }, { - "autoincrement": False, - "default": None, - "is_hidden": False, "name": "q", - "nullable": True, "type": VARCHAR(length=5), + "nullable": True, + "default": None, + "autoincrement": False, + "is_hidden": False, + "comment": None, }, ], - (None, "users"): [ + (None, "email_addresses"): [ { - "autoincrement": True, + "name": "address_id", + "type": INTEGER(), + "nullable": False, "default": "unique_rowid()", + "autoincrement": True, "is_hidden": False, - "name": "user_id", - "nullable": False, - "type": INTEGER(), + "comment": None, }, { - "autoincrement": False, + "name": "remote_user_id", + "type": INTEGER(), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "test1", - "nullable": False, - "type": VARCHAR(length=5), + "comment": None, }, { - "autoincrement": False, + "name": "email_address", + "type": VARCHAR(length=20), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "test2", + "comment": None, + }, + ], + (None, "dingalings"): [ + { + "name": "dingaling_id", + "type": INTEGER(), "nullable": False, - "type": FLOAT(), + "default": "unique_rowid()", + "autoincrement": True, + "is_hidden": False, + "comment": None, }, { + "name": "address_id", + "type": INTEGER(), + "nullable": True, + "default": None, "autoincrement": False, + "is_hidden": False, + "comment": None, + }, + { + "name": "id_user", + "type": INTEGER(), + "nullable": True, "default": None, + "autoincrement": False, "is_hidden": False, - "name": "parent_user_id", + "comment": None, + }, + { + "name": "data", + "type": VARCHAR(length=30), "nullable": True, - "type": INTEGER(), + "default": None, + "autoincrement": False, + "is_hidden": False, + "comment": None, }, ], } eq_(len(actual), len(expected)) eq_(actual.keys(), expected.keys()) - eq_(len(actual[(None, "comment_test")]), len(expected[(None, "comment_test")])) + eq_( + len(actual[(None, "comment_test")]), + len(expected[(None, "comment_test")]), + ) + if config.db.dialect.supports_comments: + act = [x for x in actual[(None, "comment_test")] if x["name"] == "data"][0] + exp = [x for x in expected[(None, "comment_test")] if x["name"] == "data"][0] + eq_(act["comment"], exp["comment"]) @skip("cockroachdb") def test_get_multi_indexes(self): @@ -244,13 +272,9 @@ def test_get_view_names(self): pass @testing.combinations(True, False, argnames="use_schema") - @testing.combinations( - (True, testing.requires.views), False, argnames="views" - ) + @testing.combinations((True, testing.requires.views), False, argnames="views") def test_metadata(self, connection, use_schema, views): - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_metadata(connection, use_schema, views, []) @skip("cockroachdb") @@ -310,9 +334,7 @@ class LongNameBlowoutTest(_LongNameBlowoutTest): argnames="type_", ) def test_long_convention_name(self, type_, metadata, connection): - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_long_convention_name(type_, metadata, connection, None) @@ -326,9 +348,7 @@ def quote_fixtures(fn): @quote_fixtures def test_get_indexes(self, name): # could not decorrelate subquery - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_get_indexes(name, None) @@ -358,7 +378,5 @@ def test_floordiv_integer_bound(self): class UnicodeSchemaTest(_UnicodeSchemaTest): def test_reflect(self, connection): - if not ( - config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus - ): + if not (config.db.dialect.driver == "asyncpg" and not config.db.dialect._is_v231plus): super().test_reflect(connection)