From f15486426dab81a41c53275075754f2fae13457d Mon Sep 17 00:00:00 2001 From: K Date: Thu, 18 May 2017 19:37:59 +0900 Subject: [PATCH 1/5] Fixed issue `ERROR: type "serial" does not exist` Sould not use "ALTER TABLE ... ALTER COLUMN ... TYPE (big|small)?serial". See https://www.postgresql.org/docs/current/static/datatype-numeric.html#DATATYPE-SERIAL --- lib/SQL/Translator/Producer/PostgreSQL.pm | 34 +++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 455f745df..43a61b885 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -768,14 +768,32 @@ sub alter_field my $from_dt = convert_datatype($from_field); my $to_dt = convert_datatype($to_field); - push @out, sprintf('ALTER TABLE %s ALTER COLUMN %s TYPE %s', - map($generator->quote($_), - $to_field->table->name, - $to_field->name - ), - $to_dt, - ) - if($to_dt ne $from_dt); + + if ($to_dt ne $from_dt) { + if ($to_dt =~ /(big|small)?serial/) { + my $table_name = $to_field->table->name; + my $field_name = $to_field->name; + + push @out, sprintf('ALTER TABLE %s ALTER COLUMN %s TYPE %s', + $to_field->table->name, + $to_field->name, + $to_dt eq 'serial' ? 'integer' : "$1int"); + + push @out, <<"__SERIAL__"; +CREATE SEQUENCE ${table_name}_${field_name}_seq; + +ALTER TABLE ${table_name} ALTER COLUMN ${field_name} SET DEFAULT nextval('${table_name}_${field_name}_seq'); + +ALTER SEQUENCE ${table_name}_${field_name}_seq OWNED BY ${table_name}.${field_name}; + +__SERIAL__ + } else { + push @out, sprintf('ALTER TABLE %s ALTER COLUMN %s TYPE %s', + $to_field->table->name, + $to_field->name, + $to_dt); + } + } my $old_default = $from_field->default_value; my $new_default = $to_field->default_value; From 62aa2af64b2c6e3923d3941a039929020e75e2cb Mon Sep 17 00:00:00 2001 From: K Date: Thu, 18 May 2017 21:49:59 +0900 Subject: [PATCH 2/5] Modified tests for postgres --- lib/SQL/Translator/Parser/PostgreSQL.pm | 9 ++++ lib/SQL/Translator/Producer/PostgreSQL.pm | 57 ++++++++++++----------- t/30sqlt-new-diff-pgsql.t | 16 ++++++- 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/lib/SQL/Translator/Parser/PostgreSQL.pm b/lib/SQL/Translator/Parser/PostgreSQL.pm index ad4ec64cb..d6d8c567e 100644 --- a/lib/SQL/Translator/Parser/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/PostgreSQL.pm @@ -589,6 +589,15 @@ pg_data_type : }; } | + /(smallserial|serial2)/i + { + $return = { + type => 'integer', + size => 5, + is_auto_increment => 1 + }; + } + | /(bit varying|varbit)/i { $return = { type => 'varbit' }; diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 43a61b885..f0ab6a874 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -651,10 +651,11 @@ sub convert_datatype $data_type = 'character varying'; } elsif ( $field->is_auto_increment ) { - if ( (defined $size[0] && $size[0] > 11) or $data_type eq 'bigint' ) { + if (11 < $size[0] || $data_type eq 'bigint') { $data_type = 'bigserial'; - } - else { + } elsif (0 < $size[0] && $size[0] < 5 || $data_type eq 'smallint') { + $data_type = 'smallserial'; + } else { $data_type = 'serial'; } undef @size; @@ -672,19 +673,10 @@ sub convert_datatype } if ( $data_type eq 'integer' ) { - if ( defined $size[0] && $size[0] > 0) { - if ( $size[0] > 10 ) { - $data_type = 'bigint'; - } - elsif ( $size[0] < 5 ) { - $data_type = 'smallint'; - } - else { - $data_type = 'integer'; - } - } - else { - $data_type = 'integer'; + if (10 < $size[0]) { + $data_type = 'bigint'; + } elsif (0 < $size[0] && $size[0] <= 5) { + $data_type = 'smallint'; } } @@ -775,22 +767,35 @@ sub alter_field my $field_name = $to_field->name; push @out, sprintf('ALTER TABLE %s ALTER COLUMN %s TYPE %s', + map($generator->quote($_), $to_field->table->name, - $to_field->name, - $to_dt eq 'serial' ? 'integer' : "$1int"); - - push @out, <<"__SERIAL__"; -CREATE SEQUENCE ${table_name}_${field_name}_seq; - -ALTER TABLE ${table_name} ALTER COLUMN ${field_name} SET DEFAULT nextval('${table_name}_${field_name}_seq'); - -ALTER SEQUENCE ${table_name}_${field_name}_seq OWNED BY ${table_name}.${field_name}; + $to_field->name), + $to_dt eq 'serial' ? 'integer' : "$1int" + ); + + my $seq_name = "${table_name}_${field_name}_seq"; + my $by_name = "${table_name}.${field_name}"; + my @args = map( + $generator->quote($_), + $seq_name, + $table_name, + $field_name, + $seq_name, + $by_name + ); + push @out, sprintf(<<"__SERIAL__", @args); +CREATE SEQUENCE %s; + +ALTER TABLE %s ALTER COLUMN %s SET DEFAULT nextval('${table_name}_${field_name}_seq'); + +ALTER SEQUENCE %s OWNED BY %s; __SERIAL__ } else { push @out, sprintf('ALTER TABLE %s ALTER COLUMN %s TYPE %s', + map($generator->quote($_), $to_field->table->name, - $to_field->name, + $to_field->name), $to_dt); } } diff --git a/t/30sqlt-new-diff-pgsql.t b/t/30sqlt-new-diff-pgsql.t index 5bc782038..c29dce196 100644 --- a/t/30sqlt-new-diff-pgsql.t +++ b/t/30sqlt-new-diff-pgsql.t @@ -72,7 +72,13 @@ DROP INDEX "u_name"; ALTER TABLE "person" ADD COLUMN "is_rock_star" smallint DEFAULT 1; -ALTER TABLE "person" ALTER COLUMN "person_id" TYPE serial; +ALTER TABLE "person" ALTER COLUMN "person_id" TYPE integer; + +CREATE SEQUENCE "person_person_id_seq"; + +ALTER TABLE "person" ALTER COLUMN "person_id" SET DEFAULT nextval('person_person_id_seq'); + +ALTER SEQUENCE "person_person_id_seq" OWNED BY "person"."person_id"; ALTER TABLE "person" ALTER COLUMN "name" SET NOT NULL; @@ -127,7 +133,13 @@ ALTER TABLE person DROP CONSTRAINT UC_age_name; ALTER TABLE person ADD COLUMN is_rock_star smallint DEFAULT 1; -ALTER TABLE person ALTER COLUMN person_id TYPE serial; +ALTER TABLE person ALTER COLUMN person_id TYPE integer; + +CREATE SEQUENCE person_person_id_seq; + +ALTER TABLE person ALTER COLUMN person_id SET DEFAULT nextval('person_person_id_seq'); + +ALTER SEQUENCE person_person_id_seq OWNED BY person.person_id; ALTER TABLE person ALTER COLUMN name SET NOT NULL; From 6a6cfc5fee7d4cea81469168d08a3ff5c1885db3 Mon Sep 17 00:00:00 2001 From: K Date: Thu, 18 May 2017 22:06:20 +0900 Subject: [PATCH 3/5] Modified producer to generate time or timestamp --- lib/SQL/Translator/Producer/PostgreSQL.pm | 16 +++++++++++++--- t/47postgres-producer.t | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index f0ab6a874..15f940cbf 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -689,12 +689,22 @@ sub convert_datatype @size = (); } - if (defined $size[0] && $size[0] > 0 && $data_type =~ /^time/i ) { - $data_type =~ s/^(time.*?)( with.*)?$/$1($size[0])/; - $data_type .= $2 if(defined $2); + if ( $data_type =~ /^(time(?:[a-z]+)?)/i ) { + my $time_type = $1; + if (defined $size[0] && 0 < $size[0]) { + $time_type .= "($size[0])"; + } + if ($data_type =~ /((?:with|without)? time zone)$/i) { + $time_type .= " $1"; + } else { + $time_type .= ' without time zone'; + } + $data_type = $time_type; } elsif ( defined $size[0] && $size[0] > 0 ) { + $data_type =~ s/\([0-9]+\)$//; # Bug fix for type(size)(size) $data_type .= '(' . join( ',', @size ) . ')'; } + if($array) { $data_type .= '[]'; diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index 9c50db736..dd03d048f 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -292,7 +292,7 @@ my $field3 = SQL::Translator::Schema::Field->new( name => 'time_field', my $field3_sql = SQL::Translator::Producer::PostgreSQL::create_field($field3); -is($field3_sql, 'time_field time NOT NULL', 'Create time field works'); +is($field3_sql, 'time_field time without time zone NOT NULL', 'Create time field works'); my $field3_datetime_with_TZ = SQL::Translator::Schema::Field->new( name => 'datetime_with_TZ', @@ -496,7 +496,7 @@ my $field12 = SQL::Translator::Schema::Field->new( name => 'time_field', my $field12_sql = SQL::Translator::Producer::PostgreSQL::create_field($field12,{ postgres_version => 8.3 }); -is($field12_sql, 'time_field timestamp NOT NULL', 'time with precision'); +is($field12_sql, 'time_field timestamp without time zone NOT NULL', 'time with precision'); my $field13 = SQL::Translator::Schema::Field->new( name => 'enum_field_with_type_name', table => $table, From 31bd66c48714b9acc4773d54808fc7f07617d14a Mon Sep 17 00:00:00 2001 From: K Date: Thu, 18 May 2017 22:13:27 +0900 Subject: [PATCH 4/5] Fixed to parse from DBI using PostgreSQL For example, I got result 259 when varchar size is 255. So expected 255. I also fixed the problem that unexpected types will be returned. --- lib/SQL/Translator/Parser/DBI/PostgreSQL.pm | 26 +++++++++++++++++---- t/66-postgres-dbi-parser.t | 5 ++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm b/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm index 28017a6b9..59f401209 100644 --- a/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm @@ -37,14 +37,32 @@ sub parse { my $schema = $tr->schema; my $column_select = $dbh->prepare( - "SELECT a.attname, format_type(t.oid, a.atttypmod) as typname, a.attnum, - a.atttypmod as length, a.attnotnull, a.atthasdef, ad.adsrc, + "SELECT a.attname, case + when t.oid = any ('{int,int8,int2}'::regtype[]) + and ad.adsrc = 'nextval(''' + || (pg_get_serial_sequence (a.attrelid::regclass::text + , a.attname))::regclass + || '''::regclass)' + then case t.oid + when 'int8'::regtype then 'bigserial' + when 'int'::regtype then 'serial' + when 'int2'::regtype then 'smallserial' + end + else format_type(t.oid, a.atttypmod) + end as typname, a.attnum, + case typname + when 'varchar' then a.atttypmod - 4 + when 'bpchar' then a.atttypmod - 4 + when 'numeric' then (a.atttypmod - 4) / 65536 + when 'decimal' then (a.atttypmod - 4) / 65536 + else a.atttypmod + end as length, a.attnotnull, a.atthasdef, ad.adsrc, d.description FROM pg_type t, pg_attribute a LEFT JOIN pg_attrdef ad ON (ad.adrelid = a.attrelid AND a.attnum = ad.adnum) LEFT JOIN pg_description d ON (a.attrelid=d.objoid AND a.attnum=d.objsubid) - WHERE a.attrelid=? AND attnum>0 - AND a.atttypid=t.oid + WHERE a.attrelid = ? AND attnum > 0 + AND a.atttypid = t.oid ORDER BY a.attnum" ); diff --git a/t/66-postgres-dbi-parser.t b/t/66-postgres-dbi-parser.t index aae516227..39219e1ad 100644 --- a/t/66-postgres-dbi-parser.t +++ b/t/66-postgres-dbi-parser.t @@ -91,7 +91,7 @@ is( scalar @t1_fields, 4, '4 fields in sqlt_test1' ); my $f1 = shift @t1_fields; is( $f1->name, 'f_serial', 'First field is "f_serial"' ); -is( $f1->data_type, 'integer', 'Field is an integer' ); +is( $f1->data_type, 'serial', 'Field is a serial' ); is( $f1->is_nullable, 0, 'Field cannot be null' ); is( $f1->default_value, "nextval('sqlt_test1_f_serial_seq'::regclass)", 'Default value is nextval()' ); is( $f1->is_primary_key, 1, 'Field is PK' ); @@ -102,8 +102,7 @@ my $f2 = shift @t1_fields; is( $f2->name, 'f_varchar', 'Second field is "f_varchar"' ); is( $f2->data_type, 'character varying(255)', 'Field is a character varying(255)' ); is( $f2->is_nullable, 1, 'Field can be null' ); -#FIXME: should not be 255? -is( $f2->size, 259, 'Size is "259"' ); +is( $f2->size, 255, 'Size is "255"' ); is( $f2->default_value, undef, 'Default value is undefined' ); is( $f2->is_primary_key, 0, 'Field is not PK' ); is( $f2->is_auto_increment, 0, 'Field is not auto increment' ); From 5ad1563af2a6921c950fc5ee7d48ab7e59d78ec9 Mon Sep 17 00:00:00 2001 From: K Date: Fri, 19 May 2017 00:08:07 +0900 Subject: [PATCH 5/5] Fixed some test --- lib/SQL/Translator/Parser/DBI/PostgreSQL.pm | 7 +++---- lib/SQL/Translator/Producer/PostgreSQL.pm | 2 -- t/46xml-to-pg.t | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm b/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm index 59f401209..f878af5d5 100644 --- a/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/DBI/PostgreSQL.pm @@ -37,17 +37,16 @@ sub parse { my $schema = $tr->schema; my $column_select = $dbh->prepare( - "SELECT a.attname, case + "SELECT a.attname, case when t.oid = any ('{int,int8,int2}'::regtype[]) and ad.adsrc = 'nextval(''' - || (pg_get_serial_sequence (a.attrelid::regclass::text - , a.attname))::regclass + || (pg_get_serial_sequence (a.attrelid::regclass::text, a.attname))::regclass || '''::regclass)' then case t.oid when 'int8'::regtype then 'bigserial' when 'int'::regtype then 'serial' when 'int2'::regtype then 'smallserial' - end + end else format_type(t.oid, a.atttypmod) end as typname, a.attnum, case typname diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 15f940cbf..c29307cea 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -704,7 +704,6 @@ sub convert_datatype $data_type =~ s/\([0-9]+\)$//; # Bug fix for type(size)(size) $data_type .= '(' . join( ',', @size ) . ')'; } - if($array) { $data_type .= '[]'; @@ -782,7 +781,6 @@ sub alter_field $to_field->name), $to_dt eq 'serial' ? 'integer' : "$1int" ); - my $seq_name = "${table_name}_${field_name}_seq"; my $by_name = "${table_name}.${field_name}"; my @args = map( diff --git a/t/46xml-to-pg.t b/t/46xml-to-pg.t index d9dd3277b..b02ff368e 100644 --- a/t/46xml-to-pg.t +++ b/t/46xml-to-pg.t @@ -45,7 +45,7 @@ CREATE TABLE "Basic" ( -- Hello emptytagdef "emptytagdef" character varying DEFAULT '', "another_id" integer DEFAULT 2, - "timest" timestamp, + "timest" timestamp without time zone, PRIMARY KEY ("id"), CONSTRAINT "emailuniqueindex" UNIQUE ("email"), CONSTRAINT "very_long_index_name_on_title_field_which_should_be_truncated_for_various_rdbms" UNIQUE ("title")