From 17f4d558c802f12f66aa9af6da9c2bf9c2923614 Mon Sep 17 00:00:00 2001 From: Paul Draper Date: Sun, 31 Jan 2021 20:45:56 -0700 Subject: [PATCH 1/2] Implement comments --- schemainspect/misc.py | 4 +- schemainspect/pg/obj.py | 62 +++++++++++++++++++++++++++++++ schemainspect/pg/sql/comments.sql | 54 +++++++++++++++++++++++++++ tests/test_all.py | 13 +++++++ 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 schemainspect/pg/sql/comments.sql diff --git a/schemainspect/misc.py b/schemainspect/misc.py index 26e16ca..3d6c083 100644 --- a/schemainspect/misc.py +++ b/schemainspect/misc.py @@ -53,10 +53,12 @@ def unquoted_identifier(identifier, *, schema=None, identity_arguments=None): return s -def quoted_identifier(identifier, schema=None, identity_arguments=None): +def quoted_identifier(identifier, schema=None, identity_arguments=None, table=None): if identifier is None and schema is not None: return '"{}"'.format(schema.replace('"', '""')) s = '"{}"'.format(identifier.replace('"', '""')) + if table: + s = '"{}".{}'.format(table.replace('"', '""'), s) if schema: s = '"{}".{}'.format(schema.replace('"', '""'), s) if identity_arguments is not None: diff --git a/schemainspect/pg/obj.py b/schemainspect/pg/obj.py index bc8639a..9ca4165 100644 --- a/schemainspect/pg/obj.py +++ b/schemainspect/pg/obj.py @@ -34,6 +34,7 @@ COLLATIONS_QUERY = resource_text("sql/collations.sql") COLLATIONS_QUERY_9 = resource_text("sql/collations9.sql") RLSPOLICIES_QUERY = resource_text("sql/rlspolicies.sql") +COMMENTS_QUERY = resource_text("sql/comments.sql") class InspectedSelectable(BaseInspectedSelectable): @@ -994,6 +995,49 @@ def key(self): return self.object_type, self.quoted_full_name, self.target_user, self.privilege +class InspectedComment(Inspected): + def __init__(self, object_type, schema, table, name, args, comment): + self.object_type = object_type + self.schema = schema + self.table = table + self.name = name + self.args = args + self.comment = comment + + @property + def _identifier(self): + return quoted_identifier( + self.name, + schema=self.schema, + table=self.table, + identity_arguments=self.args, + ) + + @property + def drop_statement(self): + return "comment on {} {} is null;".format(self.object_type, self._identifier) + + @property + def create_statement(self): + return "comment on {} {} is '{}';".format( + self.object_type, self._identifier, self.comment + ) + + @property + def key(self): + return "{} {}".format(self.object_type, self._identifier) + + def __eq__(self, other): + return ( + self.object_type == other.object_type + and self.schema == other.schema + and self.table == other.table + and self.name == other.name + and self.args == other.args + and self.comment == other.comment + ) + + RLS_POLICY_CREATE = """create policy {name} on {table_name} as {permissiveness} @@ -1134,6 +1178,7 @@ def processed(q): self.SCHEMAS_QUERY = processed(SCHEMAS_QUERY) self.PRIVILEGES_QUERY = processed(PRIVILEGES_QUERY) self.TRIGGERS_QUERY = processed(TRIGGERS_QUERY) + self.COMMENTS_QUERY = processed(COMMENTS_QUERY) super(PostgreSQL, self).__init__(c, include_internal) @@ -1160,6 +1205,7 @@ def load_all(self): self.load_rlspolicies() self.load_types() self.load_domains() + self.load_comments() self.load_deps() self.load_deps_all() @@ -1663,6 +1709,21 @@ def col(defn): ] # type: list[InspectedType] self.domains = od((t.signature, t) for t in domains) + def load_comments(self): + q = self.c.execute(self.COMMENTS_QUERY) + comments = [ + InspectedComment( + i.object_type, + i.schema, + i.table, + i.name, + i.args, + i.comment, + ) + for i in q + ] # type: list[InspectedComment] + self.comments = od((t.key, t) for t in comments) + def filter_schema(self, schema=None, exclude_schema=None): if schema and exclude_schema: raise ValueError("Can only have schema or exclude schema, not both") @@ -1765,4 +1826,5 @@ def __eq__(self, other): and self.triggers == other.triggers and self.collations == other.collations and self.rlspolicies == other.rlspolicies + and self.comments == other.comments ) diff --git a/schemainspect/pg/sql/comments.sql b/schemainspect/pg/sql/comments.sql new file mode 100644 index 0000000..c622c02 --- /dev/null +++ b/schemainspect/pg/sql/comments.sql @@ -0,0 +1,54 @@ +select + 'function' object_type, + n.nspname "schema", + NULL "table", + p.proname "name", + pg_catalog.pg_get_function_identity_arguments(p.oid) args, + pg_catalog.obj_description(p.oid, 'pg_proc') "comment" +from + pg_catalog.pg_proc p + join pg_catalog.pg_namespace n on n.oid = p.pronamespace +where + n.nspname <> 'pg_catalog' + and n.nspname <> 'information_schema' + and pg_catalog.obj_description(p.oid, 'pg_proc') is not null +union all +select + case c.relkind + when 'I' then 'index' + when 'c' then 'type' + when 'i' then 'index' + when 'm' then 'materialized view' + when 'p' then 'table' + when 'r' then 'table' + when 's' then 'sequence' + when 'v' then 'view' + end, + n.nspname, + NULL, + c.relname, + NULL, + pg_catalog.obj_description(c.oid, 'pg_class') +from + pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace +where + n.nspname <> 'pg_catalog' + and n.nspname <> 'information_schema' + and pg_catalog.obj_description(c.oid, 'pg_class') is not null +union all +select + 'column', + n.nspname, + c.relname, + a.attname, + NULL, + pg_catalog.col_description(c.oid, a.attnum) +from + pg_catalog.pg_attribute a + join pg_catalog.pg_class c on c.oid = a.attrelid + join pg_catalog.pg_namespace n on n.oid = c.relnamespace +where + n.nspname <> 'pg_catalog' + and n.nspname <> 'information_schema' + and pg_catalog.col_description(c.oid, a.attnum) is not null; diff --git a/tests/test_all.py b/tests/test_all.py index 89c66f8..1a4dd9c 100644 --- a/tests/test_all.py +++ b/tests/test_all.py @@ -495,6 +495,19 @@ def asserts_pg(i, has_timescale=False): with raises(ValueError): tid.change_string_to_enum_statement("t") + # comments + assert len(i.comments) == 2 + assert ( + i.comments[ + 'function "public"."films_f"(d date, def_t text, def_d date)' + ].create_statement + == 'comment on function "public"."films_f"(d date, def_t text, def_d date) is \'films_f comment\';' + ) + assert ( + i.comments['table "public"."emptytable"'].create_statement + == 'comment on table "public"."emptytable" is \'emptytable comment\';' + ) + def test_weird_names(db): with S(db) as s: From 3259505c145e3ef83ba558f9c5a2b40ae7633fee Mon Sep 17 00:00:00 2001 From: Adam Thomas Date: Mon, 16 Jan 2023 10:25:12 +1100 Subject: [PATCH 2/2] Extend comment support This patch adds support for almost all of the comments in postgres. Comments on domain constraints are currently unspported due to a bug in postgres' pg_identify_object. --- schemainspect/pg/obj.py | 36 ++++----------- schemainspect/pg/sql/comments.sql | 77 +++++++++---------------------- tests/test_all.py | 12 +++-- 3 files changed, 39 insertions(+), 86 deletions(-) diff --git a/schemainspect/pg/obj.py b/schemainspect/pg/obj.py index 9ca4165..8bc8d46 100644 --- a/schemainspect/pg/obj.py +++ b/schemainspect/pg/obj.py @@ -996,44 +996,29 @@ def key(self): class InspectedComment(Inspected): - def __init__(self, object_type, schema, table, name, args, comment): + def __init__(self, object_type, identifier, comment): + self.identifier = identifier self.object_type = object_type - self.schema = schema - self.table = table - self.name = name - self.args = args self.comment = comment - @property - def _identifier(self): - return quoted_identifier( - self.name, - schema=self.schema, - table=self.table, - identity_arguments=self.args, - ) - @property def drop_statement(self): - return "comment on {} {} is null;".format(self.object_type, self._identifier) + return "comment on {} {} is null;".format(self.object_type, self.identifier) @property def create_statement(self): - return "comment on {} {} is '{}';".format( - self.object_type, self._identifier, self.comment + return "comment on {} {} is $cmt${}$cmt$;".format( + self.object_type, self.identifier, self.comment ) @property def key(self): - return "{} {}".format(self.object_type, self._identifier) + return "{} {}".format(self.object_type, self.identifier) def __eq__(self, other): return ( self.object_type == other.object_type - and self.schema == other.schema - and self.table == other.table - and self.name == other.name - and self.args == other.args + and self.identifier == other.identifier and self.comment == other.comment ) @@ -1710,14 +1695,11 @@ def col(defn): self.domains = od((t.signature, t) for t in domains) def load_comments(self): - q = self.c.execute(self.COMMENTS_QUERY) + q = self.execute(self.COMMENTS_QUERY) comments = [ InspectedComment( i.object_type, - i.schema, - i.table, - i.name, - i.args, + i.identifier, i.comment, ) for i in q diff --git a/schemainspect/pg/sql/comments.sql b/schemainspect/pg/sql/comments.sql index c622c02..d0ec9ad 100644 --- a/schemainspect/pg/sql/comments.sql +++ b/schemainspect/pg/sql/comments.sql @@ -1,54 +1,23 @@ -select - 'function' object_type, - n.nspname "schema", - NULL "table", - p.proname "name", - pg_catalog.pg_get_function_identity_arguments(p.oid) args, - pg_catalog.obj_description(p.oid, 'pg_proc') "comment" -from - pg_catalog.pg_proc p - join pg_catalog.pg_namespace n on n.oid = p.pronamespace -where - n.nspname <> 'pg_catalog' - and n.nspname <> 'information_schema' - and pg_catalog.obj_description(p.oid, 'pg_proc') is not null -union all -select - case c.relkind - when 'I' then 'index' - when 'c' then 'type' - when 'i' then 'index' - when 'm' then 'materialized view' - when 'p' then 'table' - when 'r' then 'table' - when 's' then 'sequence' - when 'v' then 'view' - end, - n.nspname, - NULL, - c.relname, - NULL, - pg_catalog.obj_description(c.oid, 'pg_class') -from - pg_catalog.pg_class c - join pg_catalog.pg_namespace n on n.oid = c.relnamespace -where - n.nspname <> 'pg_catalog' - and n.nspname <> 'information_schema' - and pg_catalog.obj_description(c.oid, 'pg_class') is not null -union all -select - 'column', - n.nspname, - c.relname, - a.attname, - NULL, - pg_catalog.col_description(c.oid, a.attnum) -from - pg_catalog.pg_attribute a - join pg_catalog.pg_class c on c.oid = a.attrelid - join pg_catalog.pg_namespace n on n.oid = c.relnamespace -where - n.nspname <> 'pg_catalog' - and n.nspname <> 'information_schema' - and pg_catalog.col_description(c.oid, a.attnum) is not null; +SELECT + CASE (d.iden).type + WHEN 'domain constraint' THEN 'constraint' + WHEN 'table column' THEN 'column' + WHEN 'table constraint' THEN 'constraint' + ELSE (d.iden).type::TEXT + END AS object_type, + (d.iden).identity AS identifier, + d.description AS comment +FROM ( + SELECT + pg_identify_object(classoid, objoid, objsubid) AS iden, + DESCRIPTION + FROM pg_description +) d +WHERE + ( + (d.iden).schema IS NULL + AND (d.iden).type = 'trigger' + ) OR ( + (d.iden).schema <> 'pg_catalog' + AND (d.iden).schema <> 'information_schema' + ); diff --git a/tests/test_all.py b/tests/test_all.py index 1a4dd9c..6f27575 100644 --- a/tests/test_all.py +++ b/tests/test_all.py @@ -279,7 +279,9 @@ def setup_pg_schema(s): language sql; """ ) - s.execute("comment on function films_f(date, text, date) is 'films_f comment'") + s.execute( + "comment on function public.films_f(date, text, date) is 'films_f comment'" + ) s.execute( """ CREATE OR REPLACE FUNCTION inc_f(integer) RETURNS integer AS $$ @@ -499,13 +501,13 @@ def asserts_pg(i, has_timescale=False): assert len(i.comments) == 2 assert ( i.comments[ - 'function "public"."films_f"(d date, def_t text, def_d date)' + "function public.films_f(pg_catalog.date,pg_catalog.text,pg_catalog.date)" ].create_statement - == 'comment on function "public"."films_f"(d date, def_t text, def_d date) is \'films_f comment\';' + == "comment on function public.films_f(pg_catalog.date,pg_catalog.text,pg_catalog.date) is $cmt$films_f comment$cmt$;" ) assert ( - i.comments['table "public"."emptytable"'].create_statement - == 'comment on table "public"."emptytable" is \'emptytable comment\';' + i.comments["table public.emptytable"].create_statement + == "comment on table public.emptytable is $cmt$emptytable comment$cmt$;" )