Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect serial instead of bigserial in PG #72

Closed
wants to merge 2 commits into from

Conversation

abeverley
Copy link
Contributor

When generating PostgreSQL, auto-incrementing bigint columns are incorrectly converted to serial types. Instead they should be converted to bigserial.

When generating PostgreSQL, auto-incrementing bigint columns are
incorrectly converted to serial types. Instead they should be
converted to bigserial.
@jazd
Copy link

jazd commented Oct 1, 2015

👍

@ribasushi
Copy link
Contributor

@abeverley Is it possible to augment one of the PG tests to check this particular case?

Thanks!

@abeverley
Copy link
Contributor Author

On Tue, 2015-11-03 at 03:30 -0800, Peter Rabbitson wrote:

Is it possible to augment one of the PG tests to check this particular
case?

@ribasushi done. I noticed the test suite is failing elsewhere within
that PG test file, but I don't think that is anything to do with my
PR...

@ribasushi
Copy link
Contributor

@abeverley That's not right... Nothing seems to fail for me here with or without your changes. Can you paste the failure?

@abeverley
Copy link
Contributor Author

Hmmm, it's quite possible that my hacked environment is causing the problem (I'm pretty sure I didn't get this problem when submitting the pull requests previously). FWIW, here is the failure:

abeverley@andy-laptop:~/git/sql-translator$ prove  t/47postgres-producer.t 
t/47postgres-producer.t .. 1/? 
#   Failed test at t/47postgres-producer.t line 63.
#          got: '--
# -- Table: foo.bar
# --
# 
# -- Comments: 
# -- multi
# line
# -- single line
# --
# CREATE TABLE "foo"."bar" (
#   -- multi
# line
# single line
#   "baz" character varying(10) DEFAULT 'quux' NOT NULL
# )'
#     expected: '--
# -- Table: foo.bar
# --
# 
# -- Comments:
# -- multi
# -- line
# -- single line
# --
# CREATE TABLE "foo"."bar" (
#   -- multi
#   -- line
#   -- single line
#   "baz" character varying(10) DEFAULT 'quux' NOT NULL
# )'
# Looks like you failed 1 test of 64.

@ribasushi
Copy link
Contributor

Please install this guy, run the following and get me the output (there will be a lot of it):
$ perl -Ilib -MModule::Versions::Report t/47postgres-producer.t

@ribasushi
Copy link
Contributor

@abeverley Meanwhile applied as 9d430e0.

I am closing the PR itself, but please do get me that diagnostic output when you have a chance.

@ribasushi ribasushi closed this Nov 5, 2015
@abeverley
Copy link
Contributor Author

Ah, my mistake? prove -Ilib t/47postgres-producer.t passes all tests, but prove t/47postgres-producer.t doesn't. I remember I had to do that before. I guess I have another old version of a module kicking around somewhere.

Anyway, output requested:

ok 1
ok 2
ok 3 - Create field works
ok 4 - Create field works
ok 5 - precheck of create_Primary Key constraint
ok 6 - Create Primary Key Constraint works
ok 7 - Alter drop Primary Key constraint works
ok 8 - precheck of create_Foreign Key constraint
ok 9 - Create Foreign Key Constraint works
ok 10 - Alter drop Foreign Key constraint works
ok 11 - precheck of create_Foreign Key constraint
ok 12 - Create un-named Foreign Key Constraint works
ok 13 - Alter drop un-named Foreign Key constraint works
ok 14 - Create Primary Key Constraint works
ok 15 - Alter drop Primary Key constraint works
ok 16 - Create un-named Primary Key Constraint works
ok 17 - Alter drop un-named Foreign Key constraint works
ok 18 - Alter field works
ok 19 - Complex Alter field works
ok 20 - Add field works
ok 21 - Drop field works
ok 22 - Create serial field works
ok 23 - Create bigserial field works (from bigint type)
ok 24 - Create bigserial field works (based on size)
ok 25 - Create time field works
ok 26 - Create time field with time zone and size, works
ok 27 - Create time field without time zone but with size, works
ok 28 - Create numeric field works
ok 29 - Create bytea field works
ok 30 - Create real enum field works
ok 31 - Create real enum type works
ok 32 - DROP DEFAULT
ok 33 - DEFAULT with escaping
ok 34 - DEFAULT unescaped if scalarref
ok 35 - DROP NOT NULL
ok 36 - timestamp with precision
ok 37 - time with precision
ok 38 - time with precision
ok 39 - time with precision
ok 40 - time with precision
ok 41 - Create real enum field works
ok 42 - Create real enum type works
ok 43 - default str
ok 44 - default null
ok 45 - default null from special cased string
ok 46 - unquoted default from scalar ref
ok 47 - correct "CREATE OR REPLACE VIEW" SQL
ok 48 - correct "CREATE OR REPLACE VIEW" SQL 2
ok 49 - index created
ok 50 - index created w/ quotes
ok 51 - index created
ok 52 - index created w/ quotes
ok 53 - index created
ok 54 - index created w/ quotes
ok 55 - constraint created
ok 56 - constraint created w/ quotes
ok 57 - constraint created
ok 58 - constraint created w/ quotes
ok 59 - constraint created
ok 60 - constraint created w/ quotes
ok 61 - index using & where created
ok 62 - index using & where created w/ quotes
ok 63 - My DROP VIEW statement for 8.1 is correct
ok 64 - My DROP VIEW statement for 9.1 is correct
1..64


Perl v5.20.2 under linux 
 Modules in memory:
  Algorithm;
  Algorithm::Diff v1.1902;
  Algorithm::Diff::_impl;
  Algorithm::Diff::_obj;
  B v1.48;
  B::AV;
  B::BINOP;
  B::BM;
  B::COP;
  B::CV;
  B::Deparse;
  B::FM;
  B::GV;
  B::HE;
  B::HV;
  B::INVLIST;
  B::IO;
  B::IV;
  B::LISTOP;
  B::LOGOP;
  B::LOOP;
  B::MAGIC;
  B::NULL;
  B::NV;
  B::OBJECT;
  B::OP;
  B::PADLIST;
  B::PADOP;
  B::PMOP;
  B::PV;
  B::PVIV;
  B::PVLV;
  B::PVMG;
  B::PVNV;
  B::PVOP;
  B::REGEXP;
  B::RHE;
  B::RV;
  B::Section;
  B::SPECIAL;
  B::SV;
  B::SVOP;
  B::UNOP;
  bareword;
  base v2.22;
  bytes v1.04;
  Carp v1.3301;
  Carp::Clan v6.04;
  Class;
  Class::Method;
  Class::Method::Modifiers v2.11;
  Class::MOP;
  Class::Struct v0.65;
  Class::Struct::Tie_ISA;
  Class::XSAccessor v1.19;
  Class::XSAccessor::Array;
  Class::XSAccessor::Heavy v1.19;
  Config v5.020002;
  constant v1.31;
  CORE;
  CORE::GLOBAL;
  Cwd v3.48_01;
  Data;
  Data::Dumper v2.151_01;
  DB;
  DBD;
  DBD::_;
  DBD::_::common;
  DBD::_::db;
  DBD::_::dr;
  DBD::_::st;
  DBD::_mem;
  DBD::_mem::common;
  DBD::_mem::db;
  DBD::_mem::dr;
  DBD::_mem::st;
  DBD::Switch;
  DBD::Switch::db;
  DBD::Switch::db_mem;
  DBD::Switch::dr;
  DBD::Switch::dr_mem;
  DBD::Switch::st;
  DBD::Switch::st_mem;
  DBI v1.631;
  DBI::common;
  DBI::db;
  DBI::dr;
  DBI::PurePerl;
  DBI::st;
  DBI::var;
  Devel;
  Devel::GlobalDestruction v0.13;
  Devel::GlobalDestruction::XS;
  Digest;
  Digest::base v1.16;
  Digest::SHA v5.88;
  Does;
  Does::Not;
  Does::Not::Exist;
  Dos;
  DynaLoader v1.25;
  EPOC;
  Exporter v5.71;
  Exporter::Heavy v5.71;
  ExtUtils;
  ExtUtils::Constant;
  ExtUtils::Constant::ProxySubs;
  Fcntl v1.11;
  fields;
  File;
  File::Basename v2.85;
  File::Find v1.27;
  File::Spec v3.48_01;
  File::Spec::Functions v3.48_01;
  File::Spec::Unix v3.48_01;
  File::stat v1.07;
  FindBin v1.51;
  Graph;
  Graph::Directed;
  Import;
  import;
  Import::Into v1.002004;
  integer v1.01;
  Internals;
  IO v1.31;
  IO::Dir v1.1;
  IO::File v1.16;
  IO::Handle v1.35;
  IO::Poll;
  IO::Seekable v1.1;
  IO::Socket;
  isn;
  List;
  List::Util v1.42;
  List::Util::_Pair;
  MacPerl;
  maybe;
  maybe::next;
  Method;
  Method::Generate;
  Method::Generate::Accessor;
  Method::Generate::BuildAll;
  Method::Generate::Constructor;
  Method::Generate::DemolishAll;
  MIME;
  MIME::Base64;
  Module;
  Module::Runtime v0.014;
  Module::Versions;
  Module::Versions::Report v1.06;
  Moo v2.000001;
  Moo::_mro;
  Moo::_strictures;
  Moo::_Utils;
  Moo::HandleMoose;
  Moo::HandleMoose::_TypeMap;
  Moo::HandleMoose::AuthorityHack;
  Moo::HandleMoose::FakeMetaClass;
  Moo::Object;
  Moo::Role v2.000001;
  Moo::sification;
  Moose;
  Mouse;
  Mouse::Util;
  MRO;
  mro v1.16;
  MRO::Compat;
  next;
  overload v1.22;
  overload::numbers;
  overloading v0.02;
  Package;
  Package::Variant v1.002002;
  Parse;
  Parse::RecDescent;
  PerlIO v1.09;
  PerlIO::Layer;
  re;
  Regexp;
  Role;
  Role::Tiny v2.000001;
  Role::Tiny::__GUARD__;
  Scalar;
  Scalar::Util v1.42;
  SelectSaver v1.02;
  SQL;
  SQL::Translator v0.11021;
  SQL::Translator::Generator;
  SQL::Translator::Generator::DDL;
  SQL::Translator::Generator::DDL::PostgreSQL;
  SQL::Translator::Generator::Role;
  SQL::Translator::Generator::Role::Quote;
  SQL::Translator::Parser;
  SQL::Translator::Parser::DB2;
  SQL::Translator::Parser::DB2::Grammar;
  SQL::Translator::Producer v1.59;
  SQL::Translator::Producer::PostgreSQL v1.59;
  SQL::Translator::Role;
  SQL::Translator::Role::BuildArgs;
  SQL::Translator::Role::Debug;
  SQL::Translator::Role::Error;
  SQL::Translator::Role::ListAttr;
  SQL::Translator::Role::ListAttr::_Variant_A001;
  SQL::Translator::Role::ListAttr::_Variant_A002;
  SQL::Translator::Role::ListAttr::_Variant_A003;
  SQL::Translator::Role::ListAttr::_Variant_A004;
  SQL::Translator::Role::ListAttr::_Variant_A005;
  SQL::Translator::Role::ListAttr::_Variant_A006;
  SQL::Translator::Role::ListAttr::_Variant_A007;
  SQL::Translator::Role::ListAttr::_Variant_A008;
  SQL::Translator::Role::ListAttr::_Variant_A009;
  SQL::Translator::Role::ListAttr::_Variant_A010;
  SQL::Translator::Schema v1.59;
  SQL::Translator::Schema::Constants v1.59;
  SQL::Translator::Schema::Constraint v1.59;
  SQL::Translator::Schema::Field v1.59;
  SQL::Translator::Schema::Index v1.59;
  SQL::Translator::Schema::Object v1.59;
  SQL::Translator::Schema::Procedure v1.59;
  SQL::Translator::Schema::Role;
  SQL::Translator::Schema::Role::Compare;
  SQL::Translator::Schema::Role::Extra;
  SQL::Translator::Schema::Table v1.59;
  SQL::Translator::Schema::Trigger v1.59;
  SQL::Translator::Schema::View v1.59;
  SQL::Translator::Types;
  SQL::Translator::Utils v1.59;
  SQL::Translator::Utils::Error;
  strict v1.08;
  strictures v1.005005;
  Sub;
  Sub::Defer v2.000001;
  Sub::Exporter;
  Sub::Exporter::Progressive v0.001011;
  Sub::Name v0.12;
  Sub::Quote v2.000001;
  Sub::Uplevel v0.24;
  Sub::Util;
  Symbol v1.07;
  Test;
  Test::Builder v1.001014;
  Test::Builder::IO;
  Test::Builder::IO::Scalar;
  Test::Builder::Module v1.001014;
  Test::Differences v0.61;
  Test::Exception v0.32;
  Test::More v1.001014;
  Test::SQL;
  Test::SQL::Translator v1.59;
  Text;
  Text::Diff v1.41;
  Text::Diff::Base;
  Text::Diff::Context;
  Text::Diff::OldStyle;
  Text::Diff::Unified;
  threads;
  threads::shared;
  Tie;
  Tie::ExtraHash;
  Tie::Hash v1.05;
  Tie::StdHash;
  Try;
  Try::Tiny v0.22;
  Try::Tiny::Catch;
  Try::Tiny::ScopeGuard;
  unimport;
  UNIVERSAL;
  utf8;
  vars v1.03;
  version;
  VMS;
  VMS::Feature;
  VMS::Filespec;
  warnings v1.23;
  warnings::register v1.03;
  Win32;
  XSLoader v0.17;
[at Thu Nov  5 11:33:47 2015 (local) / Thu Nov  5 11:33:47 2015 (GMT)]

@abeverley
Copy link
Contributor Author

Thanks for merging anyway.

@ribasushi
Copy link
Contributor

I guess I have another old version of a module kicking around somewhere.

No. All -l does is the equivalent to -Ilib. Without doing so you are running the tests you wrote for changes you made in your workdir against stuff outside of your workdir. Given you just made the changes it is extremely unlikely that you will have them also installed under your usual @INC. Hence the failures ;)

@abeverley
Copy link
Contributor Author

Thanks @ribasushi, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants