From: Peter Rabbitson Date: Fri, 26 Sep 2014 02:20:51 +0000 (+0200) Subject: Adjust things for the is_literal_value and -ident SQLA 1.80 fixes X-Git-Tag: v0.082801~3 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=f6fff270679468d60c6a3928cf8a220d52275a2d;p=dbsrgits%2FDBIx-Class.git Adjust things for the is_literal_value and -ident SQLA 1.80 fixes (SQLA commits 52ce7dca and ddd6fbb6f) The Oracle workaround is absolutely horrific and needs to be redone from scratch. For the time being the tests seem to pass, so punt for later... sigh --- diff --git a/Changes b/Changes index 6676078..6931858 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,8 @@ Revision history for DBIx::Class + * Misc + - Adjust internals to accommodate fixed API in SQL::Abstract + 0.082800 2014-09-25 14:45 (UTC) * Known Issues - Passing large amounts of objects with stringification overload diff --git a/lib/DBIx/Class/SQLMaker/OracleJoins.pm b/lib/DBIx/Class/SQLMaker/OracleJoins.pm index fe9bd07..0f50467 100644 --- a/lib/DBIx/Class/SQLMaker/OracleJoins.pm +++ b/lib/DBIx/Class/SQLMaker/OracleJoins.pm @@ -93,13 +93,22 @@ sub _recurse_oracle_joins { @{$on->{-and}} == 1 ); - # sadly SQLA treats where($scalar) as literal, so we need to jump some hoops - push @where, map { \sprintf ('%s%s = %s%s', - ref $_ ? $self->_recurse_where($_) : $self->_quote($_), - $left_join, - ref $on->{$_} ? $self->_recurse_where($on->{$_}) : $self->_quote($on->{$_}), - $right_join, - )} keys %$on; + + push @where, map { \do { + my ($sql) = $self->_recurse_where({ + # FIXME - more borkage, more or less a copy of the kludge in ::SQLMaker::_join_condition() + $_ => ( length ref $on->{$_} + ? $on->{$_} + : { -ident => $on->{$_} } + ) + }); + + $sql =~ s/\s*\=/$left_join =/ + if $left_join; + + "$sql$right_join"; + } + } sort keys %$on; } return { -and => \@where }; diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index f8f908d..29b7f13 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -1204,6 +1204,7 @@ sub _collapse_cond_unroll_pairs { if (ref $rhs eq 'HASH' and ! keys %$rhs) { # FIXME - SQLA seems to be doing... nothing...? } + # normalize top level -ident, for saner extract_fixed_condition_columns code elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-ident}) { push @conds, { $lhs => { '=', $rhs } }; } @@ -1211,7 +1212,7 @@ sub _collapse_cond_unroll_pairs { push @conds, { $lhs => $rhs->{-value} }; } elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='}) { - if( is_literal_value $rhs->{'='}) { + if ( length ref $rhs->{'='} and is_literal_value $rhs->{'='} ) { push @conds, { $lhs => $rhs }; } else { @@ -1229,7 +1230,14 @@ sub _collapse_cond_unroll_pairs { my ($l, $r) = %$p; - push @conds, ( ! length ref $r or is_plain_value($r) ) + push @conds, ( + ! length ref $r + or + # the unroller recursion may return a '=' prepended value already + ref $r eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='} + or + is_plain_value($r) + ) ? { $l => $r } : { $l => { '=' => $r } } ; @@ -1327,7 +1335,15 @@ sub _extract_fixed_condition_columns { } } # do not need to check for plain values - _collapse_cond did it for us - elsif(length ref $v->{'='} and is_literal_value($v->{'='}) ) { + elsif( + length ref $v->{'='} + and + ( + ( ref $v->{'='} eq 'HASH' and keys %{$v->{'='}} == 1 and exists $v->{'='}{-ident} ) + or + is_literal_value($v->{'='}) + ) + ) { $vals->{ 'SER_' . serialize $v->{'='} } = $v->{'='}; } } diff --git a/t/sqlmaker/oraclejoin.t b/t/sqlmaker/oraclejoin.t index c1725e0..e8e7444 100644 --- a/t/sqlmaker/oraclejoin.t +++ b/t/sqlmaker/oraclejoin.t @@ -15,13 +15,15 @@ use DBIx::Class::SQLMaker::OracleJoins; my $sa = DBIx::Class::SQLMaker::OracleJoins->new; +for my $rhs ( "me.artist", { -ident => "me.artist" } ) { + # my ($self, $table, $fields, $where, $order, @rest) = @_; my ($sql, @bind) = $sa->select( [ { me => "cd" }, [ { "-join_type" => "LEFT", artist => "artist" }, - { "artist.artistid" => "me.artist" }, + { "artist.artistid" => $rhs }, ], ], [ 'cd.cdid', 'cd.artist', 'cd.title', 'cd.year', 'artist.artistid', 'artist.name' ], @@ -39,7 +41,7 @@ is_same_sql_bind( { me => "cd" }, [ { "-join_type" => "", artist => "artist" }, - { "artist.artistid" => "me.artist" }, + { "artist.artistid" => $rhs }, ], ], [ 'cd.cdid', 'cd.artist', 'cd.title', 'cd.year', 'artist.artistid', 'artist.name' ], @@ -56,8 +58,26 @@ is_same_sql_bind( [ { me => "cd" }, [ + { "-join_type" => "right", artist => "artist" }, + { "artist.artistid" => $rhs }, + ], + ], + [ 'cd.cdid', 'cd.artist', 'cd.title', 'cd.year', 'artist.artistid', 'artist.name' ], + { 'artist.artistid' => 3 }, + undef +); +is_same_sql_bind( + $sql, \@bind, + 'SELECT cd.cdid, cd.artist, cd.title, cd.year, artist.artistid, artist.name FROM cd me, artist artist WHERE ( ( ( artist.artistid = me.artist(+) ) AND ( artist.artistid = ? ) ) )', [3], + 'WhereJoins search with where clause' +); + +($sql, @bind) = $sa->select( + [ + { me => "cd" }, + [ { "-join_type" => "LEFT", artist => "artist" }, - { "artist.artistid" => "me.artist" }, + { "artist.artistid" => $rhs }, ], ], [ 'cd.cdid', 'cd.artist', 'cd.title', 'cd.year', 'artist.artistid', 'artist.name' ], @@ -70,5 +90,7 @@ is_same_sql_bind( 'WhereJoins search with or in where clause' ); +} + done_testing;