From 13f96d5357a67cb18a66033584d8e0f25b00b013 Mon Sep 17 00:00:00 2001 From: dilame Date: Wed, 29 Apr 2026 00:51:28 +0800 Subject: [PATCH] feat: add COMMENT ON ... support to plan/apply Plumb pg_description through the introspection path and emit `COMMENT ON IS '...'` (or `IS NULL` to clear) whenever the description on a schema object is added, changed, or removed. Covered objects: schema, extension, table, column, enum/type, sequence, index, primary/unique constraint (via index), check constraint, foreign key, function, procedure, trigger, view, materialized view, policy. Acceptance coverage: internal/migration_acceptance_tests/comment_cases_test.go. --- .../comment_cases_test.go | 471 ++++++++++++++++++ internal/queries/dml.sql.go | 2 +- internal/queries/models.sql.go | 4 +- internal/queries/queries.sql | 74 ++- internal/queries/queries.sql.go | 125 ++++- internal/schema/schema.go | 79 ++- internal/schema/schema_test.go | 34 +- pkg/diff/comment_sql_generator.go | 135 +++++ pkg/diff/enum_sql_generator.go | 14 +- pkg/diff/function_sql_vertex_generator.go | 26 +- pkg/diff/materialized_view_sql_generator.go | 37 +- pkg/diff/named_schema_sql_generator.go | 10 +- pkg/diff/policy_sql_generator.go | 25 +- pkg/diff/procedure_sql_vertex_generator.go | 65 ++- pkg/diff/sql_generator.go | 93 +++- pkg/diff/trigger_sql_vertex_generator.go | 17 +- pkg/diff/view_sql_generator.go | 37 +- pkg/tempdb/factory_test.go | 3 +- 18 files changed, 1114 insertions(+), 137 deletions(-) create mode 100644 internal/migration_acceptance_tests/comment_cases_test.go create mode 100644 pkg/diff/comment_sql_generator.go diff --git a/internal/migration_acceptance_tests/comment_cases_test.go b/internal/migration_acceptance_tests/comment_cases_test.go new file mode 100644 index 0000000..4e885b7 --- /dev/null +++ b/internal/migration_acceptance_tests/comment_cases_test.go @@ -0,0 +1,471 @@ +package migration_acceptance_tests + +import "testing" + +// commentAcceptanceTestCases exercises COMMENT ON ... migrations across every +// object kind PostgreSQL allows comments on. Each case verifies one of three +// transitions per object: add a comment, change a comment, or remove a comment. +var commentAcceptanceTestCases = []acceptanceTestCase{ + // ─── Schema ─── + { + name: "schema: add comment", + oldSchemaDDL: []string{` + CREATE SCHEMA app; + `}, + newSchemaDDL: []string{` + CREATE SCHEMA app; + COMMENT ON SCHEMA app IS 'application schema'; + `}, + }, + { + name: "schema: change comment", + oldSchemaDDL: []string{` + CREATE SCHEMA app; + COMMENT ON SCHEMA app IS 'application schema'; + `}, + newSchemaDDL: []string{` + CREATE SCHEMA app; + COMMENT ON SCHEMA app IS 'app schema (renamed)'; + `}, + }, + { + name: "schema: remove comment", + oldSchemaDDL: []string{` + CREATE SCHEMA app; + COMMENT ON SCHEMA app IS 'application schema'; + `}, + newSchemaDDL: []string{` + CREATE SCHEMA app; + `}, + }, + { + name: "schema: create with comment", + oldSchemaDDL: []string{}, + newSchemaDDL: []string{` + CREATE SCHEMA app; + COMMENT ON SCHEMA app IS 'application schema'; + `}, + }, + + // ─── Table + Column ─── + { + name: "table: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON TABLE foo IS '@interface mode:union'; + `}, + }, + { + name: "table: change comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON TABLE foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON TABLE foo IS 'new'; + `}, + }, + { + name: "table: remove comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON TABLE foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + `}, + }, + { + name: "table: create with comment + column comment", + oldSchemaDDL: []string{}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT, name TEXT); + COMMENT ON TABLE foo IS '@behavior +select'; + COMMENT ON COLUMN foo.id IS 'primary id'; + COMMENT ON COLUMN foo.name IS 'display name'; + `}, + }, + { + name: "column: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON COLUMN foo.id IS 'primary id'; + `}, + }, + { + name: "column: change comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON COLUMN foo.id IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON COLUMN foo.id IS 'new'; + `}, + }, + { + name: "column: remove comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON COLUMN foo.id IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + `}, + }, + { + name: "column: comment with single-quote literal", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + COMMENT ON COLUMN foo.id IS 'this isn''t escaped easily'; + `}, + }, + + // ─── Enum (type) ─── + { + name: "enum: add comment", + oldSchemaDDL: []string{` + CREATE TYPE color AS ENUM ('red', 'green'); + `}, + newSchemaDDL: []string{` + CREATE TYPE color AS ENUM ('red', 'green'); + COMMENT ON TYPE color IS 'color codes'; + `}, + }, + { + name: "enum: change comment", + oldSchemaDDL: []string{` + CREATE TYPE color AS ENUM ('red', 'green'); + COMMENT ON TYPE color IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TYPE color AS ENUM ('red', 'green'); + COMMENT ON TYPE color IS 'new'; + `}, + }, + { + name: "enum: create with comment", + oldSchemaDDL: []string{}, + newSchemaDDL: []string{` + CREATE TYPE color AS ENUM ('red', 'green'); + COMMENT ON TYPE color IS 'color codes'; + `}, + }, + + // ─── Function ─── + { + name: "function: add comment", + oldSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + `}, + newSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + COMMENT ON FUNCTION add(int, int) IS '@name addition'; + `}, + }, + { + name: "function: change comment only (no body change)", + oldSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + COMMENT ON FUNCTION add(int, int) IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + COMMENT ON FUNCTION add(int, int) IS 'new'; + `}, + }, + { + name: "function: remove comment", + oldSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + COMMENT ON FUNCTION add(int, int) IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + `}, + }, + { + name: "function: create with comment", + oldSchemaDDL: []string{}, + newSchemaDDL: []string{` + CREATE FUNCTION add(a int, b int) RETURNS int LANGUAGE sql AS 'SELECT a + b'; + COMMENT ON FUNCTION add(int, int) IS '@name addition'; + `}, + }, + + // ─── Procedure ─── + { + name: "procedure: add comment", + oldSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + `}, + newSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + COMMENT ON PROCEDURE noop() IS 'a procedure'; + `}, + }, + { + name: "procedure: change comment only", + oldSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + COMMENT ON PROCEDURE noop() IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + COMMENT ON PROCEDURE noop() IS 'new'; + `}, + }, + { + name: "procedure: remove comment", + oldSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + COMMENT ON PROCEDURE noop() IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE PROCEDURE noop() LANGUAGE plpgsql AS $$ BEGIN END $$; + `}, + }, + + // ─── Trigger ─── + { + name: "trigger: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + COMMENT ON TRIGGER trg ON foo IS 'audit trigger'; + `}, + }, + { + name: "trigger: change comment only", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + COMMENT ON TRIGGER trg ON foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + COMMENT ON TRIGGER trg ON foo IS 'new'; + `}, + }, + { + name: "trigger: remove comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + COMMENT ON TRIGGER trg ON foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE FUNCTION trg_fn() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; + CREATE TRIGGER trg BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION trg_fn(); + `}, + }, + + // ─── View ─── + { + name: "view: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + COMMENT ON VIEW v IS '@behavior +select'; + `}, + }, + { + name: "view: change comment only", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + COMMENT ON VIEW v IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + COMMENT ON VIEW v IS 'new'; + `}, + }, + { + name: "view: remove comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + COMMENT ON VIEW v IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE VIEW v AS SELECT id FROM foo; + `}, + }, + + // ─── Materialized view ─── + { + name: "materialized view: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE MATERIALIZED VIEW mv AS SELECT id FROM foo; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE MATERIALIZED VIEW mv AS SELECT id FROM foo; + COMMENT ON MATERIALIZED VIEW mv IS 'mv comment'; + `}, + }, + { + name: "materialized view: change comment only", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE MATERIALIZED VIEW mv AS SELECT id FROM foo; + COMMENT ON MATERIALIZED VIEW mv IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + CREATE MATERIALIZED VIEW mv AS SELECT id FROM foo; + COMMENT ON MATERIALIZED VIEW mv IS 'new'; + `}, + }, + + // ─── Sequence ─── + { + name: "sequence: add comment", + oldSchemaDDL: []string{` + CREATE SEQUENCE s; + `}, + newSchemaDDL: []string{` + CREATE SEQUENCE s; + COMMENT ON SEQUENCE s IS 'sequence'; + `}, + }, + { + name: "sequence: change comment", + oldSchemaDDL: []string{` + CREATE SEQUENCE s; + COMMENT ON SEQUENCE s IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE SEQUENCE s; + COMMENT ON SEQUENCE s IS 'new'; + `}, + }, + + // ─── Index ─── + { + name: "index: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT, name TEXT); + CREATE INDEX foo_name_idx ON foo (name); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT, name TEXT); + CREATE INDEX foo_name_idx ON foo (name); + COMMENT ON INDEX foo_name_idx IS 'lookup by name'; + `}, + }, + { + name: "index: change comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT, name TEXT); + CREATE INDEX foo_name_idx ON foo (name); + COMMENT ON INDEX foo_name_idx IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT, name TEXT); + CREATE INDEX foo_name_idx ON foo (name); + COMMENT ON INDEX foo_name_idx IS 'new'; + `}, + }, + + // ─── Constraint (CHECK) ─── + { + name: "check constraint: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT, CONSTRAINT foo_id_pos CHECK (id > 0)); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT, CONSTRAINT foo_id_pos CHECK (id > 0)); + COMMENT ON CONSTRAINT foo_id_pos ON foo IS 'positive'; + `}, + }, + { + name: "check constraint: change comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT, CONSTRAINT foo_id_pos CHECK (id > 0)); + COMMENT ON CONSTRAINT foo_id_pos ON foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT, CONSTRAINT foo_id_pos CHECK (id > 0)); + COMMENT ON CONSTRAINT foo_id_pos ON foo IS 'new'; + `}, + }, + + // ─── Constraint (FOREIGN KEY) ─── + { + name: "fk constraint: add comment", + oldSchemaDDL: []string{` + CREATE TABLE parent (id INT PRIMARY KEY); + CREATE TABLE child (id INT, parent_id INT, CONSTRAINT child_parent_fk FOREIGN KEY (parent_id) REFERENCES parent(id)); + `}, + newSchemaDDL: []string{` + CREATE TABLE parent (id INT PRIMARY KEY); + CREATE TABLE child (id INT, parent_id INT, CONSTRAINT child_parent_fk FOREIGN KEY (parent_id) REFERENCES parent(id)); + COMMENT ON CONSTRAINT child_parent_fk ON child IS 'fk to parent'; + `}, + }, + + // ─── Policy ─── + { + name: "policy: add comment", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + ALTER TABLE foo ENABLE ROW LEVEL SECURITY; + CREATE POLICY p ON foo FOR SELECT USING (true); + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + ALTER TABLE foo ENABLE ROW LEVEL SECURITY; + CREATE POLICY p ON foo FOR SELECT USING (true); + COMMENT ON POLICY p ON foo IS 'allow all reads'; + `}, + }, + { + name: "policy: change comment only", + oldSchemaDDL: []string{` + CREATE TABLE foo (id INT); + ALTER TABLE foo ENABLE ROW LEVEL SECURITY; + CREATE POLICY p ON foo FOR SELECT USING (true); + COMMENT ON POLICY p ON foo IS 'old'; + `}, + newSchemaDDL: []string{` + CREATE TABLE foo (id INT); + ALTER TABLE foo ENABLE ROW LEVEL SECURITY; + CREATE POLICY p ON foo FOR SELECT USING (true); + COMMENT ON POLICY p ON foo IS 'new'; + `}, + }, +} + +func TestCommentTestCases(t *testing.T) { + runTestCases(t, commentAcceptanceTestCases) +} diff --git a/internal/queries/dml.sql.go b/internal/queries/dml.sql.go index 7c7e70e..579c823 100644 --- a/internal/queries/dml.sql.go +++ b/internal/queries/dml.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.18.0 +// sqlc v1.31.1 package queries diff --git a/internal/queries/models.sql.go b/internal/queries/models.sql.go index 5da8a9e..d6f60be 100644 --- a/internal/queries/models.sql.go +++ b/internal/queries/models.sql.go @@ -1,7 +1,5 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.18.0 +// sqlc v1.31.1 package queries - -import () diff --git a/internal/queries/queries.sql b/internal/queries/queries.sql index d8397c8..5367ff7 100644 --- a/internal/queries/queries.sql +++ b/internal/queries/queries.sql @@ -1,5 +1,9 @@ -- name: GetSchemas :many -SELECT nspname::TEXT AS schema_name +SELECT + nspname::TEXT AS schema_name, + COALESCE( + pg_catalog.obj_description(pg_namespace.oid, 'pg_namespace'), '' + )::TEXT AS description FROM pg_catalog.pg_namespace WHERE nspname NOT IN ('pg_catalog', 'information_schema') @@ -33,7 +37,10 @@ SELECT (CASE WHEN c.relispartition THEN pg_catalog.pg_get_expr(c.relpartbound, c.oid) ELSE '' - END)::TEXT AS partition_for_values + END)::TEXT AS partition_for_values, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS table_namespace @@ -115,7 +122,10 @@ SELECT END, '' )::TEXT AS generation_expression, (a.attgenerated = 's') AS is_generated, - pg_catalog.format_type(a.atttypid, a.atttypmod) AS column_type + pg_catalog.format_type(a.atttypid, a.atttypmod) AS column_type, + COALESCE( + pg_catalog.col_description(a.attrelid, a.attnum), '' + )::TEXT AS description FROM pg_catalog.pg_attribute AS a LEFT JOIN pg_catalog.pg_attrdef AS d @@ -164,7 +174,13 @@ SELECT pg_catalog.pg_attribute AS att ON att.attrelid = table_c.oid AND indkey_ord.attnum = att.attnum )::TEXT [] AS column_names, - COALESCE(con.conislocal, false) AS constraint_is_local + COALESCE(con.conislocal, false) AS constraint_is_local, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description, + COALESCE( + pg_catalog.obj_description(con.oid, 'pg_constraint'), '' + )::TEXT AS constraint_description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_index AS i ON (c.oid = i.indexrelid) INNER JOIN pg_catalog.pg_class AS table_c ON (i.indrelid = table_c.oid) @@ -216,7 +232,10 @@ SELECT pg_constraint.connoinherit AS is_not_inheritable, pg_catalog.pg_get_expr( pg_constraint.conbin, pg_constraint.conrelid - ) AS constraint_expression + ) AS constraint_expression, + COALESCE( + pg_catalog.obj_description(pg_constraint.oid, 'pg_constraint'), '' + )::TEXT AS description FROM pg_catalog.pg_constraint INNER JOIN pg_catalog.pg_class ON pg_constraint.conrelid = pg_class.oid INNER JOIN @@ -237,7 +256,10 @@ SELECT foreign_table_c.relname::TEXT AS foreign_table_name, foreign_table_namespace.nspname::TEXT AS foreign_table_schema_name, pg_constraint.convalidated AS is_valid, - pg_catalog.pg_get_constraintdef(pg_constraint.oid) AS constraint_def + pg_catalog.pg_get_constraintdef(pg_constraint.oid) AS constraint_def, + COALESCE( + pg_catalog.obj_description(pg_constraint.oid, 'pg_constraint'), '' + )::TEXT AS description FROM pg_catalog.pg_constraint INNER JOIN pg_catalog.pg_class AS constraint_c @@ -266,7 +288,10 @@ SELECT pg_catalog.pg_get_function_identity_arguments( pg_proc.oid ) AS func_identity_arguments, - pg_catalog.pg_get_functiondef(pg_proc.oid) AS func_def + pg_catalog.pg_get_functiondef(pg_proc.oid) AS func_def, + COALESCE( + pg_catalog.obj_description(pg_proc.oid, 'pg_proc'), '' + )::TEXT AS description FROM pg_catalog.pg_proc INNER JOIN pg_catalog.pg_namespace AS proc_namespace @@ -320,7 +345,10 @@ SELECT pg_proc.oid ) AS func_identity_arguments, pg_catalog.pg_get_triggerdef(trig.oid) AS trigger_def, - trig.tgconstraint != 0 AS is_constraint + trig.tgconstraint != 0 AS is_constraint, + COALESCE( + pg_catalog.obj_description(trig.oid, 'pg_trigger'), '' + )::TEXT AS description FROM pg_catalog.pg_trigger AS trig INNER JOIN pg_catalog.pg_class AS owning_c ON trig.tgrelid = owning_c.oid INNER JOIN @@ -350,7 +378,10 @@ SELECT pg_seq.seqmin AS min_value, pg_seq.seqcache AS cache_size, pg_seq.seqcycle AS is_cycle, - FORMAT_TYPE(pg_seq.seqtypid, null) AS data_type + FORMAT_TYPE(pg_seq.seqtypid, null) AS data_type, + COALESCE( + pg_catalog.obj_description(pg_seq.seqrelid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_sequence AS pg_seq INNER JOIN pg_catalog.pg_class AS seq_c ON pg_seq.seqrelid = seq_c.oid INNER JOIN pg_catalog.pg_namespace AS seq_ns ON seq_c.relnamespace = seq_ns.oid @@ -390,7 +421,10 @@ SELECT ext.oid, ext.extname::TEXT AS extension_name, ext.extversion AS extension_version, - extension_namespace.nspname::TEXT AS schema_name + extension_namespace.nspname::TEXT AS schema_name, + COALESCE( + pg_catalog.obj_description(ext.oid, 'pg_extension'), '' + )::TEXT AS description FROM pg_catalog.pg_namespace AS extension_namespace INNER JOIN pg_catalog.pg_extension AS ext @@ -411,7 +445,10 @@ SELECT ORDER BY pg_enum.enumsortorder ) FROM pg_catalog.pg_enum - WHERE pg_enum.enumtypid = pg_type.oid)::TEXT [] AS enum_labels + WHERE pg_enum.enumtypid = pg_type.oid)::TEXT [] AS enum_labels, + COALESCE( + pg_catalog.obj_description(pg_type.oid, 'pg_type'), '' + )::TEXT AS description FROM pg_catalog.pg_type AS pg_type INNER JOIN pg_catalog.pg_namespace AS type_namespace @@ -473,7 +510,10 @@ SELECT AND d.refclassid = 'pg_class'::REGCLASS AND a.attrelid = table_c.oid AND NOT a.attisdropped - )::TEXT [] AS column_names + )::TEXT [] AS column_names, + COALESCE( + pg_catalog.obj_description(pol.oid, 'pg_policy'), '' + )::TEXT AS description FROM pg_catalog.pg_policy AS pol INNER JOIN pg_catalog.pg_class AS table_c ON pol.polrelid = table_c.oid INNER JOIN @@ -536,7 +576,10 @@ SELECT -- Instead, they must be unmarshalled as string arrays. -- https://github.com/lib/pq/pull/466 WHERE d.refobjid = c.oid)::TEXT [] AS table_dependencies, - PG_GET_VIEWDEF(c.oid, true) AS view_definition + PG_GET_VIEWDEF(c.oid, true) AS view_definition, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid WHERE @@ -607,7 +650,10 @@ SELECT -- Instead, they must be unmarshalled as string arrays. -- https://github.com/lib/pq/pull/466 WHERE d.refobjid = c.oid)::TEXT [] AS table_dependencies, - PG_GET_VIEWDEF(c.oid, true) AS view_definition + PG_GET_VIEWDEF(c.oid, true) AS view_definition, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid LEFT JOIN pg_catalog.pg_tablespace AS ts ON c.reltablespace = ts.oid diff --git a/internal/queries/queries.sql.go b/internal/queries/queries.sql.go index 4315102..ceb8b9c 100644 --- a/internal/queries/queries.sql.go +++ b/internal/queries/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.18.0 +// sqlc v1.31.1 // source: queries.sql package queries @@ -31,7 +31,10 @@ SELECT pg_constraint.connoinherit AS is_not_inheritable, pg_catalog.pg_get_expr( pg_constraint.conbin, pg_constraint.conrelid - ) AS constraint_expression + ) AS constraint_expression, + COALESCE( + pg_catalog.obj_description(pg_constraint.oid, 'pg_constraint'), '' + )::TEXT AS description FROM pg_catalog.pg_constraint INNER JOIN pg_catalog.pg_class ON pg_constraint.conrelid = pg_class.oid INNER JOIN @@ -54,6 +57,7 @@ type GetCheckConstraintsRow struct { IsValid bool IsNotInheritable bool ConstraintExpression string + Description string } func (q *Queries) GetCheckConstraints(ctx context.Context) ([]GetCheckConstraintsRow, error) { @@ -74,6 +78,7 @@ func (q *Queries) GetCheckConstraints(ctx context.Context) ([]GetCheckConstraint &i.IsValid, &i.IsNotInheritable, &i.ConstraintExpression, + &i.Description, ); err != nil { return nil, err } @@ -141,7 +146,10 @@ SELECT END, '' )::TEXT AS generation_expression, (a.attgenerated = 's') AS is_generated, - pg_catalog.format_type(a.atttypid, a.atttypmod) AS column_type + pg_catalog.format_type(a.atttypid, a.atttypmod) AS column_type, + COALESCE( + pg_catalog.col_description(a.attrelid, a.attnum), '' + )::TEXT AS description FROM pg_catalog.pg_attribute AS a LEFT JOIN pg_catalog.pg_attrdef AS d @@ -180,6 +188,7 @@ type GetColumnsForTableRow struct { GenerationExpression string IsGenerated bool ColumnType string + Description string } func (q *Queries) GetColumnsForTable(ctx context.Context, attrelid interface{}) ([]GetColumnsForTableRow, error) { @@ -209,6 +218,7 @@ func (q *Queries) GetColumnsForTable(ctx context.Context, attrelid interface{}) &i.GenerationExpression, &i.IsGenerated, &i.ColumnType, + &i.Description, ); err != nil { return nil, err } @@ -288,7 +298,10 @@ SELECT ORDER BY pg_enum.enumsortorder ) FROM pg_catalog.pg_enum - WHERE pg_enum.enumtypid = pg_type.oid)::TEXT [] AS enum_labels + WHERE pg_enum.enumtypid = pg_type.oid)::TEXT [] AS enum_labels, + COALESCE( + pg_catalog.obj_description(pg_type.oid, 'pg_type'), '' + )::TEXT AS description FROM pg_catalog.pg_type AS pg_type INNER JOIN pg_catalog.pg_namespace AS type_namespace @@ -313,6 +326,7 @@ type GetEnumsRow struct { EnumName string EnumSchemaName string EnumLabels []string + Description string } func (q *Queries) GetEnums(ctx context.Context) ([]GetEnumsRow, error) { @@ -324,7 +338,12 @@ func (q *Queries) GetEnums(ctx context.Context) ([]GetEnumsRow, error) { var items []GetEnumsRow for rows.Next() { var i GetEnumsRow - if err := rows.Scan(&i.EnumName, &i.EnumSchemaName, pq.Array(&i.EnumLabels)); err != nil { + if err := rows.Scan( + &i.EnumName, + &i.EnumSchemaName, + pq.Array(&i.EnumLabels), + &i.Description, + ); err != nil { return nil, err } items = append(items, i) @@ -343,7 +362,10 @@ SELECT ext.oid, ext.extname::TEXT AS extension_name, ext.extversion AS extension_version, - extension_namespace.nspname::TEXT AS schema_name + extension_namespace.nspname::TEXT AS schema_name, + COALESCE( + pg_catalog.obj_description(ext.oid, 'pg_extension'), '' + )::TEXT AS description FROM pg_catalog.pg_namespace AS extension_namespace INNER JOIN pg_catalog.pg_extension AS ext @@ -359,6 +381,7 @@ type GetExtensionsRow struct { ExtensionName string ExtensionVersion string SchemaName string + Description string } func (q *Queries) GetExtensions(ctx context.Context) ([]GetExtensionsRow, error) { @@ -375,6 +398,7 @@ func (q *Queries) GetExtensions(ctx context.Context) ([]GetExtensionsRow, error) &i.ExtensionName, &i.ExtensionVersion, &i.SchemaName, + &i.Description, ); err != nil { return nil, err } @@ -397,7 +421,10 @@ SELECT foreign_table_c.relname::TEXT AS foreign_table_name, foreign_table_namespace.nspname::TEXT AS foreign_table_schema_name, pg_constraint.convalidated AS is_valid, - pg_catalog.pg_get_constraintdef(pg_constraint.oid) AS constraint_def + pg_catalog.pg_get_constraintdef(pg_constraint.oid) AS constraint_def, + COALESCE( + pg_catalog.obj_description(pg_constraint.oid, 'pg_constraint'), '' + )::TEXT AS description FROM pg_catalog.pg_constraint INNER JOIN pg_catalog.pg_class AS constraint_c @@ -426,6 +453,7 @@ type GetForeignKeyConstraintsRow struct { ForeignTableSchemaName string IsValid bool ConstraintDef string + Description string } func (q *Queries) GetForeignKeyConstraints(ctx context.Context) ([]GetForeignKeyConstraintsRow, error) { @@ -445,6 +473,7 @@ func (q *Queries) GetForeignKeyConstraints(ctx context.Context) ([]GetForeignKey &i.ForeignTableSchemaName, &i.IsValid, &i.ConstraintDef, + &i.Description, ); err != nil { return nil, err } @@ -488,7 +517,13 @@ SELECT pg_catalog.pg_attribute AS att ON att.attrelid = table_c.oid AND indkey_ord.attnum = att.attnum )::TEXT [] AS column_names, - COALESCE(con.conislocal, false) AS constraint_is_local + COALESCE(con.conislocal, false) AS constraint_is_local, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description, + COALESCE( + pg_catalog.obj_description(con.oid, 'pg_constraint'), '' + )::TEXT AS constraint_description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_index AS i ON (c.oid = i.indexrelid) INNER JOIN pg_catalog.pg_class AS table_c ON (i.indrelid = table_c.oid) @@ -539,6 +574,8 @@ type GetIndexesRow struct { ParentIndexSchemaName string ColumnNames []string ConstraintIsLocal bool + Description string + ConstraintDescription string } func (q *Queries) GetIndexes(ctx context.Context) ([]GetIndexesRow, error) { @@ -567,6 +604,8 @@ func (q *Queries) GetIndexes(ctx context.Context) ([]GetIndexesRow, error) { &i.ParentIndexSchemaName, pq.Array(&i.ColumnNames), &i.ConstraintIsLocal, + &i.Description, + &i.ConstraintDescription, ); err != nil { return nil, err } @@ -635,7 +674,10 @@ SELECT -- Instead, they must be unmarshalled as string arrays. -- https://github.com/lib/pq/pull/466 WHERE d.refobjid = c.oid)::TEXT [] AS table_dependencies, - PG_GET_VIEWDEF(c.oid, true) AS view_definition + PG_GET_VIEWDEF(c.oid, true) AS view_definition, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid LEFT JOIN pg_catalog.pg_tablespace AS ts ON c.reltablespace = ts.oid @@ -661,6 +703,7 @@ type GetMaterializedViewsRow struct { TablespaceName string TableDependencies []string ViewDefinition string + Description string } func (q *Queries) GetMaterializedViews(ctx context.Context) ([]GetMaterializedViewsRow, error) { @@ -679,6 +722,7 @@ func (q *Queries) GetMaterializedViews(ctx context.Context) ([]GetMaterializedVi &i.TablespaceName, pq.Array(&i.TableDependencies), &i.ViewDefinition, + &i.Description, ); err != nil { return nil, err } @@ -734,7 +778,10 @@ SELECT AND d.refclassid = 'pg_class'::REGCLASS AND a.attrelid = table_c.oid AND NOT a.attisdropped - )::TEXT [] AS column_names + )::TEXT [] AS column_names, + COALESCE( + pg_catalog.obj_description(pol.oid, 'pg_policy'), '' + )::TEXT AS description FROM pg_catalog.pg_policy AS pol INNER JOIN pg_catalog.pg_class AS table_c ON pol.polrelid = table_c.oid INNER JOIN @@ -756,6 +803,7 @@ type GetPoliciesRow struct { CheckExpression string UsingExpression string ColumnNames []string + Description string } func (q *Queries) GetPolicies(ctx context.Context) ([]GetPoliciesRow, error) { @@ -777,6 +825,7 @@ func (q *Queries) GetPolicies(ctx context.Context) ([]GetPoliciesRow, error) { &i.CheckExpression, &i.UsingExpression, pq.Array(&i.ColumnNames), + &i.Description, ); err != nil { return nil, err } @@ -800,7 +849,10 @@ SELECT pg_catalog.pg_get_function_identity_arguments( pg_proc.oid ) AS func_identity_arguments, - pg_catalog.pg_get_functiondef(pg_proc.oid) AS func_def + pg_catalog.pg_get_functiondef(pg_proc.oid) AS func_def, + COALESCE( + pg_catalog.obj_description(pg_proc.oid, 'pg_proc'), '' + )::TEXT AS description FROM pg_catalog.pg_proc INNER JOIN pg_catalog.pg_namespace AS proc_namespace @@ -831,6 +883,7 @@ type GetProcsRow struct { FuncLang string FuncIdentityArguments string FuncDef string + Description string } func (q *Queries) GetProcs(ctx context.Context, prokind interface{}) ([]GetProcsRow, error) { @@ -849,6 +902,7 @@ func (q *Queries) GetProcs(ctx context.Context, prokind interface{}) ([]GetProcs &i.FuncLang, &i.FuncIdentityArguments, &i.FuncDef, + &i.Description, ); err != nil { return nil, err } @@ -864,7 +918,11 @@ func (q *Queries) GetProcs(ctx context.Context, prokind interface{}) ([]GetProcs } const getSchemas = `-- name: GetSchemas :many -SELECT nspname::TEXT AS schema_name +SELECT + nspname::TEXT AS schema_name, + COALESCE( + pg_catalog.obj_description(pg_namespace.oid, 'pg_namespace'), '' + )::TEXT AS description FROM pg_catalog.pg_namespace WHERE nspname NOT IN ('pg_catalog', 'information_schema') @@ -881,19 +939,24 @@ WHERE ) ` -func (q *Queries) GetSchemas(ctx context.Context) ([]string, error) { +type GetSchemasRow struct { + SchemaName string + Description string +} + +func (q *Queries) GetSchemas(ctx context.Context) ([]GetSchemasRow, error) { rows, err := q.db.QueryContext(ctx, getSchemas) if err != nil { return nil, err } defer rows.Close() - var items []string + var items []GetSchemasRow for rows.Next() { - var schema_name string - if err := rows.Scan(&schema_name); err != nil { + var i GetSchemasRow + if err := rows.Scan(&i.SchemaName, &i.Description); err != nil { return nil, err } - items = append(items, schema_name) + items = append(items, i) } if err := rows.Close(); err != nil { return nil, err @@ -917,7 +980,10 @@ SELECT pg_seq.seqmin AS min_value, pg_seq.seqcache AS cache_size, pg_seq.seqcycle AS is_cycle, - FORMAT_TYPE(pg_seq.seqtypid, null) AS data_type + FORMAT_TYPE(pg_seq.seqtypid, null) AS data_type, + COALESCE( + pg_catalog.obj_description(pg_seq.seqrelid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_sequence AS pg_seq INNER JOIN pg_catalog.pg_class AS seq_c ON pg_seq.seqrelid = seq_c.oid INNER JOIN pg_catalog.pg_namespace AS seq_ns ON seq_c.relnamespace = seq_ns.oid @@ -966,6 +1032,7 @@ type GetSequencesRow struct { CacheSize int64 IsCycle bool DataType string + Description string } func (q *Queries) GetSequences(ctx context.Context) ([]GetSequencesRow, error) { @@ -990,6 +1057,7 @@ func (q *Queries) GetSequences(ctx context.Context) ([]GetSequencesRow, error) { &i.CacheSize, &i.IsCycle, &i.DataType, + &i.Description, ); err != nil { return nil, err } @@ -1102,7 +1170,10 @@ SELECT (CASE WHEN c.relispartition THEN pg_catalog.pg_get_expr(c.relpartbound, c.oid) ELSE '' - END)::TEXT AS partition_for_values + END)::TEXT AS partition_for_values, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS table_namespace @@ -1143,6 +1214,7 @@ type GetTablesRow struct { ParentTableSchemaName string PartitionKeyDef string PartitionForValues string + Description string } func (q *Queries) GetTables(ctx context.Context) ([]GetTablesRow, error) { @@ -1165,6 +1237,7 @@ func (q *Queries) GetTables(ctx context.Context) ([]GetTablesRow, error) { &i.ParentTableSchemaName, &i.PartitionKeyDef, &i.PartitionForValues, + &i.Description, ); err != nil { return nil, err } @@ -1190,7 +1263,10 @@ SELECT pg_proc.oid ) AS func_identity_arguments, pg_catalog.pg_get_triggerdef(trig.oid) AS trigger_def, - trig.tgconstraint != 0 AS is_constraint + trig.tgconstraint != 0 AS is_constraint, + COALESCE( + pg_catalog.obj_description(trig.oid, 'pg_trigger'), '' + )::TEXT AS description FROM pg_catalog.pg_trigger AS trig INNER JOIN pg_catalog.pg_class AS owning_c ON trig.tgrelid = owning_c.oid INNER JOIN @@ -1217,6 +1293,7 @@ type GetTriggersRow struct { FuncIdentityArguments string TriggerDef string IsConstraint bool + Description string } func (q *Queries) GetTriggers(ctx context.Context) ([]GetTriggersRow, error) { @@ -1237,6 +1314,7 @@ func (q *Queries) GetTriggers(ctx context.Context) ([]GetTriggersRow, error) { &i.FuncIdentityArguments, &i.TriggerDef, &i.IsConstraint, + &i.Description, ); err != nil { return nil, err } @@ -1303,7 +1381,10 @@ SELECT -- Instead, they must be unmarshalled as string arrays. -- https://github.com/lib/pq/pull/466 WHERE d.refobjid = c.oid)::TEXT [] AS table_dependencies, - PG_GET_VIEWDEF(c.oid, true) AS view_definition + PG_GET_VIEWDEF(c.oid, true) AS view_definition, + COALESCE( + pg_catalog.obj_description(c.oid, 'pg_class'), '' + )::TEXT AS description FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid WHERE @@ -1327,6 +1408,7 @@ type GetViewsRow struct { RelOptions []string TableDependencies []string ViewDefinition string + Description string } func (q *Queries) GetViews(ctx context.Context) ([]GetViewsRow, error) { @@ -1344,6 +1426,7 @@ func (q *Queries) GetViews(ctx context.Context) ([]GetViewsRow, error) { pq.Array(&i.RelOptions), pq.Array(&i.TableDependencies), &i.ViewDefinition, + &i.Description, ); err != nil { return nil, err } diff --git a/internal/schema/schema.go b/internal/schema/schema.go index b3945d5..0b23157 100644 --- a/internal/schema/schema.go +++ b/internal/schema/schema.go @@ -196,6 +196,8 @@ const ( // schema type NamedSchema struct { Name string + // Description is the comment attached to the schema (pg_description). Empty means no comment. + Description string } func (n NamedSchema) GetName() string { @@ -205,11 +207,15 @@ func (n NamedSchema) GetName() string { type Extension struct { SchemaQualifiedName Version string + // Description is the comment attached to the extension (pg_description). Empty means no comment. + Description string } type Enum struct { SchemaQualifiedName Labels []string + // Description is the comment attached to the enum type (pg_description). Empty means no comment. + Description string } type Table struct { @@ -229,6 +235,9 @@ type Table struct { ParentTable *SchemaQualifiedName ForValues string + + // Description is the comment attached to the table (pg_description). Empty means no comment. + Description string } func (t Table) IsPartitioned() bool { @@ -303,6 +312,8 @@ type ( // It is used for data-packing purposes Size int Identity *ColumnIdentity + // Description is the comment attached to the column (pg_description). Empty means no comment. + Description string } ) @@ -347,6 +358,8 @@ type ( EscapedConstraintName string ConstraintDef string IsLocal bool + // Description is the comment attached to the constraint (pg_description). Empty means no comment. + Description string } Index struct { @@ -367,6 +380,11 @@ type ( GetIndexDefStmt GetIndexDefStatement ParentIdx *SchemaQualifiedName + + // Description is the comment attached to the index (pg_description). Empty means no comment. + // Note: when the index backs a constraint (PRIMARY KEY / UNIQUE), the comment lives on the + // constraint instead — see IndexConstraint.Description. + Description string } ) @@ -401,6 +419,8 @@ type CheckConstraint struct { IsValid bool IsInheritable bool DependsOnFunctions []SchemaQualifiedName + // Description is the comment attached to the constraint (pg_description). Empty means no comment. + Description string } func (c CheckConstraint) GetName() string { @@ -413,6 +433,8 @@ type ForeignKeyConstraint struct { ForeignTable SchemaQualifiedName ConstraintDef string IsValid bool + // Description is the comment attached to the constraint (pg_description). Empty means no comment. + Description string } func (f ForeignKeyConstraint) GetName() string { @@ -436,6 +458,8 @@ type ( MinValue int64 CacheSize int64 Cycle bool + // Description is the comment attached to the sequence (pg_description). Empty means no comment. + Description string } ) @@ -449,6 +473,8 @@ type Function struct { // can track the dependencies of the function (or not) Language string DependsOnFunctions []SchemaQualifiedName + // Description is the comment attached to the function (pg_description). Empty means no comment. + Description string } type Procedure struct { @@ -457,6 +483,8 @@ type Procedure struct { // the procedure, as returned by `pg_get_functiondef`. It is a CREATE OR REPLACE // statement. Def string + // Description is the comment attached to the procedure (pg_description). Empty means no comment. + Description string } var ( @@ -496,6 +524,8 @@ type Policy struct { UsingExpression string // Columns are the columns that the policy applies to. Columns []string + // Description is the comment attached to the policy (pg_description). Empty means no comment. + Description string } func (p Policy) GetName() string { @@ -510,6 +540,8 @@ type Trigger struct { // by pg_get_triggerdef GetTriggerDefStmt GetTriggerDefStatement IsConstraint bool + // Description is the comment attached to the trigger (pg_description). Empty means no comment. + Description string } func (t Trigger) GetName() string { @@ -531,6 +563,8 @@ type View struct { // TableDependencies is a list of tables the view depends on. TableDependencies []TableDependency + // Description is the comment attached to the view (pg_description). Empty means no comment. + Description string } type MaterializedView struct { @@ -544,6 +578,8 @@ type MaterializedView struct { // TableDependencies is a list of tables the materialized view depends on. TableDependencies []TableDependency + // Description is the comment attached to the materialized view (pg_description). Empty means no comment. + Description string } type ( @@ -842,15 +878,16 @@ func (s *schemaFetcher) getSchema(ctx context.Context) (Schema, error) { } func (s *schemaFetcher) fetchNamedSchemas(ctx context.Context) ([]NamedSchema, error) { - schemaNames, err := s.q.GetSchemas(ctx) + rawSchemas, err := s.q.GetSchemas(ctx) if err != nil { return nil, fmt.Errorf("GetSchemas(): %w", err) } var schemas []NamedSchema - for _, schemaName := range schemaNames { + for _, rs := range rawSchemas { schemas = append(schemas, NamedSchema{ - Name: schemaName, + Name: rs.SchemaName, + Description: rs.Description, }) } @@ -881,7 +918,8 @@ func (s *schemaFetcher) fetchExtensions(ctx context.Context) ([]Extension, error EscapedName: EscapeIdentifier(e.ExtensionName), SchemaName: e.SchemaName, }, - Version: e.ExtensionVersion, + Version: e.ExtensionVersion, + Description: e.Description, }) } @@ -909,7 +947,8 @@ func (s *schemaFetcher) fetchEnums(ctx context.Context) ([]Enum, error) { SchemaName: rawEnum.EnumSchemaName, EscapedName: EscapeIdentifier(rawEnum.EnumName), }, - Labels: rawEnum.EnumLabels, + Labels: rawEnum.EnumLabels, + Description: rawEnum.Description, }) } @@ -1036,6 +1075,7 @@ func (s *schemaFetcher) buildTable( GenerationExpression: column.GenerationExpression, Size: int(column.ColumnSize), Identity: identity, + Description: column.Description, }) } @@ -1064,6 +1104,7 @@ func (s *schemaFetcher) buildTable( ParentTable: parentTable, ForValues: table.PartitionForValues, + Description: table.Description, }, nil } @@ -1131,6 +1172,7 @@ func (s *schemaFetcher) buildCheckConstraint(ctx context.Context, cc queries.Get IsValid: cc.IsValid, IsInheritable: !cc.IsNotInheritable, DependsOnFunctions: dependsOnFunctions, + Description: cc.Description, }, nil } @@ -1165,6 +1207,7 @@ func (s *schemaFetcher) buildIndex(rawIndex queries.GetIndexesRow) Index { EscapedConstraintName: EscapeIdentifier(rawIndex.ConstraintName), ConstraintDef: rawIndex.ConstraintDef, IsLocal: rawIndex.ConstraintIsLocal, + Description: rawIndex.ConstraintDescription, } } @@ -1191,6 +1234,8 @@ func (s *schemaFetcher) buildIndex(rawIndex queries.GetIndexesRow) Index { Constraint: indexConstraint, ParentIdx: parentIdx, + + Description: rawIndex.Description, } } @@ -1214,6 +1259,7 @@ func (s *schemaFetcher) fetchForeignKeyCons(ctx context.Context) ([]ForeignKeyCo }, ConstraintDef: rawFkCon.ConstraintDef, IsValid: rawFkCon.IsValid, + Description: rawFkCon.Description, }) } @@ -1254,14 +1300,15 @@ func (s *schemaFetcher) fetchSequences(ctx context.Context) ([]Sequence, error) SchemaName: rawSeq.SequenceSchemaName, EscapedName: EscapeIdentifier(rawSeq.SequenceName), }, - Owner: owner, - Type: rawSeq.DataType, - StartValue: rawSeq.StartValue, - Increment: rawSeq.IncrementValue, - MaxValue: rawSeq.MaxValue, - MinValue: rawSeq.MinValue, - CacheSize: rawSeq.CacheSize, - Cycle: rawSeq.IsCycle, + Owner: owner, + Type: rawSeq.DataType, + StartValue: rawSeq.StartValue, + Increment: rawSeq.IncrementValue, + MaxValue: rawSeq.MaxValue, + MinValue: rawSeq.MinValue, + CacheSize: rawSeq.CacheSize, + Cycle: rawSeq.IsCycle, + Description: rawSeq.Description, }) } @@ -1325,6 +1372,7 @@ func (s *schemaFetcher) buildFunction(ctx context.Context, rawFunction queries.G FunctionDef: rawFunction.FuncDef, Language: rawFunction.FuncLang, DependsOnFunctions: dependsOnFunctions, + Description: rawFunction.Description, }, nil } @@ -1356,6 +1404,7 @@ func (s *schemaFetcher) fetchProcedures(ctx context.Context) ([]Procedure, error p := Procedure{ SchemaQualifiedName: buildProcName(rawProcedure.FuncName, rawProcedure.FuncIdentityArguments, rawProcedure.FuncSchemaName), Def: rawProcedure.FuncDef, + Description: rawProcedure.Description, } procedures = append(procedures, p) } @@ -1398,6 +1447,7 @@ func (s *schemaFetcher) fetchPolicies(ctx context.Context) ([]policyAndTable, er CheckExpression: rp.CheckExpression, UsingExpression: rp.UsingExpression, Columns: rp.ColumnNames, + Description: rp.Description, }, table: buildNameFromUnescaped(rp.OwningTableName, rp.OwningTableSchemaName), }) @@ -1468,6 +1518,7 @@ func (s *schemaFetcher) fetchTriggers(ctx context.Context) ([]Trigger, error) { Function: buildProcName(rawTrigger.FuncName, rawTrigger.FuncIdentityArguments, rawTrigger.FuncSchemaName), GetTriggerDefStmt: GetTriggerDefStatement(rawTrigger.TriggerDef), IsConstraint: rawTrigger.IsConstraint, + Description: rawTrigger.Description, }) } @@ -1509,6 +1560,7 @@ func (s *schemaFetcher) fetchViews(ctx context.Context) ([]View, error) { Options: options, TableDependencies: tableDependencies, + Description: v.Description, }) } @@ -1548,6 +1600,7 @@ func (s *schemaFetcher) fetchMaterializedViews(ctx context.Context) ([]Materiali Tablespace: mv.TablespaceName, TableDependencies: tableDependencies, + Description: mv.Description, }) } diff --git a/internal/schema/schema_test.go b/internal/schema/schema_test.go index c73bb17..08b0f32 100644 --- a/internal/schema/schema_test.go +++ b/internal/schema/schema_test.go @@ -239,10 +239,10 @@ var ( GRANT SELECT ON schema_2.foo TO some_role_1; GRANT INSERT ON schema_2.foo TO some_role_2 WITH GRANT OPTION; `}, - expectedHash: "4c2174e2cac3956b", + expectedHash: "64ef09172f9f5924", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, {Name: "schema_1"}, {Name: "schema_2"}, }, @@ -252,14 +252,16 @@ var ( SchemaName: "schema_1", EscapedName: EscapeIdentifier("pg_trgm"), }, - Version: "1.6", + Version: "1.6", + Description: "text similarity measurement and index searching based on trigrams", }, { SchemaQualifiedName: SchemaQualifiedName{ SchemaName: "public", EscapedName: EscapeIdentifier("pg_stat_statements"), }, - Version: "1.9", + Version: "1.9", + Description: "track planning and execution statistics of all SQL statements executed", }, }, Enums: []Enum{ @@ -591,10 +593,10 @@ var ( ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content) NOT VALID; `}, - expectedHash: "32c5a9c52dcfb15e", + expectedHash: "9cec6881c66436cb", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -913,7 +915,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -970,7 +972,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -1043,7 +1045,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -1077,7 +1079,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -1105,7 +1107,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -1130,7 +1132,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, {Name: "schema_1"}, }, Tables: []Table{ @@ -1173,10 +1175,10 @@ var ( CREATE TYPE pg_temp.color AS ENUM ('red', 'green', 'blue'); `}, // Assert empty schema hash, since we want to validate specifically that this hash is deterministic - expectedHash: "9c413c6ad2f4a042", + expectedHash: "586c891bd7a36300", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, }, }, @@ -1189,7 +1191,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, }, Tables: []Table{ { @@ -1217,7 +1219,7 @@ var ( `}, expectedSchema: Schema{ NamedSchemas: []NamedSchema{ - {Name: "public"}, + {Name: "public", Description: "standard public schema"}, {Name: "schema_2"}, }, Tables: []Table{ diff --git a/pkg/diff/comment_sql_generator.go b/pkg/diff/comment_sql_generator.go new file mode 100644 index 0000000..3aa0d43 --- /dev/null +++ b/pkg/diff/comment_sql_generator.go @@ -0,0 +1,135 @@ +package diff + +import ( + "fmt" + "strings" + + "github.com/stripe/pg-schema-diff/internal/schema" +) + +// escapeCommentLiteral escapes a string for use as a single-quoted PostgreSQL literal in +// `COMMENT ON ... IS '...'`. +func escapeCommentLiteral(s string) string { + return strings.ReplaceAll(s, "'", "''") +} + +// commentOnStatement builds a `COMMENT ON IS ...` statement. If description +// is empty, the statement uses `IS NULL` to remove any existing comment. +// +// kindAndTarget is the part between `COMMENT ON ` and ` IS ...`, e.g. +// +// COLUMN "public"."t"."c" +// TRIGGER "trg" ON "public"."t" +// CONSTRAINT "ck" ON "public"."t" +// FUNCTION "public"."f"(int) +// TABLE "public"."t" +// SCHEMA "public" +// +// Callers are responsible for assembling that string with the correct kind keyword. +func commentOnStatement(kindAndTarget, description string) Statement { + var literal string + if description == "" { + literal = "NULL" + } else { + literal = fmt.Sprintf("'%s'", escapeCommentLiteral(description)) + } + return Statement{ + DDL: fmt.Sprintf("COMMENT ON %s IS %s", kindAndTarget, literal), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + } +} + +// commentDDLForAdd returns a slice that contains a single COMMENT statement when +// description is non-empty, or nil otherwise. Use this from a SQL generator's `Add` +// path: a freshly-created object has no comment in PG, so we only need to emit when +// the new schema specifies one. +func commentDDLForAdd(kindAndTarget, description string) []Statement { + if description == "" { + return nil + } + return []Statement{commentOnStatement(kindAndTarget, description)} +} + +// commentDDLForAlter returns a slice that contains a single COMMENT statement when +// the description has changed (added, modified, or removed), or nil when both are +// equal. Use this from a SQL generator's `Alter` path. +func commentDDLForAlter(kindAndTarget, oldDescription, newDescription string) []Statement { + if oldDescription == newDescription { + return nil + } + return []Statement{commentOnStatement(kindAndTarget, newDescription)} +} + +// commentTargetTable formats `TABLE `. +func commentTargetTable(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("TABLE %s", name.GetFQEscapedName()) +} + +// commentTargetColumn formats `COLUMN .`. +func commentTargetColumn(table schema.SchemaQualifiedName, columnName string) string { + return fmt.Sprintf("COLUMN %s", schema.FQEscapedColumnName(table, columnName)) +} + +// commentTargetView formats `VIEW `. +func commentTargetView(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("VIEW %s", name.GetFQEscapedName()) +} + +// commentTargetMaterializedView formats `MATERIALIZED VIEW `. +func commentTargetMaterializedView(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("MATERIALIZED VIEW %s", name.GetFQEscapedName()) +} + +// commentTargetSequence formats `SEQUENCE `. +func commentTargetSequence(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("SEQUENCE %s", name.GetFQEscapedName()) +} + +// commentTargetIndex formats `INDEX `. +func commentTargetIndex(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("INDEX %s", name.GetFQEscapedName()) +} + +// commentTargetSchema formats `SCHEMA `. +func commentTargetSchema(unescapedName string) string { + return fmt.Sprintf("SCHEMA %s", schema.EscapeIdentifier(unescapedName)) +} + +// commentTargetExtension formats `EXTENSION `. +func commentTargetExtension(extension schema.Extension) string { + return fmt.Sprintf("EXTENSION %s", extension.EscapedName) +} + +// commentTargetType formats `TYPE `. Used for enum/domain/composite types. +func commentTargetType(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("TYPE %s", name.GetFQEscapedName()) +} + +// commentTargetFunction formats `FUNCTION `. SchemaQualifiedName.EscapedName +// for a Function already includes the argument signature (see schema.buildProcName). +func commentTargetFunction(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("FUNCTION %s", name.GetFQEscapedName()) +} + +// commentTargetProcedure formats `PROCEDURE `. +func commentTargetProcedure(name schema.SchemaQualifiedName) string { + return fmt.Sprintf("PROCEDURE %s", name.GetFQEscapedName()) +} + +// commentTargetTrigger formats `TRIGGER ON `. +func commentTargetTrigger(escapedTriggerName string, owningTable schema.SchemaQualifiedName) string { + return fmt.Sprintf("TRIGGER %s ON %s", escapedTriggerName, owningTable.GetFQEscapedName()) +} + +// commentTargetPolicy formats `POLICY ON `. +func commentTargetPolicy(escapedPolicyName string, owningTable schema.SchemaQualifiedName) string { + return fmt.Sprintf("POLICY %s ON %s", escapedPolicyName, owningTable.GetFQEscapedName()) +} + +// commentTargetConstraint formats `CONSTRAINT ON `. Use this for +// CHECK / FOREIGN KEY / PRIMARY KEY / UNIQUE constraints — comments live on the +// constraint, not the underlying index. +func commentTargetConstraint(escapedConstraintName string, owningTable schema.SchemaQualifiedName) string { + return fmt.Sprintf("CONSTRAINT %s ON %s", escapedConstraintName, owningTable.GetFQEscapedName()) +} diff --git a/pkg/diff/enum_sql_generator.go b/pkg/diff/enum_sql_generator.go index de5ab4f..5fafba5 100644 --- a/pkg/diff/enum_sql_generator.go +++ b/pkg/diff/enum_sql_generator.go @@ -19,13 +19,15 @@ func (e *enumSQLGenerator) Add(enum schema.Enum) ([]Statement, error) { for _, val := range enum.Labels { escapedEnumVals = append(escapedEnumVals, fmt.Sprintf("'%s'", val)) } - return []Statement{ + stmts := []Statement{ { DDL: fmt.Sprintf("CREATE TYPE %s AS ENUM (%s)", enum.GetFQEscapedName(), strings.Join(escapedEnumVals, ", ")), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, }, - }, nil + } + stmts = append(stmts, commentDDLForAdd(commentTargetType(enum.SchemaQualifiedName), enum.Description)...) + return stmts, nil } func (e *enumSQLGenerator) Delete(enum schema.Enum) ([]Statement, error) { @@ -40,6 +42,9 @@ func (e *enumSQLGenerator) Delete(enum schema.Enum) ([]Statement, error) { func (e *enumSQLGenerator) Alter(diff enumDiff) ([]Statement, error) { oldCopy := diff.old + // Mask Description: a comment-only diff is handled by an explicit COMMENT statement + // emitted at the end, so it must not trip the final cmp.Diff equality check. + oldCopy.Description = diff.new.Description oldVals := set.NewSet(diff.old.Labels...) newVals := set.NewSet(diff.new.Labels...) if len(set.Difference(oldVals, newVals)) > 0 { @@ -88,9 +93,10 @@ func (e *enumSQLGenerator) Alter(diff enumDiff) ([]Statement, error) { } oldCopy.Labels = diff.new.Labels - if diff := cmp.Diff(oldCopy, diff.new); diff != "" { - return nil, fmt.Errorf("unable to resolve the diff %s: %w", diff, ErrNotImplemented) + if d := cmp.Diff(oldCopy, diff.new); d != "" { + return nil, fmt.Errorf("unable to resolve the diff %s: %w", d, ErrNotImplemented) } + stmts = append(stmts, commentDDLForAlter(commentTargetType(diff.new.SchemaQualifiedName), diff.old.Description, diff.new.Description)...) return stmts, nil } diff --git a/pkg/diff/function_sql_vertex_generator.go b/pkg/diff/function_sql_vertex_generator.go index 088ff6a..993647c 100644 --- a/pkg/diff/function_sql_vertex_generator.go +++ b/pkg/diff/function_sql_vertex_generator.go @@ -30,12 +30,14 @@ func (f *functionSQLVertexGenerator) Add(function schema.Function) ([]Statement, "created/altered before this statement.", }) } - return []Statement{{ + stmts := []Statement{{ DDL: function.FunctionDef, Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: hazards, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetFunction(function.SchemaQualifiedName), function.Description)...) + return stmts, nil } func (f *functionSQLVertexGenerator) Delete(function schema.Function) ([]Statement, error) { @@ -63,7 +65,25 @@ func (f *functionSQLVertexGenerator) Alter(diff functionDiff) ([]Statement, erro if cmp.Equal(diff.old, diff.new) { return nil, nil } - return f.Add(diff.new) + + // Comment-only diff: don't `CREATE OR REPLACE`, just emit a COMMENT statement. + oldCopy := diff.old + oldCopy.Description = diff.new.Description + if cmp.Equal(oldCopy, diff.new) { + return commentDDLForAlter(commentTargetFunction(diff.new.SchemaQualifiedName), diff.old.Description, diff.new.Description), nil + } + + // Add() emits CREATE OR REPLACE plus the COMMENT statement (if Description is non-empty + // in the new schema). For ALTER we additionally need to emit `COMMENT ON ... IS NULL` + // when Description was removed. + stmts, err := f.Add(diff.new) + if err != nil { + return nil, err + } + if diff.new.Description == "" && diff.old.Description != "" { + stmts = append(stmts, commentOnStatement(commentTargetFunction(diff.new.SchemaQualifiedName), "")) + } + return stmts, nil } func canFunctionDependenciesBeTracked(function schema.Function) bool { diff --git a/pkg/diff/materialized_view_sql_generator.go b/pkg/diff/materialized_view_sql_generator.go index 9b5d216..e86977b 100644 --- a/pkg/diff/materialized_view_sql_generator.go +++ b/pkg/diff/materialized_view_sql_generator.go @@ -109,15 +109,18 @@ func (mvsg *materializedViewSQLGenerator) Add(mv schema.MaterializedView) (parti deps = append(deps, mustRun(addVertexId).after(buildTableVertexId(t.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: materializedViewSb.String(), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + }} + stmts = append(stmts, commentDDLForAdd(commentTargetMaterializedView(mv.SchemaQualifiedName), mv.Description)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: addVertexId, - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: materializedViewSb.String(), - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }}, + id: addVertexId, + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -148,11 +151,25 @@ func (mvsg *materializedViewSQLGenerator) Delete(mv schema.MaterializedView) (pa } func (mvsg *materializedViewSQLGenerator) Alter(mvd materializedViewDiff) (partialSQLGraph, error) { - // In the initial MVP, we will not support altering. - if !cmp.Equal(mvd.old, mvd.new) { + // Mask Description: a comment-only diff is altered via an explicit COMMENT statement. + oldCopy := mvd.old + oldCopy.Description = mvd.new.Description + if !cmp.Equal(oldCopy, mvd.new) { + // In the initial MVP, we don't support altering anything other than the comment. return partialSQLGraph{}, ErrNotImplemented } - return partialSQLGraph{}, nil + + commentStmts := commentDDLForAlter(commentTargetMaterializedView(mvd.new.SchemaQualifiedName), mvd.old.Description, mvd.new.Description) + if len(commentStmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildMaterializedViewVertexId(mvd.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: commentStmts, + }}, + }, nil } func buildMaterializedViewVertexId(n schema.SchemaQualifiedName, d diffType) sqlVertexId { diff --git a/pkg/diff/named_schema_sql_generator.go b/pkg/diff/named_schema_sql_generator.go index a8d758f..c8c6ae8 100644 --- a/pkg/diff/named_schema_sql_generator.go +++ b/pkg/diff/named_schema_sql_generator.go @@ -11,11 +11,13 @@ import ( type namedSchemaSQLGenerator struct{} func (n *namedSchemaSQLGenerator) Add(s schema.NamedSchema) ([]Statement, error) { - return []Statement{{ + stmts := []Statement{{ DDL: fmt.Sprintf("CREATE SCHEMA %s", schema.EscapeIdentifier(s.Name)), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetSchema(s.Name), s.Description)...) + return stmts, nil } func (n *namedSchemaSQLGenerator) Delete(s schema.NamedSchema) ([]Statement, error) { @@ -26,6 +28,6 @@ func (n *namedSchemaSQLGenerator) Delete(s schema.NamedSchema) ([]Statement, err }}, nil } -func (n *namedSchemaSQLGenerator) Alter(_ namedSchemaDiff) ([]Statement, error) { - return nil, nil +func (n *namedSchemaSQLGenerator) Alter(d namedSchemaDiff) ([]Statement, error) { + return commentDDLForAlter(commentTargetSchema(d.new.Name), d.old.Description, d.new.Description), nil } diff --git a/pkg/diff/policy_sql_generator.go b/pkg/diff/policy_sql_generator.go index 370df96..8aff468 100644 --- a/pkg/diff/policy_sql_generator.go +++ b/pkg/diff/policy_sql_generator.go @@ -178,12 +178,14 @@ func (psg *policySQLVertexGenerator) Add(p schema.Policy) ([]Statement, error) { hazard = migrationHazardPermissivePolicyAdded } - return []Statement{{ + stmts := []Statement{{ DDL: sb.String(), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: []MigrationHazard{hazard}, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetPolicy(p.EscapedName, psg.table.SchemaQualifiedName), p.Description)...) + return stmts, nil } func policyCharToSQL(c schema.PolicyCmd) (string, error) { @@ -218,6 +220,9 @@ func (psg *policySQLVertexGenerator) Delete(p schema.Policy) ([]Statement, error func (psg *policySQLVertexGenerator) Alter(diff policyDiff) ([]Statement, error) { oldCopy := diff.old + // Mask Description: a comment-only diff is altered via an explicit COMMENT statement + // emitted at the end. Without masking it would trip the unsupported-diff check below. + oldCopy.Description = diff.new.Description // alterPolicyParts represents the set of strings to include in the ALTER POLICY ... ON TABLE ... statement var alterPolicyParts []string @@ -241,25 +246,29 @@ func (psg *policySQLVertexGenerator) Alter(diff policyDiff) ([]Statement, error) } oldCopy.Columns = diff.new.Columns - if diff := cmp.Diff(oldCopy, diff.new); diff != "" { - return nil, fmt.Errorf("unsupported diff %s: %w", diff, ErrNotImplemented) + if d := cmp.Diff(oldCopy, diff.new); d != "" { + return nil, fmt.Errorf("unsupported diff %s: %w", d, ErrNotImplemented) } + commentStmts := commentDDLForAlter(commentTargetPolicy(diff.new.EscapedName, psg.table.SchemaQualifiedName), diff.old.Description, diff.new.Description) + if len(alterPolicyParts) == 0 { - // There is no diff - return nil, nil + // No structural diff — return the comment-only diff (possibly empty). + return commentStmts, nil } sb := strings.Builder{} sb.WriteString(fmt.Sprintf("ALTER POLICY %s ON %s\n\t", diff.new.EscapedName, psg.table.GetFQEscapedName())) sb.WriteString(strings.Join(alterPolicyParts, "\n\t")) - return []Statement{{ + stmts := []Statement{{ DDL: sb.String(), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: []MigrationHazard{migrationHazardPolicyAltered}, - }}, nil + }} + stmts = append(stmts, commentStmts...) + return stmts, nil } func (psg *policySQLVertexGenerator) GetSQLVertexId(p schema.Policy, diffType diffType) sqlVertexId { diff --git a/pkg/diff/procedure_sql_vertex_generator.go b/pkg/diff/procedure_sql_vertex_generator.go index d9f725b..492ff0d 100644 --- a/pkg/diff/procedure_sql_vertex_generator.go +++ b/pkg/diff/procedure_sql_vertex_generator.go @@ -39,22 +39,25 @@ func (p procedureSQLVertexGenerator) Add(s schema.Procedure) (partialSQLGraph, e deps = append(deps, mustRun(buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter)).after(buildSequenceVertexId(seq.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: s.Def, + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + Hazards: []MigrationHazard{{ + Type: MigrationHazardTypeHasUntrackableDependencies, + Message: "Dependencies of procedures are not tracked by Postgres. " + + "As a result, we cannot guarantee that this procedure's dependencies are ordered properly relative to " + + "this statement. For adds, this means you need to ensure that all objects this function depends on " + + "are added before this statement.", + }}, + }} + stmts = append(stmts, commentDDLForAdd(commentTargetProcedure(s.SchemaQualifiedName), s.Description)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter), - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: s.Def, - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{{ - Type: MigrationHazardTypeHasUntrackableDependencies, - Message: "Dependencies of procedures are not tracked by Postgres. " + - "As a result, we cannot guarantee that this procedure's dependencies are ordered properly relative to " + - "this statement. For adds, this means you need to ensure that all objects this function depends on " + - "are added before this statement.", - }}, - }}, + id: buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -108,8 +111,38 @@ func (p procedureSQLVertexGenerator) Alter(d procedureDiff) (partialSQLGraph, er if cmp.Equal(d.old, d.new) { return partialSQLGraph{}, nil } - // New adds or replaces the procedure. - return p.Add(d.new) + + // Comment-only diff: don't recreate, emit a COMMENT statement only. + oldCopy := d.old + oldCopy.Description = d.new.Description + if cmp.Equal(oldCopy, d.new) { + commentStmts := commentDDLForAlter(commentTargetProcedure(d.new.SchemaQualifiedName), d.old.Description, d.new.Description) + if len(commentStmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildProcedureVertexId(d.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: commentStmts, + }}, + }, nil + } + + // New adds or replaces the procedure (Add() also re-emits the COMMENT for the new schema). + graph, err := p.Add(d.new) + if err != nil { + return partialSQLGraph{}, err + } + // Add() didn't emit anything when Description == "" — but if the old schema had a + // description and the new one doesn't, we still need to clear it explicitly. + if d.new.Description == "" && d.old.Description != "" { + for i := range graph.vertices { + graph.vertices[i].statements = append(graph.vertices[i].statements, + commentOnStatement(commentTargetProcedure(d.new.SchemaQualifiedName), "")) + } + } + return graph, nil } func buildProcedureVertexId(name schema.SchemaQualifiedName, diffType diffType) sqlVertexId { diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go index 6c31e22..87da0f2 100644 --- a/pkg/diff/sql_generator.go +++ b/pkg/diff/sql_generator.go @@ -536,6 +536,15 @@ func buildIndexDiff(deps indexDiffConfig, old, new schema.Index) (diff indexDiff updatedOld.IsInvalid = new.IsInvalid } + // Description-only diffs are emitted as a COMMENT ON ... statement by the index Alter + // generator, so they must not force recreation here. + updatedOld.Description = new.Description + if updatedOld.Constraint != nil && new.Constraint != nil { + updatedConstraint := *updatedOld.Constraint + updatedConstraint.Description = new.Constraint.Description + updatedOld.Constraint = &updatedConstraint + } + recreateIndex := !cmp.Equal(updatedOld, new) return indexDiff{ oldAndNew: oldAndNew[schema.Index]{ @@ -841,6 +850,14 @@ func (t *tableSQLVertexGenerator) Add(table schema.Table) ([]Statement, error) { LockTimeout: lockTimeoutDefault, }) + // Emit COMMENT ON TABLE / COMMENT ON COLUMN immediately after CREATE TABLE so that + // metadata travels with the structural DDL. PG drops these along with the table on + // failure rollback, so no hazards are needed. + stmts = append(stmts, commentDDLForAdd(commentTargetTable(table.SchemaQualifiedName), table.Description)...) + for _, c := range table.Columns { + stmts = append(stmts, commentDDLForAdd(commentTargetColumn(table.SchemaQualifiedName, c.Name), c.Description)...) + } + csg := checkConstraintSQLVertexGenerator{ tableName: table.SchemaQualifiedName, isNewTable: true, @@ -930,6 +947,7 @@ func (t *tableSQLVertexGenerator) Alter(diff tableDiff) ([]Statement, error) { } var stmts []Statement + stmts = append(stmts, commentDDLForAlter(commentTargetTable(diff.new.SchemaQualifiedName), diff.old.Description, diff.new.Description)...) // Only handle disabling RLS if it was previously enabled. // We want to disable RLS before we do any other operations on the table, e.g., delete policies, to avoid creating an // outage while RLS is being disabled @@ -1267,7 +1285,9 @@ func (csg *columnSQLVertexGenerator) Add(column schema.Column) ([]Statement, err if newColumnRequiresFullTableRewrite(column) { stmt.Hazards = append(stmt.Hazards, migrationHazardNewColumnFullTableRewrite) } - return []Statement{stmt}, nil + stmts := []Statement{stmt} + stmts = append(stmts, commentDDLForAdd(commentTargetColumn(csg.tableName, column.Name), column.Description)...) + return stmts, nil } func newColumnRequiresFullTableRewrite(column schema.Column) bool { @@ -1387,6 +1407,8 @@ func (csg *columnSQLVertexGenerator) Alter(diff columnDiff) ([]Statement, error) }) } + stmts = append(stmts, commentDDLForAlter(commentTargetColumn(csg.tableName, newColumn.Name), oldColumn.Description, newColumn.Description)...) + return stmts, nil } @@ -1732,11 +1754,14 @@ func (isg *indexSQLVertexGenerator) addIdxStmtsWithHazards(index schema.Index) ( // If the table is the base table of a partitioned table, the constraint should "ONLY" be added to the base //table. We can then concurrently build all of the partitioned indexes and attach them. // Without "ONLY", all the partitioned indexes will be automatically built - return []Statement{{ + stmts := []Statement{{ DDL: fmt.Sprintf("ALTER TABLE ONLY %s ADD CONSTRAINT %s %s", index.OwningRelName.GetFQEscapedName(), index.Constraint.EscapedConstraintName, index.Constraint.ConstraintDef), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(index.Constraint.EscapedConstraintName, index.OwningRelName), index.Constraint.Description)...) + stmts = append(stmts, commentDDLForAdd(commentTargetIndex(index.GetSchemaQualifiedName()), index.Description)...) + return stmts, nil } // Only indexes on non-partitioned tables can be created concurrently @@ -1753,8 +1778,11 @@ func (isg *indexSQLVertexGenerator) addIdxStmtsWithHazards(index schema.Index) ( return nil, fmt.Errorf("generating add constraint statement: %w", err) } stmts = append(stmts, addConstraintStmt) + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(index.Constraint.EscapedConstraintName, index.OwningRelName), index.Constraint.Description)...) } + stmts = append(stmts, commentDDLForAdd(commentTargetIndex(index.GetSchemaQualifiedName()), index.Description)...) + if index.ParentIdx != nil && isg.attachPartitionSQLVertexGenerator.isPartitionAlreadyAttachedBeforeIndexBuilds(index.OwningRelName) { // Only attach the index if the index is built after the table is partitioned. If the partition // hasn't already been attached, the index/constraint will be automatically attached when the table partition is @@ -1885,6 +1913,7 @@ func (isg *indexSQLVertexGenerator) Alter(diff indexDiff) ([]Statement, error) { return nil, fmt.Errorf("generating add constraint statement: %w", err) } stmts = append(stmts, addConstraintStmt) + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(diff.new.Constraint.EscapedConstraintName, diff.new.OwningRelName), diff.new.Constraint.Description)...) diff.old.Constraint = diff.new.Constraint } @@ -1893,10 +1922,28 @@ func (isg *indexSQLVertexGenerator) Alter(diff indexDiff) ([]Statement, error) { diff.old.ParentIdx = diff.new.ParentIdx } + // Mask Description fields: handled by explicit COMMENT statements emitted at the end. + indexDescChanged := diff.old.Description != diff.new.Description + diff.old.Description = diff.new.Description + var oldConstraintDescChanged bool + if diff.old.Constraint != nil && diff.new.Constraint != nil { + oldConstraintDescChanged = diff.old.Constraint.Description != diff.new.Constraint.Description + oldCon := *diff.old.Constraint + oldCon.Description = diff.new.Constraint.Description + diff.old.Constraint = &oldCon + } + if !cmp.Equal(diff.old, diff.new) { return nil, fmt.Errorf("index diff could not be resolved %s", cmp.Diff(diff.old, diff.new)) } + if indexDescChanged { + stmts = append(stmts, commentOnStatement(commentTargetIndex(diff.new.GetSchemaQualifiedName()), diff.new.Description)) + } + if oldConstraintDescChanged { + stmts = append(stmts, commentOnStatement(commentTargetConstraint(diff.new.Constraint.EscapedConstraintName, diff.new.OwningRelName), diff.new.Constraint.Description)) + } + return stmts, nil } @@ -2096,6 +2143,7 @@ func (csg *checkConstraintSQLVertexGenerator) Add(con schema.CheckConstraint) ([ stmts = append(stmts, validateConstraintStatement(csg.tableName, schema.EscapeIdentifier(con.Name))) } + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(schema.EscapeIdentifier(con.Name), csg.tableName), con.Description)...) return stmts, nil } @@ -2142,6 +2190,8 @@ func (csg *checkConstraintSQLVertexGenerator) Delete(con schema.CheckConstraint) func (csg *checkConstraintSQLVertexGenerator) Alter(diff checkConstraintDiff) ([]Statement, error) { oldCopy := diff.old + // Mask Description: handled by an explicit COMMENT statement at the end. + oldCopy.Description = diff.new.Description var stmts []Statement if !diff.old.IsValid && diff.new.IsValid { @@ -2161,6 +2211,7 @@ func (csg *checkConstraintSQLVertexGenerator) Alter(diff checkConstraintDiff) ([ return nil, fmt.Errorf("check constraints that depend on UDFs: %w", ErrNotImplemented) } + stmts = append(stmts, commentDDLForAlter(commentTargetConstraint(schema.EscapeIdentifier(diff.new.Name), csg.tableName), diff.old.Description, diff.new.Description)...) return stmts, nil } @@ -2419,18 +2470,20 @@ func (f *foreignKeyConstraintSQLVertexGenerator) Add(con schema.ForeignKeyConstr "Instead, consider adding the constraint as NOT VALID and validating it later.", }) } - return []Statement{{ + stmts := []Statement{{ DDL: fmt.Sprintf("%s %s", addConstraintPrefix(con.OwningTable, con.EscapedName), con.ConstraintDef), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: hazards, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(con.EscapedName, con.OwningTable), con.Description)...) + return stmts, nil } func (f *foreignKeyConstraintSQLVertexGenerator) addAsInvalidThenValidateStatements(con schema.ForeignKeyConstraint) []Statement { // If adding a valid constraint, we will first add the constraint as not valid then validate it in order to // circumvent requiring a SHARE_ROW_EXCLUSIVE lock on the tables. - return []Statement{ + stmts := []Statement{ { DDL: fmt.Sprintf("%s %s NOT VALID", addConstraintPrefix(con.OwningTable, con.EscapedName), con.ConstraintDef), Timeout: statementTimeoutDefault, @@ -2438,6 +2491,8 @@ func (f *foreignKeyConstraintSQLVertexGenerator) addAsInvalidThenValidateStateme }, validateConstraintStatement(con.OwningTable, con.EscapedName), } + stmts = append(stmts, commentDDLForAdd(commentTargetConstraint(con.EscapedName, con.OwningTable), con.Description)...) + return stmts } func (f *foreignKeyConstraintSQLVertexGenerator) Delete(con schema.ForeignKeyConstraint) ([]Statement, error) { @@ -2464,10 +2519,16 @@ func (f *foreignKeyConstraintSQLVertexGenerator) Alter(diff foreignKeyConstraint diff.old.ConstraintDef = strings.TrimSuffix(diff.old.ConstraintDef, " NOT VALID") stmts = append(stmts, validateConstraintStatement(diff.new.OwningTable, diff.new.EscapedName)) } + // Mask Description: handled by an explicit COMMENT statement at the end. + descChanged := diff.old.Description != diff.new.Description + diff.old.Description = diff.new.Description if !cmp.Equal(diff.old, diff.new) { return nil, fmt.Errorf("altering foreign key constraint to resolve the following diff %s: %w", cmp.Diff(diff.old, diff.new), ErrNotImplemented) } + if descChanged { + stmts = append(stmts, commentOnStatement(commentTargetConstraint(diff.new.EscapedName, diff.new.OwningTable), diff.new.Description)) + } return stmts, nil } @@ -2520,9 +2581,11 @@ type sequenceSQLVertexGenerator struct { } func (s *sequenceSQLVertexGenerator) Add(seq schema.Sequence) ([]Statement, error) { - return []Statement{ + stmts := []Statement{ s.buildAddAlterSequenceStatement(seq, false), - }, nil + } + stmts = append(stmts, commentDDLForAdd(commentTargetSequence(seq.SchemaQualifiedName), seq.Description)...) + return stmts, nil } func (s *sequenceSQLVertexGenerator) Delete(seq schema.Sequence) ([]Statement, error) { @@ -2547,6 +2610,9 @@ func (s *sequenceSQLVertexGenerator) Alter(diff sequenceDiff) ([]Statement, erro var stmts []Statement // Ownership changes handled by the sequenceOwnershipSQLVertexGenerator diff.old.Owner = diff.new.Owner + // Mask Description: handled by an explicit COMMENT statement below. + descChanged := diff.old.Description != diff.new.Description + diff.old.Description = diff.new.Description // Explicitly list all the diffs supported by the alter statement, rather than just using !cmp.Equal, so we don't // risk introducing a bug if we add new fields to schema.Sequence @@ -2573,6 +2639,10 @@ func (s *sequenceSQLVertexGenerator) Alter(diff sequenceDiff) ([]Statement, erro return nil, fmt.Errorf("altering sequence to resolve the following diff %s: %w", cmp.Diff(diff.old, diff.new), ErrNotImplemented) } + if descChanged { + stmts = append(stmts, commentOnStatement(commentTargetSequence(diff.new.SchemaQualifiedName), diff.new.Description)) + } + return stmts, nil } @@ -2734,12 +2804,14 @@ func (e *extensionSQLGenerator) Add(extension schema.Extension) ([]Statement, er s += fmt.Sprintf(" VERSION %s", schema.EscapeIdentifier(extension.Version)) } - return []Statement{{ + stmts := []Statement{{ DDL: s, Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: nil, - }}, nil + }} + stmts = append(stmts, commentDDLForAdd(commentTargetExtension(extension), extension.Description)...) + return stmts, nil } func (e *extensionSQLGenerator) Delete(extension schema.Extension) ([]Statement, error) { @@ -2753,6 +2825,7 @@ func (e *extensionSQLGenerator) Delete(extension schema.Extension) ([]Statement, func (e *extensionSQLGenerator) Alter(diff extensionDiff) ([]Statement, error) { var statements []Statement + statements = append(statements, commentDDLForAlter(commentTargetExtension(diff.new), diff.old.Description, diff.new.Description)...) if diff.new.Version != diff.old.Version { if len(diff.new.Version) == 0 { // This is an implicit upgrade to the latest extension version. diff --git a/pkg/diff/trigger_sql_vertex_generator.go b/pkg/diff/trigger_sql_vertex_generator.go index 00ef8a1..abd8256 100644 --- a/pkg/diff/trigger_sql_vertex_generator.go +++ b/pkg/diff/trigger_sql_vertex_generator.go @@ -28,11 +28,13 @@ func (t *triggerSQLVertexGenerator) Add(trigger schema.Trigger) ([]Statement, er } func (t *triggerSQLVertexGenerator) addStatements(trigger schema.Trigger) []Statement { - return []Statement{{ + stmts := []Statement{{ DDL: string(trigger.GetTriggerDefStmt), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, }} + stmts = append(stmts, commentDDLForAdd(commentTargetTrigger(trigger.EscapedName, trigger.OwningTable), trigger.Description)...) + return stmts } func (t *triggerSQLVertexGenerator) Delete(trigger schema.Trigger) ([]Statement, error) { @@ -52,6 +54,13 @@ func (t *triggerSQLVertexGenerator) Alter(diff triggerDiff) ([]Statement, error) return nil, nil } + // Comment-only diff: don't recreate the trigger, just emit a COMMENT statement. + oldCopy := diff.old + oldCopy.Description = diff.new.Description + if cmp.Equal(oldCopy, diff.new) { + return commentDDLForAlter(commentTargetTrigger(diff.new.EscapedName, diff.new.OwningTable), diff.old.Description, diff.new.Description), nil + } + if diff.old.IsConstraint || diff.new.IsConstraint { // Constraint triggers do not support "CREATE OR REPLACE", so just drop the original trigger and // create the new one. @@ -63,11 +72,13 @@ func (t *triggerSQLVertexGenerator) Alter(diff triggerDiff) ([]Statement, error) if err != nil { return nil, fmt.Errorf("modifying get trigger def statement to create or replace: %w", err) } - return []Statement{{ + stmts := []Statement{{ DDL: createOrReplaceStmt, Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, - }}, nil + }} + stmts = append(stmts, commentDDLForAlter(commentTargetTrigger(diff.new.EscapedName, diff.new.OwningTable), diff.old.Description, diff.new.Description)...) + return stmts, nil } func (t *triggerSQLVertexGenerator) GetSQLVertexId(trigger schema.Trigger, diffType diffType) sqlVertexId { diff --git a/pkg/diff/view_sql_generator.go b/pkg/diff/view_sql_generator.go index d96f28d..1569e5c 100644 --- a/pkg/diff/view_sql_generator.go +++ b/pkg/diff/view_sql_generator.go @@ -104,15 +104,18 @@ func (vsg *viewSQLGenerator) Add(v schema.View) (partialSQLGraph, error) { deps = append(deps, mustRun(addVertexId).after(buildTableVertexId(t.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: viewSb.String(), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + }} + stmts = append(stmts, commentDDLForAdd(commentTargetView(v.SchemaQualifiedName), v.Description)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: addVertexId, - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: viewSb.String(), - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }}, + id: addVertexId, + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -143,11 +146,25 @@ func (vsg *viewSQLGenerator) Delete(v schema.View) (partialSQLGraph, error) { } func (vsg *viewSQLGenerator) Alter(vd viewDiff) (partialSQLGraph, error) { - // In the initial MVP, we will not support altering. - if !cmp.Equal(vd.old, vd.new) { + // Mask Description: a comment-only diff is altered via an explicit COMMENT statement. + oldCopy := vd.old + oldCopy.Description = vd.new.Description + if !cmp.Equal(oldCopy, vd.new) { + // In the initial MVP, we don't support altering anything other than the comment. return partialSQLGraph{}, ErrNotImplemented } - return partialSQLGraph{}, nil + + commentStmts := commentDDLForAlter(commentTargetView(vd.new.SchemaQualifiedName), vd.old.Description, vd.new.Description) + if len(commentStmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildTableVertexId(vd.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: commentStmts, + }}, + }, nil } func buildViewVertexId(n schema.SchemaQualifiedName, d diffType) sqlVertexId { diff --git a/pkg/tempdb/factory_test.go b/pkg/tempdb/factory_test.go index d271fb6..0b88033 100644 --- a/pkg/tempdb/factory_test.go +++ b/pkg/tempdb/factory_test.go @@ -174,7 +174,8 @@ func (suite *onInstanceTempDbFactorySuite) TestCreate_CreateAndDropFlow() { suite.Require().NoError(err) suite.Equal(internalschema.Schema{ NamedSchemas: []internalschema.NamedSchema{{ - Name: "public", + Name: "public", + Description: "standard public schema", }}, }, schema)