From 8670ec57fb22529566866db110ed92c7f5bfd2e7 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Thu, 7 Jul 2022 20:51:48 +0300 Subject: [PATCH 1/5] update parser test to assert that parsing COMMENT comments work on Producer::PG * also verifies that `comments` become an array --- t/14postgres-parser.t | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/14postgres-parser.t b/t/14postgres-parser.t index 3c9dcd449..14d99c179 100644 --- a/t/14postgres-parser.t +++ b/t/14postgres-parser.t @@ -41,6 +41,8 @@ baz $foo$, f_text2 text default $$$$, f_text3 text default $$$ $$ ); + COMMENT on TABLE t_test1 IS 'this is another comment on t_test1'; + COMMENT on COLUMN t_test1.f_serial IS 'this is another comment on f_serial'; create table t_test2 ( f_id integer NOT NULL, @@ -128,7 +130,7 @@ is( scalar @tables, 5, 'Five tables' ); my $t1 = shift @tables; is( $t1->name, 't_test1', 'Table t_test1 exists' ); -is( $t1->comments, 'comment on t_test1', 'Table comment exists' ); +is_deeply([ $t1->comments ], [ 'comment on t_test1', 'this is another comment on t_test1' ], 'Table comment exists' ); my @t1_fields = $t1->get_fields; is( scalar @t1_fields, 21, '21 fields in t_test1' ); @@ -140,7 +142,7 @@ is( $f1->is_nullable, 0, 'Field cannot be null' ); is( $f1->size, 11, 'Size is "11"' ); is( $f1->default_value, '0', 'Default value is "0"' ); is( $f1->is_primary_key, 1, 'Field is PK' ); -is( $f1->comments, 'this is the primary key', 'Comment' ); +is_deeply( [ $f1->comments ], [ 'this is the primary key', 'this is another comment on f_serial' ], 'Comment' ); is( $f1->is_auto_increment, 1, 'Field is auto increment' ); my $f2 = shift @t1_fields; From be4fe502d7d3010b1175d239096c34b7e82eaa1a Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Thu, 7 Jul 2022 22:09:18 +0300 Subject: [PATCH 2/5] implementation of comment DDL *should* be correct * let's fix them tests! --- lib/SQL/Translator/Producer/PostgreSQL.pm | 32 ++++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 7520563ff..0dae4378b 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -273,9 +273,15 @@ sub create_table push @comments, "--\n-- Table: $table_name\n--\n" unless $no_comments; - if ( !$no_comments and my $comments = $table->comments ) { - $comments =~ s/^/-- /mg; - push @comments, "-- Comments:\n$comments\n--\n"; + my @comment_statements; + if ( my $comments = $table->comments ) { + # this follows the example in the MySQL producer, where all comments are added as + # table comments, even though they could have originally been parsed as DDL comments + my $comment_ddl = + 'CREATE COMMENT on TABLE ' + . $table_name_qt + . ' IS $comment$' . (join ',', @$comments) . '$comment$;'; + push @comment_statements, $comment_ddl; } # @@ -288,6 +294,15 @@ sub create_table type_defs => $type_defs, constraint_defs => \@constraint_defs, }); + my $field_comments = $field->comments; + next unless $field_comments; + my $field_name_qt = $generator->quote($field->name); + my $comment_ddl = + 'CREATE COMMENT ON COLUMN ' + . "$table_name_qt.$field_name_qt " + . ' IS $comment$' . (join ',', @$field_comments) . '$comment$'; + push @comment_statements, $comment_ddl; + } # @@ -337,6 +352,9 @@ sub create_table $create_statement .= join(";\n", '', map{ drop_geometry_column($_, $options) } @geometry_columns) if $options->{add_drop_table}; $create_statement .= join(";\n", '', map{ add_geometry_column($_, $options) } @geometry_columns); } + if (@comment_statements) { + $create_statement .= join(";\n", '', @comment_statements); + } return $create_statement, \@fks; } @@ -398,13 +416,7 @@ sub create_view { $field_name_scope{$table_name} ||= {}; my $field_name = $field->name; - my $field_comments = ''; - if (my $comments = $field->comments) { - $comments =~ s/(?quote($field_name); + my $field_def = $generator->quote($field_name); # # Datatype From 4ae6f235a2e112c22f31edc501d10e9939837485 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Thu, 7 Jul 2022 23:38:40 +0300 Subject: [PATCH 3/5] woops, didn't understand how `->comments` worked --- lib/SQL/Translator/Producer/PostgreSQL.pm | 10 +++------- t/14postgres-parser.t | 6 ++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 0dae4378b..ef5292bec 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -277,10 +277,8 @@ sub create_table if ( my $comments = $table->comments ) { # this follows the example in the MySQL producer, where all comments are added as # table comments, even though they could have originally been parsed as DDL comments - my $comment_ddl = - 'CREATE COMMENT on TABLE ' - . $table_name_qt - . ' IS $comment$' . (join ',', @$comments) . '$comment$;'; + # quoted via $$ string so there can be 'quotes' inside the comments + my $comment_ddl = "COMMENT on TABLE $table_name_qt IS \$comment\$$comments\$comment\$"; push @comment_statements, $comment_ddl; } @@ -298,9 +296,7 @@ sub create_table next unless $field_comments; my $field_name_qt = $generator->quote($field->name); my $comment_ddl = - 'CREATE COMMENT ON COLUMN ' - . "$table_name_qt.$field_name_qt " - . ' IS $comment$' . (join ',', @$field_comments) . '$comment$'; + "COMMENT ON COLUMN $table_name_qt.$field_name_qt IS \$comment\$$field_comments\$comment\$"; push @comment_statements, $comment_ddl; } diff --git a/t/14postgres-parser.t b/t/14postgres-parser.t index 14d99c179..3c9dcd449 100644 --- a/t/14postgres-parser.t +++ b/t/14postgres-parser.t @@ -41,8 +41,6 @@ baz $foo$, f_text2 text default $$$$, f_text3 text default $$$ $$ ); - COMMENT on TABLE t_test1 IS 'this is another comment on t_test1'; - COMMENT on COLUMN t_test1.f_serial IS 'this is another comment on f_serial'; create table t_test2 ( f_id integer NOT NULL, @@ -130,7 +128,7 @@ is( scalar @tables, 5, 'Five tables' ); my $t1 = shift @tables; is( $t1->name, 't_test1', 'Table t_test1 exists' ); -is_deeply([ $t1->comments ], [ 'comment on t_test1', 'this is another comment on t_test1' ], 'Table comment exists' ); +is( $t1->comments, 'comment on t_test1', 'Table comment exists' ); my @t1_fields = $t1->get_fields; is( scalar @t1_fields, 21, '21 fields in t_test1' ); @@ -142,7 +140,7 @@ is( $f1->is_nullable, 0, 'Field cannot be null' ); is( $f1->size, 11, 'Size is "11"' ); is( $f1->default_value, '0', 'Default value is "0"' ); is( $f1->is_primary_key, 1, 'Field is PK' ); -is_deeply( [ $f1->comments ], [ 'this is the primary key', 'this is another comment on f_serial' ], 'Comment' ); +is( $f1->comments, 'this is the primary key', 'Comment' ); is( $f1->is_auto_increment, 1, 'Field is auto increment' ); my $f2 = shift @t1_fields; From 3b8fb2cf0a6f08385f3ed98b6fabb4c653a60174 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Fri, 8 Jul 2022 00:19:05 +0300 Subject: [PATCH 4/5] get this right and fix all the tests! --- lib/SQL/Translator/Parser/PostgreSQL.pm | 1 + lib/SQL/Translator/Producer/PostgreSQL.pm | 3 ++- t/46xml-to-pg.t | 2 +- t/47postgres-producer.t | 17 +++++++---------- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/SQL/Translator/Parser/PostgreSQL.pm b/lib/SQL/Translator/Parser/PostgreSQL.pm index 37f657996..f6ef08b0a 100644 --- a/lib/SQL/Translator/Parser/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/PostgreSQL.pm @@ -366,6 +366,7 @@ column_name : NAME '.' NAME comment_phrase : /null/i { $return = 'NULL' } | SQSTRING + | DOLLARSTRING field : field_comment(s?) field_name data_type field_meta(s?) field_comment(s?) { diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index ef5292bec..cf0669cae 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -296,7 +296,7 @@ sub create_table next unless $field_comments; my $field_name_qt = $generator->quote($field->name); my $comment_ddl = - "COMMENT ON COLUMN $table_name_qt.$field_name_qt IS \$comment\$$field_comments\$comment\$"; + "COMMENT on COLUMN $table_name_qt.$field_name_qt IS \$comment\$$field_comments\$comment\$"; push @comment_statements, $comment_ddl; } @@ -348,6 +348,7 @@ sub create_table $create_statement .= join(";\n", '', map{ drop_geometry_column($_, $options) } @geometry_columns) if $options->{add_drop_table}; $create_statement .= join(";\n", '', map{ add_geometry_column($_, $options) } @geometry_columns); } + if (@comment_statements) { $create_statement .= join(";\n", '', @comment_statements); } diff --git a/t/46xml-to-pg.t b/t/46xml-to-pg.t index d9dd3277b..7de0bfe2f 100644 --- a/t/46xml-to-pg.t +++ b/t/46xml-to-pg.t @@ -42,7 +42,6 @@ CREATE TABLE "Basic" ( "email" character varying(500), "explicitnulldef" character varying, "explicitemptystring" character varying DEFAULT '', - -- Hello emptytagdef "emptytagdef" character varying DEFAULT '', "another_id" integer DEFAULT 2, "timest" timestamp, @@ -51,6 +50,7 @@ CREATE TABLE "Basic" ( CONSTRAINT "very_long_index_name_on_title_field_which_should_be_truncated_for_various_rdbms" UNIQUE ("title") ); CREATE INDEX "titleindex" on "Basic" ("title"); +COMMENT on COLUMN "Basic"."emptytagdef" IS \$comment\$Hello emptytagdef\$comment\$; DROP TABLE "Another" CASCADE; CREATE TABLE "Another" ( diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index 9c50db736..59eb60176 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -45,18 +45,15 @@ my $PRODUCER = \&SQL::Translator::Producer::PostgreSQL::create_field; -- -- Table: foo.bar -- - --- Comments: --- multi --- line --- single line --- CREATE TABLE "foo"."bar" ( - -- multi - -- line - -- single line "baz" character varying(10) DEFAULT 'quux' NOT NULL -) +); +COMMENT on TABLE "foo"."bar" IS \$comment\$multi +line +single line\$comment\$; +COMMENT on COLUMN "foo"."bar"."baz" IS \$comment\$multi +line +single line\$comment\$ EOESQL $expected =~ s/\n\z//; From 24c3a43a2a44712cff3fdd4d8f00b58eeabcec8a Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Fri, 8 Jul 2022 01:34:21 +0300 Subject: [PATCH 5/5] add attach_comments option to pg parser_options, update tests to use it --- lib/SQL/Translator/Producer/PostgreSQL.pm | 76 +++++++++++++++++++---- t/46xml-to-pg.t | 1 + t/47postgres-producer.t | 3 +- t/60roundtrip.t | 6 ++ 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index cf0669cae..f77167dec 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -17,6 +17,40 @@ producer. Now handles PostGIS Geometry and Geography data types on table definitions. Does not yet support PostGIS Views. +=head2 Producer Args + +=over 4 + +=item postgres_version + +The version of postgres to generate DDL for. Turns on features only available in later versions. The following features are supported + +=over 4 + +=item IF EXISTS + +If your postgres_version is higher than 8.003 (I should hope it is by now), then the DDL +generated for dropping objects in the database will contain IF EXISTS. + +=back + +=item attach_comments + +Generates table and column comments via the COMMENT command rather than as a comment in +the DDL. You could then look it up with \dt+ or \d+ (for tables and columns respectively) +in psql. The comment is dollar quoted with $comment$ so you can include ' in it. Just to clarify: you get this + + CREATE TABLE foo ...; + COMMENT on TABLE foo IS $comment$hi there$comment$; + +instead of this + + -- comment + CREAT TABLE foo ...; + +=back + + =cut use strict; @@ -163,6 +197,7 @@ sub produce { postgres_version => $postgres_version, add_drop_table => $add_drop_table, type_defs => \%type_defs, + attach_comments => $pargs->{attach_comments} }); push @table_defs, $table_def; @@ -265,6 +300,7 @@ sub create_table my $add_drop_table = $options->{add_drop_table} || 0; my $postgres_version = $options->{postgres_version} || 0; my $type_defs = $options->{type_defs} || {}; + my $attach_comments = $options->{attach_comments}; my $table_name = $table->name or next; my $table_name_qt = $generator->quote($table_name); @@ -275,11 +311,16 @@ sub create_table my @comment_statements; if ( my $comments = $table->comments ) { - # this follows the example in the MySQL producer, where all comments are added as - # table comments, even though they could have originally been parsed as DDL comments - # quoted via $$ string so there can be 'quotes' inside the comments - my $comment_ddl = "COMMENT on TABLE $table_name_qt IS \$comment\$$comments\$comment\$"; - push @comment_statements, $comment_ddl; + if ( $attach_comments) { + # this follows the example in the MySQL producer, where all comments are added as + # table comments, even though they could have originally been parsed as DDL comments + # quoted via $$ string so there can be 'quotes' inside the comments + my $comment_ddl = "COMMENT on TABLE $table_name_qt IS \$comment\$$comments\$comment\$"; + push @comment_statements, $comment_ddl; + } elsif (!$no_comments) { + $comments =~ s/^/-- /mg; + push @comments, "-- Comments:\n$comments\n--\n"; + } } # @@ -291,13 +332,16 @@ sub create_table postgres_version => $postgres_version, type_defs => $type_defs, constraint_defs => \@constraint_defs, + attach_comments => $attach_comments }); - my $field_comments = $field->comments; - next unless $field_comments; - my $field_name_qt = $generator->quote($field->name); - my $comment_ddl = - "COMMENT on COLUMN $table_name_qt.$field_name_qt IS \$comment\$$field_comments\$comment\$"; - push @comment_statements, $comment_ddl; + if ( $attach_comments ) { + my $field_comments = $field->comments; + next unless $field_comments; + my $field_name_qt = $generator->quote($field->name); + my $comment_ddl = + "COMMENT on COLUMN $table_name_qt.$field_name_qt IS \$comment\$$field_comments\$comment\$"; + push @comment_statements, $comment_ddl; + } } @@ -410,10 +454,18 @@ sub create_view { my $constraint_defs = $options->{constraint_defs} || []; my $postgres_version = $options->{postgres_version} || 0; my $type_defs = $options->{type_defs} || {}; + my $attach_comments = $options->{attach_comments}; $field_name_scope{$table_name} ||= {}; my $field_name = $field->name; - my $field_def = $generator->quote($field_name); + + my $field_comments = ''; + if ( !$attach_comments and my $comments = $field->comments ) { + $comments =~ s/(?quote($field_name); # # Datatype diff --git a/t/46xml-to-pg.t b/t/46xml-to-pg.t index 7de0bfe2f..3c8c85fa3 100644 --- a/t/46xml-to-pg.t +++ b/t/46xml-to-pg.t @@ -23,6 +23,7 @@ $sqlt = SQL::Translator->new( no_comments => 1, show_warnings => 0, add_drop_table => 1, + producer_args => { attach_comments => 1 } ); die "Can't find test schema $xmlfile" unless -e $xmlfile; diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index 59eb60176..ee952fd3e 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -38,7 +38,8 @@ my $PRODUCER = \&SQL::Translator::Producer::PostgreSQL::create_field; is_foreign_key => 0, is_unique => 0 ); $table->add_field($field); - my ($create, $fks) = SQL::Translator::Producer::PostgreSQL::create_table($table, { quote_table_names => q{"} }); + my ($create, $fks) = SQL::Translator::Producer::PostgreSQL::create_table( + $table, { quote_table_names => q{"}, attach_comments => 1 }); is($table->name, 'foo.bar'); my $expected = < {}, parser_args => {}, }, + { + engine => 'PostgreSQL', + name => 'Postgres w/ attached comments', + producer_args => { attach_comments => 1 }, + parser_args => {}, + }, { engine => 'SQLServer', producer_args => {},