From: Rafael Kitover Date: Wed, 5 Aug 2009 08:46:51 +0000 (+0000) Subject: Merge 'trunk' into 'sybase' X-Git-Tag: v0.08112~14^2~73 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=1f4263ebe497f0a4a7dbde8850a8d1736946740d;hp=-c;p=dbsrgits%2FDBIx-Class.git Merge 'trunk' into 'sybase' r9075@hlagh (orig r7205): ribasushi | 2009-08-05 02:34:25 -0400 Bump dependencies: Test::More for the new no_plan/done_testing goodies File::Temp as per RT#48431 r9077@hlagh (orig r7207): ribasushi | 2009-08-05 02:36:32 -0400 r7156@Thesaurus (orig r7153): robkinyon | 2009-07-30 20:06:04 +0200 Create prefetch_redux branch r7164@Thesaurus (orig r7161): robkinyon | 2009-07-31 22:41:01 +0200 Added MooseX::Traits to Makefile.PL r7172@Thesaurus (orig r7169): robkinyon | 2009-08-03 05:49:59 +0200 Added two tests and marked one todo_skip r7187@Thesaurus (orig r7184): ribasushi | 2009-08-03 17:24:41 +0200 Use goto to preserve correct error-at-line reporting r7189@Thesaurus (orig r7186): ribasushi | 2009-08-04 12:34:58 +0200 Add an extra test specifically for distinct/prefetch Remove duplicate test in count/prefetch Switch to as_query instead of debug overloading r7190@Thesaurus (orig r7187): ribasushi | 2009-08-04 12:35:57 +0200 Fix how a distinct-induced group_by is calculated, taking in consideration the new prefetch mechanism r7197@Thesaurus (orig r7194): ribasushi | 2009-08-04 17:31:33 +0200 Traits not needed by anything currently in dbic r7198@Thesaurus (orig r7195): ribasushi | 2009-08-04 17:41:14 +0200 Move around tests a bit r7199@Thesaurus (orig r7196): mo | 2009-08-04 21:10:57 +0200 prefetch-grouped fails, again r7204@Thesaurus (orig r7201): ribasushi | 2009-08-04 22:50:51 +0200 Split the search_related prefetch tests into a standalone testfile r7205@Thesaurus (orig r7202): ribasushi | 2009-08-04 23:05:03 +0200 Move norbi's test to prefetch_redux - it's the same idea r7209@Thesaurus (orig r7206): ribasushi | 2009-08-05 08:35:48 +0200 Tadaaaa (even more prefetch insanity) r9079@hlagh (orig r7209): ribasushi | 2009-08-05 02:38:41 -0400 r7107@Thesaurus (orig r7104): caelum | 2009-07-24 06:51:57 +0200 new branch to move common mssql functionality into the base class, and other tweaks r7109@Thesaurus (orig r7106): caelum | 2009-07-24 07:28:11 +0200 moved code to ::DBI::MSSQL and added DT inflation test r7112@Thesaurus (orig r7109): caelum | 2009-07-24 08:46:16 +0200 merge in some more MSSQL code, including odbc dynamic cursor support r7113@Thesaurus (orig r7110): caelum | 2009-07-24 08:49:54 +0200 fix a warning in SQLAHacks r7114@Thesaurus (orig r7111): caelum | 2009-07-24 09:22:33 +0200 add placeholder support detection for mssql through dbd::sybase r7118@Thesaurus (orig r7115): caelum | 2009-07-24 16:39:06 +0200 minor doc clarification r7122@Thesaurus (orig r7119): caelum | 2009-07-25 16:10:30 +0200 move placeholder support detection into ::Sybase::Base r7123@Thesaurus (orig r7120): caelum | 2009-07-25 16:12:01 +0200 add a comment r7127@Thesaurus (orig r7124): caelum | 2009-07-26 18:04:29 +0200 SAVEPOINT methods for MSSQL r7140@Thesaurus (orig r7137): caelum | 2009-07-30 10:12:45 +0200 better tests for "smalldatetime" support in MSSQL r7142@Thesaurus (orig r7139): caelum | 2009-07-30 13:29:19 +0200 MSSQL GUID support r7147@Thesaurus (orig r7144): caelum | 2009-07-30 15:38:33 +0200 update sqlite test schema r7150@Thesaurus (orig r7147): caelum | 2009-07-30 16:26:47 +0200 make sure the new mssql insert method works on an un-reblessed storage r7151@Thesaurus (orig r7148): caelum | 2009-07-30 16:55:35 +0200 better rebless check for insert r7154@Thesaurus (orig r7151): caelum | 2009-07-30 18:57:22 +0200 add missing file r7155@Thesaurus (orig r7152): caelum | 2009-07-30 19:00:40 +0200 fix syntax error r7163@Thesaurus (orig r7160): caelum | 2009-07-31 15:52:41 +0200 fix a bug in _determine_driver r7166@Thesaurus (orig r7163): caelum | 2009-08-01 18:10:23 +0200 default collist for storage _resolve_column_info r7182@Thesaurus (orig r7179): caelum | 2009-08-03 13:42:31 +0200 check that dynamic cursors are functional if enabled r7184@Thesaurus (orig r7181): ribasushi | 2009-08-03 14:23:37 +0200 Adjust expected sql to match the new 'Track' table definition r7186@Thesaurus (orig r7183): ribasushi | 2009-08-03 15:16:10 +0200 Simplify code and add some comments r7200@Thesaurus (orig r7197): caelum | 2009-08-04 21:31:16 +0200 update oracle tests for new "track" table r7203@Thesaurus (orig r7200): caelum | 2009-08-04 22:39:57 +0200 update Changes r9081@hlagh (orig r7211): ribasushi | 2009-08-05 02:40:39 -0400 r7213@Thesaurus (orig r7210): ribasushi | 2009-08-05 08:40:20 +0200 Really sanify _resolve_column_info r9083@hlagh (orig r7213): ribasushi | 2009-08-05 04:19:37 -0400 Reminder about discard_changes and friends r9084@hlagh (orig r7214): ribasushi | 2009-08-05 04:26:20 -0400 Reformat and fill-in changes r9085@hlagh (orig r7215): caelum | 2009-08-05 04:37:12 -0400 rename connect_call_use_mars to connect_call_use_MARS --- 1f4263ebe497f0a4a7dbde8850a8d1736946740d diff --combined Changes index 5ed9222,aa03d9c..b33bc29 --- a/Changes +++ b/Changes @@@ -1,24 -1,34 +1,44 @@@ Revision history for DBIx::Class - - support for MSSQL 'money' type - - support for 'smalldatetime' type used in MSSQL and Sybase for + - Replication updates: + - Improved the replication tests so that they are more reliable + and accurate, and hopefully solve some cross platform issues. + - Bugfixes related to naming particular replicants in a + 'force_pool' attribute. + - Lots of documentation updates, including a new Introduction.pod + file. + - Fixed the way we detect transaction to make this more reliable + and forward looking. + - Fixed some trouble with the way Moose Types are used. + - Refactor of MSSQL storage drivers, with some new features: + - Support for placeholders for MSSQL via DBD::Sybase with proper + autodetection + - 'uniqueidentifier' support with auto newid() + - Dynamic cursor support and other MARS options for ODBC ++ - savepoints with auto_savepoint => 1 + - Support for MSSQL 'money' type + - Support for 'smalldatetime' type used in MSSQL and Sybase for InflateColumn::DateTime -- - support for Postgres 'timestamp without timezone' type in - InflateColumn::DateTime (RT#48389) ++ - Support for Postgres 'timestamp without timezone' type in + InflateColumn::DateTime - - much improved Sybase support, including support for TEXT/IMAGE ++ - Much improved Sybase support, including support for TEXT/IMAGE + columns and connecting via FreeTDS + - Replication updates: Improved the replication tests so that they are + more reliable and accurate, and hopefully solve some cross platform + issues. Bugfixes related to naming particular replicants in a + 'force_pool' attribute. Lots of documentation updates, including a + new Introduction.pod file. Fixed the way we detect transaction to + make this more reliable and forward looking. Fixed some trouble with + the way Moose Types are used. - Added new MySQL specific on_connect_call macro 'set_strict_mode' (also known as make_mysql_not_suck_as_much) - - Added call to Pod::Inherit in Makefile.PL - - currently at author-time only, so we need to add the produced - .pod files to the MANIFEST + - Multiple prefetch-related fixes: + - Adjust overly agressive subquery join-chain pruning + - Always preserve the outer join-chain - fixes numerous + problems with search_related chaining + - Deal with the distinct => 1 attribute properly when using + prefetch + - Multiple POD improvements 0.08108 2009-07-05 23:15:00 (UTC) diff --combined Makefile.PL index da7c940,79da35f..5d05239 --- a/Makefile.PL +++ b/Makefile.PL @@@ -13,9 -13,11 +13,11 @@@ all_from 'lib/DBIx/Class.pm' test_requires 'Test::Builder' => 0.33; test_requires 'Test::Deep' => 0; test_requires 'Test::Exception' => 0; - test_requires 'Test::More' => 0.82; + test_requires 'Test::More' => 0.92; test_requires 'Test::Warn' => 0.11; + test_requires 'File::Temp' => 0.22; + # Core requires 'List::Util' => 0; requires 'Scalar::Util' => 0; @@@ -111,12 -113,6 +113,12 @@@ my %force_requires_if_author = 'DateTime::Format::Oracle' => 0, ) : () , + + $ENV{DBICTEST_SYBASE_DSN} + ? ( + 'DateTime::Format::Sybase' => 0, + ) : () + , ); if ($Module::Install::AUTHOR) { diff --combined lib/DBIx/Class/ResultSet.pm index 21bc62d,e24dafe..e42e799 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@@ -1264,7 -1264,7 +1264,7 @@@ sub _count_subq_rs my $sub_attrs = { %$attrs }; # extra selectors do not go in the subquery and there is no point of ordering it - delete $sub_attrs->{$_} for qw/collapse prefetch_select select as order_by/; + delete $sub_attrs->{$_} for qw/collapse select _prefetch_select as order_by/; # if we prefetch, we group_by primary keys only as this is what we would get out of the rs via ->next/->all # clobber old group_by regardless @@@ -2780,14 -2780,7 +2780,14 @@@ sub _resolved_attrs : "${alias}.$_" ) } - } ( ref($attrs->{columns}) eq 'ARRAY' ) ? @{ delete $attrs->{columns}} : (delete $attrs->{columns} || $source->columns ); + } ( ref($attrs->{columns}) eq 'ARRAY' ) ? + @{ delete $attrs->{columns}} : + (delete $attrs->{columns} || + $source->storage->order_columns_for_select( + $source, + [ $source->columns ] + ) + ); } # add the additional columns on foreach ( 'include_columns', '+columns' ) { @@@ -2882,6 -2875,12 +2882,12 @@@ $attrs->{group_by} = [ $attrs->{group_by} ]; } + # generate the distinct induced group_by early, as prefetch will be carried via a + # subquery (since a group_by is present) + if (delete $attrs->{distinct}) { + $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ]; + } + $attrs->{collapse} ||= {}; if ( my $prefetch = delete $attrs->{prefetch} ) { $prefetch = $self->_merge_attr( {}, $prefetch ); @@@ -2893,19 -2892,16 +2899,16 @@@ my @prefetch = $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering, $attrs->{collapse} ); - $attrs->{prefetch_select} = [ map { $_->[0] } @prefetch ]; - push @{ $attrs->{select} }, @{$attrs->{prefetch_select}}; + # we need to somehow mark which columns came from prefetch + $attrs->{_prefetch_select} = [ map { $_->[0] } @prefetch ]; + + push @{ $attrs->{select} }, @{$attrs->{_prefetch_select}}; push @{ $attrs->{as} }, (map { $_->[1] } @prefetch); push( @{$attrs->{order_by}}, @$prefetch_ordering ); $attrs->{_collapse_order_by} = \@$prefetch_ordering; } - - if (delete $attrs->{distinct}) { - $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ]; - } - # if both page and offset are specified, produce a combined offset # even though it doesn't make much sense, this is what pre 081xx has # been doing diff --combined lib/DBIx/Class/Storage/DBI.pm index 22ac30c,32be00f..2cfaca4 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@@ -15,8 -15,8 +15,8 @@@ use Scalar::Util() use List::Util(); __PACKAGE__->mk_group_accessors('simple' => - qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts - _conn_pid _conn_tid transaction_depth _dbh_autocommit savepoints/ + qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts _conn_pid + _conn_tid transaction_depth _dbh_autocommit _driver_determined savepoints/ ); # the values for these accessors are picked out (and deleted) from @@@ -629,8 -629,7 +629,8 @@@ sub disconnect $self->_do_connection_actions(disconnect_call_ => $_) for @actions; - $self->_dbh->rollback unless $self->_dbh_autocommit; + $self->_dbh_rollback unless $self->_dbh_autocommit; + $self->_dbh->disconnect; $self->_dbh(undef); $self->{_dbh_gen}++; @@@ -764,23 -763,27 +764,27 @@@ sub _populate_dbh sub _determine_driver { my ($self) = @_; - if (ref $self eq 'DBIx::Class::Storage::DBI') { - my $driver; + if (not $self->_driver_determined) { + if (ref($self) eq __PACKAGE__) { + my $driver; - if ($self->_dbh) { # we are connected - $driver = $self->_dbh->{Driver}{Name}; - } else { - # try to use dsn to not require being connected, the driver may still - # force a connection in _rebless to determine version - ($driver) = $self->_dbi_connect_info->[0] =~ /dbi:([^:]+):/i; - } + if ($self->_dbh) { # we are connected + $driver = $self->_dbh->{Driver}{Name}; + } else { + # try to use dsn to not require being connected, the driver may still + # force a connection in _rebless to determine version + ($driver) = $self->_dbi_connect_info->[0] =~ /dbi:([^:]+):/i; + } - my $storage_class = "DBIx::Class::Storage::DBI::${driver}"; - if ($self->load_optional_class($storage_class)) { - mro::set_mro($storage_class, 'c3'); - bless $self, $storage_class; - $self->_rebless(); + my $storage_class = "DBIx::Class::Storage::DBI::${driver}"; + if ($self->load_optional_class($storage_class)) { + mro::set_mro($storage_class, 'c3'); + bless $self, $storage_class; + $self->_rebless(); + } } + + $self->_driver_determined(1); } } @@@ -987,25 -990,20 +991,25 @@@ sub txn_begin # this isn't ->_dbh-> because # we should reconnect on begin_work # for AutoCommit users - $self->dbh->begin_work; + $self->_dbh_begin_work; } elsif ($self->auto_savepoint) { $self->svp_begin; } $self->{transaction_depth}++; } +sub _dbh_begin_work { + my $self = shift; + $self->dbh->begin_work; +} + sub txn_commit { my $self = shift; if ($self->{transaction_depth} == 1) { my $dbh = $self->_dbh; $self->debugobj->txn_commit() if ($self->debug); - $dbh->commit; + $self->_dbh_commit; $self->{transaction_depth} = 0 if $self->_dbh_autocommit; } @@@ -1016,11 -1014,6 +1020,11 @@@ } } +sub _dbh_commit { + my $self = shift; + $self->_dbh->commit; +} + sub txn_rollback { my $self = shift; my $dbh = $self->_dbh; @@@ -1030,7 -1023,7 +1034,7 @@@ if ($self->debug); $self->{transaction_depth} = 0 if $self->_dbh_autocommit; - $dbh->rollback; + $self->_dbh_rollback; } elsif($self->{transaction_depth} > 1) { $self->{transaction_depth}--; @@@ -1053,11 -1046,6 +1057,11 @@@ } } +sub _dbh_rollback { + my $self = shift; + $self->_dbh->rollback; +} + # This used to be the top-half of _execute. It was split out to make it # easier to override in NoBindVars without duping the rest. It takes up # all of _execute's args, and emits $sql, @bind. @@@ -1158,12 -1146,17 +1162,17 @@@ sub _execute sub insert { my ($self, $source, $to_insert) = @_; + # redispatch to insert method of storage we reblessed into, if necessary + if (not $self->_driver_determined) { + $self->_determine_driver; + goto $self->can('insert'); + } + my $ident = $source->from; my $bind_attributes = $self->source_bind_attributes($source); my $updated_cols = {}; - $self->ensure_connected; foreach my $col ( $source->columns ) { if ( !defined $to_insert->{$col} ) { my $col_info = $source->column_info($col); @@@ -1459,7 -1452,7 +1468,7 @@@ sub _select_args ( $attrs->{rows} && keys %{$attrs->{collapse}} ) || ( $attrs->{group_by} && @{$attrs->{group_by}} && - $attrs->{prefetch_select} && @{$attrs->{prefetch_select}} ) + $attrs->{_prefetch_select} && @{$attrs->{_prefetch_select}} ) ) { ($ident, $select, $where, $attrs) = $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs); @@@ -1504,14 -1497,15 +1513,15 @@@ sub _adjust_select_args_for_complex_pre # separate attributes my $sub_attrs = { %$attrs }; delete $attrs->{$_} for qw/where bind rows offset group_by having/; - delete $sub_attrs->{$_} for qw/for collapse prefetch_select _collapse_order_by select as/; + delete $sub_attrs->{$_} for qw/for collapse _prefetch_select _collapse_order_by select as/; - my $alias = $attrs->{alias}; + my $select_root_alias = $attrs->{alias}; my $sql_maker = $self->sql_maker; # create subquery select list - consider only stuff *not* brought in by the prefetch my $sub_select = []; - for my $i (0 .. @{$attrs->{select}} - @{$attrs->{prefetch_select}} - 1) { + my $sub_group_by; + for my $i (0 .. @{$attrs->{select}} - @{$attrs->{_prefetch_select}} - 1) { my $sel = $attrs->{select}[$i]; # alias any functions to the dbic-side 'as' label @@@ -1533,29 -1527,15 +1543,15 @@@ ]; } - # mangle {from} + # mangle {from}, keep in mind that $from is "headless" from here on my $join_root = shift @$from; - my @outer_from = @$from; my %inner_joins; my %join_info = map { $_->[0]{-alias} => $_->[0] } (@$from); - # in complex search_related chains $alias may *not* be 'me' - # so always include it in the inner join, and also shift away - # from the outer stack, so that the two datasets actually do - # meet - if ($join_root->{-alias} ne $alias) { - $inner_joins{$alias} = 1; - - while (@outer_from && $outer_from[0][0]{-alias} ne $alias) { - shift @outer_from; - } - if (! @outer_from) { - $self->throw_exception ("Unable to find '$alias' in the {from} stack, something is wrong"); - } - - shift @outer_from; # the new subquery will represent this alias, so get rid of it - } + # in complex search_related chains $select_root_alias may *not* be + # 'me' so always include it in the inner join + $inner_joins{$select_root_alias} = 1 if ($join_root->{-alias} ne $select_root_alias); # decide which parts of the join will remain on the inside @@@ -1624,13 -1604,15 +1620,15 @@@ # if a multi-type join was needed in the subquery ("multi" is indicated by # presence in {collapse}) - add a group_by to simulate the collapse in the subq - for my $alias (keys %inner_joins) { - - # the dot comes from some weirdness in collapse - # remove after the rewrite - if ($attrs->{collapse}{".$alias"}) { - $sub_attrs->{group_by} ||= $sub_select; - last; + unless ($sub_attrs->{group_by}) { + for my $alias (keys %inner_joins) { + + # the dot comes from some weirdness in collapse + # remove after the rewrite + if ($attrs->{collapse}{".$alias"}) { + $sub_attrs->{group_by} ||= $sub_select; + last; + } } } @@@ -1641,14 -1623,42 +1639,42 @@@ $where, $sub_attrs ); - - # put it in the new {from} - unshift @outer_from, { - -alias => $alias, + my $subq_joinspec = { + -alias => $select_root_alias, -source_handle => $join_root->{-source_handle}, - $alias => $subq, + $select_root_alias => $subq, }; + # Generate a new from (really just replace the join slot with the subquery) + # Before we would start the outer chain from the subquery itself (i.e. + # SELECT ... FROM (SELECT ... ) alias JOIN ..., but this turned out to be + # a bad idea for search_related, as the root of the chain was effectively + # lost (i.e. $artist_rs->search_related ('cds'... ) would result in alias + # of 'cds', which would prevent from doing things like order_by artist.*) + # See t/prefetch/via_search_related.t for a better idea + my @outer_from; + if ($join_root->{-alias} eq $select_root_alias) { # just swap the root part and we're done + @outer_from = ( + $subq_joinspec, + @$from, + ) + } + else { # this is trickier + @outer_from = ($join_root); + + for my $j (@$from) { + if ($j->[0]{-alias} eq $select_root_alias) { + push @outer_from, [ + $subq_joinspec, + @{$j}[1 .. $#$j], + ]; + } + else { + push @outer_from, $j; + } + } + } + # This is totally horrific - the $where ends up in both the inner and outer query # Unfortunately not much can be done until SQLA2 introspection arrives, and even # then if where conditions apply to the *right* side of the prefetch, you may have @@@ -1698,7 -1708,7 +1724,7 @@@ sub _resolve_ident_sources # also note: this adds -result_source => $rsrc to the column info # # usage: - # my $col_sources = $self->_resolve_column_info($ident, [map $_->[0], @{$bind}]); + # my $col_sources = $self->_resolve_column_info($ident, @column_names); sub _resolve_column_info { my ($self, $ident, $colnames) = @_; my ($alias2src, $root_alias) = $self->_resolve_ident_sources($ident); @@@ -1706,17 -1716,39 +1732,39 @@@ my $sep = $self->_sql_maker_opts->{name_sep} || '.'; $sep = "\Q$sep\E"; - my (%return, %converted); + my (%return, %seen_cols); + + # compile a global list of column names, to be able to properly + # disambiguate unqualified column names (if at all possible) + for my $alias (keys %$alias2src) { + my $rsrc = $alias2src->{$alias}; + for my $colname ($rsrc->columns) { + push @{$seen_cols{$colname}}, $alias; + } + } + + COLUMN: foreach my $col (@$colnames) { my ($alias, $colname) = $col =~ m/^ (?: ([^$sep]+) $sep)? (.+) $/x; - # deal with unqualified cols - we assume the main alias for all - # unqualified ones, ugly but can't think of anything better right now - $alias ||= $root_alias; + unless ($alias) { + # see if the column was seen exactly once (so we know which rsrc it came from) + if ($seen_cols{$colname} and @{$seen_cols{$colname}} == 1) { + $alias = $seen_cols{$colname}[0]; + } + else { + next COLUMN; + } + } my $rsrc = $alias2src->{$alias}; - $return{$col} = $rsrc && { %{$rsrc->column_info($colname)}, -result_source => $rsrc }; + $return{$col} = $rsrc && { + %{$rsrc->column_info($colname)}, + -result_source => $rsrc, + -source_alias => $alias, + }; } + return \%return; } @@@ -2306,23 -2338,6 +2354,23 @@@ sub lag_behind_master return; } +=head2 order_columns_for_select + +Returns an ordered list of column names for use with a C