From: Rafael Kitover Date: Wed, 2 Feb 2011 14:55:27 +0000 (-0500) Subject: add quote_names connect_info option X-Git-Tag: v0.08191~99 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=2b8cc2f27d0dc881059e55dd6462bb28b7b8e414 add quote_names connect_info option --- diff --git a/Changes b/Changes index fda4f6f..a1d6ac5 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,9 @@ Revision history for DBIx::Class + * New Features / Changes + - Add quote_names connection option. When set to true automatically + sets quote_char and name_sep appropriate for your RDBMS + * Fixes - Disable mysql_auto_reconnect for MySQL - depending on the ENV it sometimes defaults to on and causes major borkage on older diff --git a/Makefile.PL b/Makefile.PL index 2a196bc..1c3fdf6 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -62,7 +62,7 @@ my $runtime_requires = { 'Class::Inspector' => '1.24', 'Config::Any' => '0.20', 'Context::Preserve' => '0.01', - 'Data::Dumper::Concise' => '1.000', + 'Data::Dumper::Concise' => '2.020', 'Data::Page' => '2.00', 'Hash::Merge' => '0.12', 'MRO::Compat' => '0.09', diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index baa9ed6..7d7a8bb 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -23,7 +23,12 @@ use namespace::clean; # default cursor class, overridable in connect_info attributes __PACKAGE__->cursor_class('DBIx::Class::Storage::DBI::Cursor'); -__PACKAGE__->mk_group_accessors('inherited' => qw/sql_maker_class sql_limit_dialect/); +__PACKAGE__->mk_group_accessors('inherited' => qw/ + sql_maker_class sql_limit_dialect sql_quote_char sql_name_sep +/); + +__PACKAGE__->sql_name_sep('.'); + __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker'); __PACKAGE__->mk_group_accessors('simple' => qw/ @@ -449,6 +454,12 @@ Sets a specific SQL::Abstract::Limit-style limit dialect, overriding the default L setting of the storage (if any). For a list of available limit dialects see L. +=item quote_names + +When true automatically sets L and L to the characters +appropriate for your particular RDBMS. This option is preferred over specifying +L directly. + =item quote_char Specifies what characters to use to quote table and column names. @@ -666,7 +677,7 @@ sub _normalize_connect_info { delete @attrs{@storage_opts} if @storage_opts; my @sql_maker_opts = grep exists $attrs{$_}, - qw/limit_dialect quote_char name_sep/; + qw/limit_dialect quote_char name_sep quote_names/; @{ $info{sql_maker_options} }{@sql_maker_opts} = delete @attrs{@sql_maker_opts} if @sql_maker_opts; @@ -998,18 +1009,39 @@ sub sql_maker { "Your storage class ($s_class) does not set sql_limit_dialect and you " . 'have not supplied an explicit limit_dialect in your connection_info. ' . 'DBIC will attempt to use the GenericSubQ dialect, which works on most ' - . 'databases but can be (and often is) painfully slow.' + . 'databases but can be (and often is) painfully slow. ' + . "Please file an RT ticket against '$s_class' ." ); 'GenericSubQ'; } ; + my ($quote_char, $name_sep); + + if ($opts{quote_names}) { + $quote_char = (delete $opts{quote_char}) || $self->sql_quote_char || do { + my $s_class = (ref $self) || $self; + carp ( + "You requested 'quote_names' but your storage class ($s_class) does " + . 'not explicitly define a default sql_quote_char and you have not ' + . 'supplied a quote_char as part of your connection_info. DBIC will ' + .q{default to the ANSI SQL standard quote '"', which works most of } + . "the time. Please file an RT ticket against '$s_class'." + ); + + '"'; # RV + }; + + $name_sep = (delete $opts{name_sep}) || $self->sql_name_sep; + } + $self->_sql_maker($sql_maker_class->new( bindtype=>'columns', array_datatypes => 1, limit_dialect => $dialect, - name_sep => '.', + ($quote_char ? (quote_char => $quote_char) : ()), + name_sep => ($name_sep || '.'), %opts, )); } diff --git a/lib/DBIx/Class/Storage/DBI/DB2.pm b/lib/DBIx/Class/Storage/DBI/DB2.pm index a5b98c3..66851d8 100644 --- a/lib/DBIx/Class/Storage/DBI/DB2.pm +++ b/lib/DBIx/Class/Storage/DBI/DB2.pm @@ -7,6 +7,7 @@ use base qw/DBIx::Class::Storage::DBI/; use mro 'c3'; __PACKAGE__->sql_limit_dialect ('RowNumberOver'); +__PACKAGE__->sql_quote_char ('"'); sub _dbh_last_insert_id { my ($self, $dbh, $source, $col) = @_; diff --git a/lib/DBIx/Class/Storage/DBI/Informix.pm b/lib/DBIx/Class/Storage/DBI/Informix.pm index 4c475d7..90587d0 100644 --- a/lib/DBIx/Class/Storage/DBI/Informix.pm +++ b/lib/DBIx/Class/Storage/DBI/Informix.pm @@ -10,6 +10,7 @@ use Context::Preserve 'preserve_context'; use namespace::clean; __PACKAGE__->sql_limit_dialect ('SkipFirst'); +__PACKAGE__->sql_quote_char ('"'); __PACKAGE__->mk_group_accessors('simple' => '__last_insert_id'); diff --git a/lib/DBIx/Class/Storage/DBI/InterBase.pm b/lib/DBIx/Class/Storage/DBI/InterBase.pm index fd21056..ab8bc70 100644 --- a/lib/DBIx/Class/Storage/DBI/InterBase.pm +++ b/lib/DBIx/Class/Storage/DBI/InterBase.pm @@ -34,6 +34,7 @@ L. # set default __PACKAGE__->_use_insert_returning (1); __PACKAGE__->sql_limit_dialect ('FirstSkip'); +__PACKAGE__->sql_quote_char ('"'); sub _sequence_fetch { my ($self, $nextval, $sequence) = @_; diff --git a/lib/DBIx/Class/Storage/DBI/MSSQL.pm b/lib/DBIx/Class/Storage/DBI/MSSQL.pm index d16d318..8f5b528 100644 --- a/lib/DBIx/Class/Storage/DBI/MSSQL.pm +++ b/lib/DBIx/Class/Storage/DBI/MSSQL.pm @@ -15,6 +15,8 @@ __PACKAGE__->mk_group_accessors(simple => qw/ __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker::MSSQL'); +__PACKAGE__->sql_quote_char([qw/[ ]/]); + sub _set_identity_insert { my ($self, $table) = @_; diff --git a/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm b/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm index 036c550..087f0c2 100644 --- a/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm +++ b/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm @@ -10,6 +10,7 @@ use DBI; my $ERR_MSG_START = __PACKAGE__ . ' failed: '; __PACKAGE__->sql_limit_dialect ('Top'); +__PACKAGE__->sql_quote_char ([qw/[ ]/]); sub insert { my $self = shift; diff --git a/lib/DBIx/Class/Storage/DBI/ODBC/DB2_400_SQL.pm b/lib/DBIx/Class/Storage/DBI/ODBC/DB2_400_SQL.pm index 29e9da9..d0eb5c9 100644 --- a/lib/DBIx/Class/Storage/DBI/ODBC/DB2_400_SQL.pm +++ b/lib/DBIx/Class/Storage/DBI/ODBC/DB2_400_SQL.pm @@ -14,6 +14,8 @@ warn 'Major advances took place in the DBIC codebase since this driver' ."\n" ; +__PACKAGE__->sql_quote_char('"'); + # FIXME # Most likely all of this code is redundant and unnecessary. We should # be able to simply use base qw/DBIx::Class::Storage::DBI::DB2/; diff --git a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm index 220eb26..20dac87 100644 --- a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm +++ b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm @@ -8,6 +8,7 @@ use Try::Tiny; use namespace::clean; __PACKAGE__->sql_limit_dialect ('RowNum'); +__PACKAGE__->sql_quote_char ('"'); =head1 NAME diff --git a/lib/DBIx/Class/Storage/DBI/Pg.pm b/lib/DBIx/Class/Storage/DBI/Pg.pm index d1319e9..0673fe8 100644 --- a/lib/DBIx/Class/Storage/DBI/Pg.pm +++ b/lib/DBIx/Class/Storage/DBI/Pg.pm @@ -14,6 +14,7 @@ use Context::Preserve 'preserve_context'; use namespace::clean; __PACKAGE__->sql_limit_dialect ('LimitOffset'); +__PACKAGE__->sql_quote_char ('"'); # Ask for a DBD::Pg with array support warn __PACKAGE__.": DBD::Pg 2.9.2 or greater is strongly recommended\n" diff --git a/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm b/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm index 728220f..38fd196 100644 --- a/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm +++ b/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm @@ -10,6 +10,7 @@ use namespace::clean; __PACKAGE__->mk_group_accessors(simple => qw/_identity/); __PACKAGE__->sql_limit_dialect ('RowNumberOver'); +__PACKAGE__->sql_quote_char ('"'); =head1 NAME diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm index 6e6e7bb..b87c63a 100644 --- a/lib/DBIx/Class/Storage/DBI/SQLite.pm +++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm @@ -8,6 +8,7 @@ use mro 'c3'; __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker::SQLite'); __PACKAGE__->sql_limit_dialect ('LimitOffset'); +__PACKAGE__->sql_quote_char ('"'); sub backup { diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm index b5f4818..2fae10f 100644 --- a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm +++ b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm @@ -17,6 +17,7 @@ use Try::Tiny; use namespace::clean; __PACKAGE__->sql_limit_dialect ('RowCountOrGenericSubQ'); +__PACKAGE__->sql_quote_char ([qw/[ ]/]); __PACKAGE__->mk_group_accessors('simple' => qw/_identity _blob_log_on_update _writer_storage _is_extra_storage diff --git a/lib/DBIx/Class/Storage/DBI/mysql.pm b/lib/DBIx/Class/Storage/DBI/mysql.pm index 8dc49ad..ef73a1a 100644 --- a/lib/DBIx/Class/Storage/DBI/mysql.pm +++ b/lib/DBIx/Class/Storage/DBI/mysql.pm @@ -11,6 +11,7 @@ use mro 'c3'; __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker::MySQL'); __PACKAGE__->sql_limit_dialect ('LimitXY'); +__PACKAGE__->sql_quote_char ('`'); sub with_deferred_fk_checks { my ($self, $sub) = @_; diff --git a/t/storage/quote_names.t b/t/storage/quote_names.t new file mode 100644 index 0000000..073d7f3 --- /dev/null +++ b/t/storage/quote_names.t @@ -0,0 +1,132 @@ +use strict; +use warnings; +use Test::More; +use Test::Exception; +use Data::Dumper::Concise; +use lib qw(t/lib); +use DBICTest; + +my %expected = ( + 'DBIx::Class::Storage::DBI' => + # no default quote_char + { name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::MSSQL' => + { quote_char => [ '[', ']' ], name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::DB2' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::Informix' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::InterBase' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::mysql' => + { quote_char => '`', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::Pg' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::ODBC::ACCESS' => + { quote_char => [ '[', ']' ], name_sep => '.' }, + +# Not testing this one, it's a pain. +# 'DBIx::Class::Storage::DBI::ODBC::DB2_400_SQL' => +# { quote_char => '"', name_sep => qr/must be connected/ }, + + 'DBIx::Class::Storage::DBI::Oracle::Generic' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::SQLAnywhere' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::SQLite' => + { quote_char => '"', name_sep => '.' }, + + 'DBIx::Class::Storage::DBI::Sybase::ASE' => + { quote_char => [ '[', ']' ], name_sep => '.' }, +); + +while (my ($class, $mapping) = each %expected) { + my ($quote_char, $name_sep) = @$mapping{qw/quote_char name_sep/}; + eval "require ${class};"; + die $@ if $@; + my $instance = $class->new; + + my $quote_char_text = dumper($quote_char); + + if (exists $mapping->{quote_char}) { + is_deeply $instance->sql_quote_char, $quote_char, + "sql_quote_char for $class is $quote_char_text"; + } + + is $instance->sql_name_sep, $name_sep, + "sql_name_sep for $class is '$name_sep'"; +} + +# Try quote_names with available DBs. + +# SQLite first. + +my $schema = DBICTest->init_schema(quote_names => 1); + +is $schema->storage->sql_maker->quote_char, '"', + q{quote_names => 1 sets correct quote_char for SQLite ('"')}; + +is $schema->storage->sql_maker->name_sep, '.', + q{quote_names => 1 sets correct name_sep for SQLite (".")}; + +# Now the others. + +# Env var to base class mapping, these are the DBs I actually have. +# -- Caelum +my %dbs = ( + ORA => 'DBIx::Class::Storage::DBI::Oracle::Generic', + PG => 'DBIx::Class::Storage::DBI::Pg', + MYSQL => 'DBIx::Class::Storage::DBI::mysql', + DB2 => 'DBIx::Class::Storage::DBI::DB2', + SYBASE => 'DBIx::Class::Storage::DBI::Sybase::ASE', + SQLANYWHERE => 'DBIx::Class::Storage::DBI::SQLAnywhere', + SQLANYWHERE_ODBC => 'DBIx::Class::Storage::DBI::SQLAnywhere', + FIREBIRD => 'DBIx::Class::Storage::DBI::InterBase', + FIREBIRD_ODBC => 'DBIx::Class::Storage::DBI::InterBase', + INFORMIX => 'DBIx::Class::Storage::DBI::Informix', + MSSQL_ODBC => 'DBIx::Class::Storage::DBI::MSSQL', +); + +while (my ($db, $base_class) = each %dbs) { + my ($dsn, $user, $pass) = map $ENV{"DBICTEST_${db}_$_"}, qw/DSN USER PASS/; + + next unless $dsn; + + my $schema = DBICTest::Schema->connect($dsn, $user, $pass, { + quote_names => 1 + }); + + my $expected_quote_char = $expected{$base_class}{quote_char}; + my $quote_char_text = dumper($expected_quote_char); + + is_deeply $schema->storage->sql_maker->quote_char, + $expected_quote_char, + "$db quote_char with quote_names => 1 is $quote_char_text"; + + my $expected_name_sep = $expected{$base_class}{name_sep}; + + is $schema->storage->sql_maker->name_sep, + $expected_name_sep, + "$db name_sep with quote_names => 1 is '$expected_name_sep'"; +} + +done_testing; + +sub dumper { + my $val = shift; + + my $dd = DumperObject; + $dd->Indent(0); + return $dd->Values([ $val ])->Dump; +} + +1;