From: Peter Rabbitson Date: Thu, 28 Feb 2013 08:31:13 +0000 (+0100) Subject: Radically rethink complex prefetch - make most useful cases just work (tm) X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=1e4f9fb3b8bd1f54518bc2942554099356fa6524;p=dbsrgits%2FDBIx-Class-Historic.git Radically rethink complex prefetch - make most useful cases just work (tm) TL;DR: mst - I AM SORRY!!! I will rebase the dq branch for you when this pile of eyebleed goes stable. The long version - since we now allow arbitrary prefetch, the old _prefetch_selector_range mechanism doesn't cut it anymore. Instead we recognize prefetch solely based on _related_results_construction. Furthermore group_by/limits do not play well with right-side order_by (which we now also support, by transforming foreign order criteria into aggregates). Thus a much more powerful introspection is needed to decide what goes on the inside and outside of the prefetch subquery. This is mostly done now by the augmented _resolve_aliastypes_from_select_args to track identifiers it saw (97e130fa48), and by extra logic considering what exactly are we grouping by. Everything is done while observing the "group over selection + aggregates only" rule, which sould allow us to remain RDBMS agnostic (even for pathological cases of "MySQL-ish aggregates"). As a bonus more cases of "the user knows what they are doing" are now correctly recognized and left alone. See a t/prefetch/with_limit.t diff for a general idea of the scope of improvements. Yes - there is more regexing crap in the codebase now, and it is possible we will call _resolve_aliastypes_from_select_args up to 4(!!!) times per statement preparation. However this allows us to establish a set of test cases towards which to write optimizations/flog the dq framework. --- diff --git a/Changes b/Changes index 24dd469..436771f 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,8 @@ Revision history for DBIx::Class * New Features / Changes + - Prefetch with limit on right-side ordered resultsets now works + correctly (via aggregated grouping) - Changing the result_class of a ResultSet in progress is now explicitly forbidden. The behavior was undefined before, and would result in wildly differing outcomes depending on $rs @@ -16,6 +18,7 @@ Revision history for DBIx::Class * Fixes - Properly consider unselected order_by criteria during complex subqueried prefetch + - Properly support "MySQL-style" left-side group_by with prefetch 0.08209 2013-03-01 12:56 (UTC) * New Features / Changes diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 2582fe2..a2e3a4c 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -1611,7 +1611,7 @@ sub _count_rs { my $tmp_attrs = { %$attrs }; # take off any limits, record_filter is cdbi, and no point of ordering nor locking a count - delete @{$tmp_attrs}{qw/rows offset order_by record_filter for/}; + delete @{$tmp_attrs}{qw/rows offset order_by _related_results_construction record_filter for/}; # overwrite the selector (supplied by the storage) $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $attrs); @@ -1633,7 +1633,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, nor locking it - delete @{$sub_attrs}{qw/collapse columns as select _prefetch_selector_range order_by for/}; + delete @{$sub_attrs}{qw/collapse columns as select _related_results_construction order_by for/}; # if we multi-prefetch we group_by something unique, as this is what we would # get out of the rs via ->next/->all. We *DO WANT* to clobber old group_by regardless @@ -1836,7 +1836,7 @@ sub _rs_update_delete { my $attrs = { %{$self->_resolved_attrs} }; my $join_classifications; - my $existing_group_by = delete $attrs->{group_by}; + my ($existing_group_by) = delete @{$attrs}{qw(group_by _grouped_by_distinct)}; # do we need a subquery for any reason? my $needs_subq = ( @@ -1897,7 +1897,7 @@ sub _rs_update_delete { ); # make a new $rs selecting only the PKs (that's all we really need for the subq) - delete $attrs->{$_} for qw/collapse select _prefetch_selector_range as/; + delete $attrs->{$_} for qw/select as collapse _related_results_construction/; $attrs->{columns} = [ map { "$attrs->{alias}.$_" } @$idcols ]; $attrs->{group_by} = \ ''; # FIXME - this is an evil hack, it causes the optimiser to kick in and throw away the LEFT joins my $subrs = (ref $self)->new($rsrc, $attrs); @@ -3282,7 +3282,7 @@ sub _chain_relationship { # ->_resolve_join as otherwise they get lost - captainL my $join = $self->_merge_joinpref_attr( $attrs->{join}, $attrs->{prefetch} ); - delete @{$attrs}{qw/join prefetch collapse group_by distinct select as columns +select +as +columns/}; + delete @{$attrs}{qw/join prefetch collapse group_by distinct _grouped_by_distinct select as columns +select +as +columns/}; my $seen = { %{ (delete $attrs->{seen_join}) || {} } }; @@ -3492,6 +3492,7 @@ sub _resolved_attrs { carp_unique ("Useless use of distinct on a grouped resultset ('distinct' is ignored when a 'group_by' is present)"); } else { + $attrs->{_grouped_by_distinct} = 1; # distinct affects only the main selection part, not what prefetch may # add below. $attrs->{group_by} = $source->storage->_group_over_selection ( @@ -3537,17 +3538,11 @@ sub _resolved_attrs { my @prefetch = $source->_resolve_prefetch( $prefetch, $alias, $join_map ); - # we need to somehow mark which columns came from prefetch - if (@prefetch) { - my $sel_end = $#{$attrs->{select}}; - $attrs->{_prefetch_selector_range} = [ $sel_end + 1, $sel_end + @prefetch ]; - } - push @{ $attrs->{select} }, (map { $_->[0] } @prefetch); push @{ $attrs->{as} }, (map { $_->[1] } @prefetch); } - if ( defined List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) { + if ( List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) { $attrs->{_related_results_construction} = 1; } else { diff --git a/lib/DBIx/Class/ResultSetColumn.pm b/lib/DBIx/Class/ResultSetColumn.pm index a3ab2cc..5f37d89 100644 --- a/lib/DBIx/Class/ResultSetColumn.pm +++ b/lib/DBIx/Class/ResultSetColumn.pm @@ -97,7 +97,7 @@ sub new { if ($colmap->{$select} and $rsrc->_identifying_column_set([$colmap->{$select}])) { $new_attrs->{group_by} = [ $select ]; - delete $new_attrs->{distinct}; # it is ignored when group_by is present + delete @{$new_attrs}{qw(distinct _grouped_by_distinct)}; # it is ignored when group_by is present } else { carp ( diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 8d4ff35..9c622f8 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -2285,24 +2285,33 @@ sub _select_args { $attrs->{rows} = $sql_maker->__max_int; } - my @limit; + my ($complex_prefetch, @limit); - # see if we need to tear the prefetch apart otherwise delegate the limiting to the - # storage, unless software limit was requested + # see if we will need to tear the prefetch apart to satisfy group_by == select + # this is *extremely tricky* to get right + # + # Follows heavy but necessary analyzis of the group_by - if it refers to any + # sort of non-root column assume the user knows what they are doing and do + # not try to be clever if ( - # limited collapsing has_many - ( $attrs->{rows} && $attrs->{collapse} ) - || - # grouped prefetch (to satisfy group_by == select) - ( $attrs->{group_by} - && - @{$attrs->{group_by}} - && - $attrs->{_prefetch_selector_range} - ) + $attrs->{_related_results_construction} + and + $attrs->{group_by} + and + @{$attrs->{group_by}} + and + my $grp_aliases = try { + $self->_resolve_aliastypes_from_select_args( $attrs->{from}, undef, undef, { group_by => $attrs->{group_by} } ) + } ) { - ($ident, $select, $where, $attrs) - = $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs); + $complex_prefetch = ! defined first { $_ ne $rs_alias } keys %{ $grp_aliases->{grouping} || {} }; + } + + $complex_prefetch ||= ( $attrs->{rows} && $attrs->{collapse} ); + + if ($complex_prefetch) { + ($ident, $select, $where, $attrs) = + $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs); } elsif (! $attrs->{software_limit} ) { push @limit, ( @@ -2313,6 +2322,8 @@ sub _select_args { # try to simplify the joinmap further (prune unreferenced type-single joins) if ( + ! $complex_prefetch + and ref $ident and reftype $ident eq 'ARRAY' diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index 75a8fb2..f136f52 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -72,18 +72,17 @@ sub _prune_unused_joins { sub _adjust_select_args_for_complex_prefetch { my ($self, $from, $select, $where, $attrs) = @_; - $self->throw_exception ('Nothing to prefetch... how did we get here?!') - if not @{$attrs->{_prefetch_selector_range}||[]}; - $self->throw_exception ('Complex prefetches are not supported on resultsets with a custom from attribute') if (ref $from ne 'ARRAY' || ref $from->[0] ne 'HASH' || ref $from->[1] ne 'ARRAY'); + my $root_alias = $attrs->{alias}; + # generate inner/outer attribute lists, remove stuff that doesn't apply my $outer_attrs = { %$attrs }; - delete $outer_attrs->{$_} for qw/where bind rows offset group_by having/; + delete $outer_attrs->{$_} for qw/where bind rows offset group_by _grouped_by_distinct having/; my $inner_attrs = { %$attrs }; - delete $inner_attrs->{$_} for qw/for collapse _prefetch_selector_range select as/; + delete $inner_attrs->{$_} for qw/from for collapse select as _related_results_construction/; # there is no point of ordering the insides if there is no limit delete $inner_attrs->{order_by} if ( @@ -107,7 +106,7 @@ sub _adjust_select_args_for_complex_prefetch { : next ; - if ( ($h->{-alias}||'') eq $attrs->{alias} and $h->{-rsrc} ) { + if ( ($h->{-alias}||'') eq $root_alias and $h->{-rsrc} ) { $root_node = $h; $root_node_offset = $i; last; @@ -121,13 +120,16 @@ sub _adjust_select_args_for_complex_prefetch { my $colinfo = $self->_resolve_column_info($from); my $selected_root_columns; - my ($p_start, $p_end) = @{$outer_attrs->{_prefetch_selector_range}}; - for my $i (0 .. $p_start - 1, $p_end + 1 .. $#$outer_select) { + for my $i (0 .. $#$outer_select) { my $sel = $outer_select->[$i]; + next if ( + $colinfo->{$sel} and $colinfo->{$sel}{-source_alias} ne $root_alias + ); + if (ref $sel eq 'HASH' ) { $sel->{-as} ||= $attrs->{as}[$i]; - $outer_select->[$i] = join ('.', $attrs->{alias}, ($sel->{-as} || "inner_column_$i") ); + $outer_select->[$i] = join ('.', $root_alias, ($sel->{-as} || "inner_column_$i") ); } elsif (! ref $sel and my $ci = $colinfo->{$sel}) { $selected_root_columns->{$ci->{-colname}} = 1; @@ -153,7 +155,7 @@ sub _adjust_select_args_for_complex_prefetch { for (sort map { keys %{$_->{-seen_columns}||{}} } map { values %$_ } values %$connecting_aliastypes) { my $ci = $colinfo->{$_} or next; if ( - $ci->{-source_alias} eq $attrs->{alias} + $ci->{-source_alias} eq $root_alias and ! $selected_root_columns->{$ci->{-colname}}++ ) { @@ -178,27 +180,104 @@ sub _adjust_select_args_for_complex_prefetch { my $inner_aliastypes = $self->_resolve_aliastypes_from_select_args( $inner_from, $inner_select, $where, $inner_attrs ); - # we need to simulate collapse in the subq if a multiplying join is pulled - # by being a non-selecting restrictor + # uh-oh a multiplier (which is not us) left in, this is a problem if ( - ! $inner_attrs->{group_by} + $inner_aliastypes->{multiplying} + and + !$inner_aliastypes->{grouping} # if there are groups - assume user knows wtf they are up to and - first { - $inner_aliastypes->{restricting}{$_} - and - ! $inner_aliastypes->{selecting}{$_} - } ( keys %{$inner_aliastypes->{multiplying}||{}} ) + my @multipliers = grep { $_ ne $root_alias } keys %{$inner_aliastypes->{multiplying}} ) { - my $unprocessed_order_chunks; - ($inner_attrs->{group_by}, $unprocessed_order_chunks) = $self->_group_over_selection ( - $inner_from, $inner_select, $inner_attrs->{order_by} - ); - - $self->throw_exception ( - 'A required group_by clause could not be constructed automatically due to a complex ' - . 'order_by criteria. Either order_by columns only (no functions) or construct a suitable ' - . 'group_by by hand' - ) if $unprocessed_order_chunks; + + # if none of the multipliers came from an order_by (guaranteed to have been combined + # with a limit) - easy - just slap a group_by to simulate a collape and be on our way + if ( + ! $inner_aliastypes->{ordering} + or + ! first { $inner_aliastypes->{ordering}{$_} } @multipliers + ) { + my $unprocessed_order_chunks; + ($inner_attrs->{group_by}, $unprocessed_order_chunks) = $self->_group_over_selection ( + $inner_from, $inner_select, $inner_attrs->{order_by} + ); + + $self->throw_exception ( + 'A required group_by clause could not be constructed automatically due to a complex ' + . 'order_by criteria. Either order_by columns only (no functions) or construct a suitable ' + . 'group_by by hand' + ) if $unprocessed_order_chunks; + } + else { + # We need to order by external columns and group at the same time + # so we can calculate the proper limit + # This doesn't really make sense in SQL, however from DBICs point + # of view is rather valid (order the leftmost objects by whatever + # criteria and get the offset/rows many). There is a way around + # this however in SQL - we simply tae the direction of each piece + # of the foreign order and convert them to MIN(X) for ASC or MAX(X) + # for DESC, and group_by the root columns. The end result should be + # exactly what we expect + + # FIXME REMOVE LATER - (just a sanity check) + if (defined ( my $impostor = first + { $_ ne $root_alias } + keys %{ $inner_aliastypes->{selecting} } + ) ) { + $self->throw_exception(sprintf + 'Unexpected inner selection during complex prefetch (%s)...', + join ', ', keys %{ $inner_aliastypes->{joining}{$impostor}{-seen_columns} || {} } + ); + } + + # supplement the main selection with pks if not already there, + # as they will have to be a part of the group_by to colapse + # things properly + my $cur_sel = { map { $_ => 1 } @$inner_select }; + my @pks = map { "$root_alias.$_" } $root_node->{-rsrc}->primary_columns + or $self->throw_exception( sprintf + 'Unable to perform complex limited prefetch off %s without declared primary key', + $root_node->{-rsrc}->source_name, + ); + for my $col (@pks) { + push @$inner_select, $col + unless $cur_sel->{$col}++; + } + + # wrap any part of the order_by that "responds" to an ordering alias + # into a MIN/MAX + # FIXME - this code is a joke, will need to be completely rewritten in + # the DQ branch. But I need to push a POC here, otherwise the + # pesky tests won't pass + my $sql_maker = $self->sql_maker; + my ($lquote, $rquote, $sep) = map { quotemeta $_ } ($sql_maker->_quote_chars, $sql_maker->name_sep); + my $own_re = qr/ $lquote \Q$root_alias\E $rquote $sep | \b \Q$root_alias\E $sep /x; + my @order = @{$attrs->{order_by}}; + my @order_chunks = map { ref $_ eq 'ARRAY' ? $_ : [ $_ ] } $sql_maker->_order_by_chunks (\@order); + $self->throw_exception ('Order By parsing failed...') if @order != @order_chunks; + for my $i (0 .. $#order) { + # skip ourselves, and anything that looks like a literal + next if $order_chunks[$i][0] =~ $own_re; + next if (ref $order[$i] and ref $order[$i] ne 'HASH'); + + my $is_desc = $order_chunks[$i][0] =~ s/\sDESC$//i; + $order_chunks[$i][0] =~ s/\sASC$//i; + + $order[$i] = \[ + sprintf( + '%s(%s)%s', + ($is_desc ? 'MAX' : 'MIN'), + $order_chunks[$i][0], + ($is_desc ? ' DESC' : ''), + ), + @ {$order_chunks[$i]} [ 1 .. $#{$order_chunks[$i]} ] + ]; + } + + $inner_attrs->{order_by} = \@order; + ($inner_attrs->{group_by}) = $self->_group_over_selection ( + $inner_from, $inner_select, $inner_attrs->{order_by} + ); + } } # we already optimized $inner_from above @@ -236,18 +315,18 @@ sub _adjust_select_args_for_complex_prefetch { push @outer_from, [ { - -alias => $attrs->{alias}, + -alias => $root_alias, -rsrc => $root_node->{-rsrc}, - $attrs->{alias} => $inner_subq, + $root_alias => $inner_subq, }, @{$from->[0]}[1 .. $#{$from->[0]}], ]; } else { @outer_from = { - -alias => $attrs->{alias}, + -alias => $root_alias, -rsrc => $root_node->{-rsrc}, - $attrs->{alias} => $inner_subq, + $root_alias => $inner_subq, }; } @@ -259,9 +338,9 @@ sub _adjust_select_args_for_complex_prefetch { $self->_resolve_aliastypes_from_select_args( $from, $outer_select, $where, $outer_attrs ); # unroll parents - my ($outer_select_chain, $outer_restrict_chain) = map { +{ - map { $_ => 1 } map { values %$_} map { @{$_->{-parents}} } values %{ $outer_aliastypes->{$_} } - } } qw/selecting restricting/; + my ($outer_select_chain, @outer_nonselecting_chains) = map { +{ + map { $_ => 1 } map { values %$_} map { @{$_->{-parents}} } values %{ $outer_aliastypes->{$_} || {} } + } } qw/selecting restricting grouping ordering/; # see what's left - throw away if not selecting/restricting # also throw in a group_by if a non-selecting multiplier, @@ -275,13 +354,13 @@ sub _adjust_select_args_for_complex_prefetch { ) { push @outer_from, $j } - elsif ($outer_restrict_chain->{$alias}) { + elsif (first { $_->{$alias} } @outer_nonselecting_chains ) { push @outer_from, $j; $need_outer_group_by ||= $outer_aliastypes->{multiplying}{$alias} ? 1 : 0; } } - if ($need_outer_group_by and ! $outer_attrs->{group_by}) { + if ( $need_outer_group_by and $attrs->{_grouped_by_distinct} ) { my $unprocessed_order_chunks; ($outer_attrs->{group_by}, $unprocessed_order_chunks) = $self->_group_over_selection ( @@ -379,9 +458,10 @@ sub _resolve_aliastypes_from_select_args { my $to_scan = { restricting => [ $sql_maker->_recurse_where ($where), - $sql_maker->_parse_rs_attrs ({ - map { $_ => $attrs->{$_} } (qw/group_by having/) - }), + $sql_maker->_parse_rs_attrs ({ having => $attrs->{having} }), + ], + grouping => [ + $sql_maker->_parse_rs_attrs ({ group_by => $attrs->{group_by} }), ], joining => [ $sql_maker->_recurse_from ( @@ -391,7 +471,9 @@ sub _resolve_aliastypes_from_select_args { ], selecting => [ $sql_maker->_recurse_fields ($select), - ( map { $_->[0] } $self->_extract_order_criteria ($attrs->{order_by}, $sql_maker) ), + ], + ordering => [ + map { $_->[0] } $self->_extract_order_criteria ($attrs->{order_by}, $sql_maker), ], }; @@ -411,7 +493,7 @@ sub _resolve_aliastypes_from_select_args { for my $piece (@{$to_scan->{$type}}) { if (my @matches = $piece =~ /$al_re/g) { $aliases_by_type->{$type}{$alias} ||= { -parents => $alias_list->{$alias}{-join_path}||[] }; - $aliases_by_type->{$type}{$alias}{-seen_columns}{"$alias.$_"} = 1 + $aliases_by_type->{$type}{$alias}{-seen_columns}{"$alias.$_"} = "$alias.$_" for grep { defined $_ } @matches; } } @@ -430,7 +512,7 @@ sub _resolve_aliastypes_from_select_args { if (my @matches = $piece =~ /$col_re/g) { my $alias = $colinfo->{$col}{-source_alias}; $aliases_by_type->{$type}{$alias} ||= { -parents => $alias_list->{$alias}{-join_path}||[] }; - $aliases_by_type->{$type}{$alias}{-seen_columns}{"$alias.$_"} = 1 + $aliases_by_type->{$type}{$alias}{-seen_columns}{"$alias.$_"} = $_ for grep { defined $_ } @matches; } } @@ -447,6 +529,10 @@ sub _resolve_aliastypes_from_select_args { ); } + for (keys %$aliases_by_type) { + delete $aliases_by_type->{$_} unless keys %{$aliases_by_type->{$_}}; + } + return $aliases_by_type; } diff --git a/t/prefetch/grouped.t b/t/prefetch/grouped.t index f5d56bb..b7ec8eb 100644 --- a/t/prefetch/grouped.t +++ b/t/prefetch/grouped.t @@ -207,6 +207,71 @@ for ($cd_rs->all) { $schema->storage->debug ($sdebug); } +{ + # test lifted from soulchild + + my $most_tracks_rs = $schema->resultset ('CD')->search ( + { + 'me.cdid' => { '!=' => undef }, # this is just to test WHERE + 'tracks.trackid' => { '!=' => undef }, + }, + { + join => 'tracks', + prefetch => 'liner_notes', + select => ['me.cdid', 'liner_notes.notes', { count => 'tracks.trackid', -as => 'tr_count' }, { max => 'tracks.trackid', -as => 'tr_maxid'} ], + as => [qw/cdid notes track_count max_track_id/], + order_by => [ { -desc => 'tr_count' }, { -asc => 'tr_maxid' } ], + group_by => 'me.cdid', + rows => 2, + } + ); + + is_same_sql_bind( + $most_tracks_rs->as_query, + '(SELECT me.cdid, liner_notes.notes, me.tr_count, me.tr_maxid, + liner_notes.liner_id, liner_notes.notes + FROM ( + SELECT me.cdid, COUNT(tracks.trackid) AS tr_count, MAX(tracks.trackid) AS tr_maxid + FROM cd me + LEFT JOIN track tracks + ON tracks.cd = me.cdid + WHERE me.cdid IS NOT NULL AND tracks.trackid IS NOT NULL + GROUP BY me.cdid + ORDER BY tr_count DESC, tr_maxid ASC + LIMIT ? + ) me + LEFT JOIN track tracks + ON tracks.cd = me.cdid + LEFT JOIN liner_notes liner_notes + ON liner_notes.liner_id = me.cdid + WHERE me.cdid IS NOT NULL AND tracks.trackid IS NOT NULL + ORDER BY tr_count DESC, tr_maxid ASC + )', + [[$ROWS => 2]], + 'Oddball mysql-ish group_by usage yields valid SQL', + ); + + is ($most_tracks_rs->count, 2, 'Limit works'); + my ($top_cd) = $most_tracks_rs->all; + is ($top_cd->id, 2, 'Correct cd fetched on top'); # 2 because of the slice(1,1) earlier + + my $query_cnt = 0; + $schema->storage->debugcb ( sub { $query_cnt++ } ); + $schema->storage->debug (1); + + is ($top_cd->get_column ('track_count'), 4, 'Track count fetched correctly'); + is ( + $top_cd->liner_notes->notes, + 'Buy Whiskey!', + 'Correct liner pre-fetched with top cd', + ); + + is ($query_cnt, 0, 'No queries executed during prefetched data access'); + $schema->storage->debugcb (undef); + $schema->storage->debug ($sdebug); +} + + # make sure that distinct still works { my $rs = $schema->resultset("CD")->search({}, { diff --git a/t/prefetch/manual.t b/t/prefetch/manual.t index c6d1f6a..8135342 100644 --- a/t/prefetch/manual.t +++ b/t/prefetch/manual.t @@ -69,118 +69,49 @@ my $hri_rs = $rs->search({}, { result_class => 'DBIx::Class::ResultClass::HashRe cmp_deeply ( [$hri_rs->all], [ - { - artist => 1, - genreid => 1, - latest_cd => 1981, + { artist => 1, genreid => 1, latest_cd => 1981, title => "Equinoxe", year => 1978, single_track => { cd => { - artist => { - artistid => 1, - cds => [ - { - cdid => 1, - genreid => 1, - tracks => [ - { - title => "m1" - }, - { - title => "m2" - }, - { - title => "m3" - }, - { - title => "m4" - } - ], - year => 1981 - }, - { - cdid => 3, - genreid => 1, - tracks => [ - { - title => "e1" - }, - { - title => "e2" - }, - { - title => "e3" - } - ], - year => 1978 - }, - { - cdid => 2, - genreid => undef, - tracks => [ - { - title => "o1" - }, - { - title => "o2" - } - ], - year => 1976 - } - ] - } - } + artist => { artistid => 1, cds => [ + { cdid => 1, genreid => 1, year => 1981, tracks => [ + { title => "m1" }, + { title => "m2" }, + { title => "m3" }, + { title => "m4" }, + ]}, + { cdid => 3, genreid => 1, year => 1978, tracks => [ + { title => "e1" }, + { title => "e2" }, + { title => "e3" }, + ]}, + { cdid => 2, genreid => undef, year => 1976, tracks => [ + { title => "o1" }, + { title => "o2" }, + ]}, + ]}, + }, }, - title => "Equinoxe", tracks => [ - { - title => "e1" - }, - { - title => "e2" - }, - { - title => "e3" - } + { title => "e1" }, + { title => "e2" }, + { title => "e3" }, ], - year => 1978 }, { - artist => 1, - genreid => undef, - latest_cd => 1981, - single_track => undef, - title => "Oxygene", + artist => 1, genreid => undef, latest_cd => 1981, title => "Oxygene", year => 1976, single_track => undef, tracks => [ - { - title => "o1" - }, - { - title => "o2" - } + { title => "o1" }, + { title => "o2" }, ], - year => 1976 }, { - artist => 1, - genreid => 1, - latest_cd => 1981, - single_track => undef, - title => "Magnetic Fields", + artist => 1, genreid => 1, latest_cd => 1981, title => "Magnetic Fields", year => 1981, single_track => undef, tracks => [ - { - title => "m1" - }, - { - title => "m2" - }, - { - title => "m3" - }, - { - title => "m4" - } + { title => "m1" }, + { title => "m2" }, + { title => "m3" }, + { title => "m4" }, ], - year => 1981 }, ], 'W00T, manual prefetch with collapse works' @@ -259,58 +190,60 @@ if ($ENV{TEST_VERBOSE}) { for @lines; } -my $queries = 0; -$schema->storage->debugcb(sub { $queries++ }); -my $orig_debug = $schema->storage->debug; -$schema->storage->debug (1); - -for my $use_next (0, 1) { - my @random_cds; - if ($use_next) { - warnings_exist { - while (my $o = $rs_random->next) { - push @random_cds, $o; - } - } qr/performed an eager cursor slurp underneath/, - 'Warned on auto-eager cursor'; - } - else { - @random_cds = $rs_random->all; - } - - is (@random_cds, 6, 'object count matches'); - - for my $cd (@random_cds) { - if ($cd->year == 1977) { - is( scalar $cd->tracks, 0, 'no tracks on 1977 cd' ); - is( $cd->single_track, undef, 'no single_track on 1977 cd' ); - } - elsif ($cd->year == 1976) { - is( scalar $cd->tracks, 2, 'Two tracks on 1976 cd' ); - like( $_->title, qr/^o\d/, "correct title" ) - for $cd->tracks; - is( $cd->single_track, undef, 'no single_track on 1976 cd' ); +{ + my $queries = 0; + $schema->storage->debugcb(sub { $queries++ }); + my $orig_debug = $schema->storage->debug; + $schema->storage->debug (1); + + for my $use_next (0, 1) { + my @random_cds; + if ($use_next) { + warnings_exist { + while (my $o = $rs_random->next) { + push @random_cds, $o; + } + } qr/performed an eager cursor slurp underneath/, + 'Warned on auto-eager cursor'; } - elsif ($cd->year == 1981) { - is( scalar $cd->tracks, 4, 'Four tracks on 1981 cd' ); - like( $_->title, qr/^m\d/, "correct title" ) - for $cd->tracks; - is( $cd->single_track, undef, 'no single_track on 1981 cd' ); + else { + @random_cds = $rs_random->all; } - elsif ($cd->year == 1978) { - is( scalar $cd->tracks, 3, 'Three tracks on 1978 cd' ); - like( $_->title, qr/^e\d/, "correct title" ) - for $cd->tracks; - ok( defined $cd->single_track, 'single track prefetched on 1987 cd' ); - is( $cd->single_track->cd->artist->id, 1, 'Single_track->cd->artist prefetched on 1978 cd' ); - is( scalar $cd->single_track->cd->artist->cds, 6, '6 cds prefetched on artist' ); + + is (@random_cds, 6, 'object count matches'); + + for my $cd (@random_cds) { + if ($cd->year == 1977) { + is( scalar $cd->tracks, 0, 'no tracks on 1977 cd' ); + is( $cd->single_track, undef, 'no single_track on 1977 cd' ); + } + elsif ($cd->year == 1976) { + is( scalar $cd->tracks, 2, 'Two tracks on 1976 cd' ); + like( $_->title, qr/^o\d/, "correct title" ) + for $cd->tracks; + is( $cd->single_track, undef, 'no single_track on 1976 cd' ); + } + elsif ($cd->year == 1981) { + is( scalar $cd->tracks, 4, 'Four tracks on 1981 cd' ); + like( $_->title, qr/^m\d/, "correct title" ) + for $cd->tracks; + is( $cd->single_track, undef, 'no single_track on 1981 cd' ); + } + elsif ($cd->year == 1978) { + is( scalar $cd->tracks, 3, 'Three tracks on 1978 cd' ); + like( $_->title, qr/^e\d/, "correct title" ) + for $cd->tracks; + ok( defined $cd->single_track, 'single track prefetched on 1987 cd' ); + is( $cd->single_track->cd->artist->id, 1, 'Single_track->cd->artist prefetched on 1978 cd' ); + is( scalar $cd->single_track->cd->artist->cds, 6, '6 cds prefetched on artist' ); + } } } -} -$schema->storage->debugcb(undef); -$schema->storage->debug($orig_debug); -is ($queries, 2, "Only two queries for rwo prefetch calls total"); + $schema->storage->debugcb(undef); + $schema->storage->debug($orig_debug); + is ($queries, 2, "Only two queries for two prefetch calls total"); +} # can't cmp_deeply a random set - need *some* order my $ord_rs = $rs->search({}, { @@ -335,153 +268,70 @@ cmp_deeply( 'Iteration works correctly', ); -cmp_deeply (\@hris_all, [ - { - single_track => undef, - tracks => [ - { - cd => 2, - title => "o1" - }, - { - cd => 2, - title => "o2" - } - ], - year => 1976 - }, - { - single_track => undef, - tracks => [], - year => 1977 - }, - { - single_track => undef, - tracks => [], - year => 1977 - }, - { - single_track => undef, - tracks => [], - year => 1977 - }, +my @hri_contents = ( + { year => 1976, single_track => undef, tracks => [ + { cd => 2, title => "o1" }, + { cd => 2, title => "o2" }, + ]}, + { year => 1977, single_track => undef, tracks => [] }, + { year => 1977, single_track => undef, tracks => [] }, + { year => 1977, single_track => undef, tracks => [] }, { + year => 1978, single_track => { + trackid => 6, cd => { artist => { - artistid => 1, - cds => [ - { - cdid => 4, - genreid => undef, - tracks => [], - year => 1977 - }, - { - cdid => 5, - genreid => undef, - tracks => [], - year => 1977 - }, - { - cdid => 6, - genreid => undef, - tracks => [], - year => 1977 - }, - { - cdid => 3, - genreid => 1, - tracks => [ - { - title => "e1" - }, - { - title => "e2" - }, - { - title => "e3" - } - ], - year => 1978 - }, - { - cdid => 1, - genreid => 1, - tracks => [ - { - title => "m1" - }, - { - title => "m2" - }, - { - title => "m3" - }, - { - title => "m4" - } - ], - year => 1981 - }, - { - cdid => 2, - genreid => undef, - tracks => [ - { - title => "o1" - }, - { - title => "o2" - } - ], - year => 1976 - } + artistid => 1, cds => [ + { cdid => 4, genreid => undef, year => 1977, tracks => [] }, + { cdid => 5, genreid => undef, year => 1977, tracks => [] }, + { cdid => 6, genreid => undef, year => 1977, tracks => [] }, + { cdid => 3, genreid => 1, year => 1978, tracks => [ + { title => "e1" }, + { title => "e2" }, + { title => "e3" }, + ]}, + { cdid => 1, genreid => 1, year => 1981, tracks => [ + { title => "m1" }, + { title => "m2" }, + { title => "m3" }, + { title => "m4" }, + ]}, + { cdid => 2, genreid => undef, year => 1976, tracks => [ + { title => "o1" }, + { title => "o2" }, + ]}, ] - } + }, }, - trackid => 6 }, tracks => [ - { - cd => 3, - title => "e1" - }, - { - cd => 3, - title => "e2" - }, - { - cd => 3, - title => "e3" - }, - ], - year => 1978 - }, - { - single_track => undef, - tracks => [ - { - cd => 1, - title => "m1" - }, - { - cd => 1, - title => "m2" - }, - { - cd => 1, - title => "m3" - }, - { - cd => 1, - title => "m4" - }, + { cd => 3, title => "e1" }, + { cd => 3, title => "e2" }, + { cd => 3, title => "e3" }, ], - year => 1981 }, -], 'W00T, multi-has_many manual underdefined root prefetch with collapse works'); + { year => 1981, single_track => undef, tracks => [ + { cd => 1, title => "m1" }, + { cd => 1, title => "m2" }, + { cd => 1, title => "m3" }, + { cd => 1, title => "m4" }, + ]}, +); + +cmp_deeply (\@hris_all, \@hri_contents, 'W00T, multi-has_many manual underdefined root prefetch with collapse works'); + +cmp_deeply( + $rs->search({}, { + order_by => [ 'me.year', 'tracks_2.title', 'tracks.title', 'cds.cdid', { -desc => 'name' } ], + rows => 4, + offset => 2, + })->all_hri, + [ @hri_contents[2..5] ], + 'multi-has_many prefetch with limit works too', +); +# left-ordered real iterator $rs = $rs->search({}, { order_by => [ 'me.year', 'me.cdid', \ 'RANDOM()' ] }); my @objs_iter; while (my $r = $rs->next) { diff --git a/t/prefetch/o2m_o2m_order_by_with_limit.t b/t/prefetch/o2m_o2m_order_by_with_limit.t index f7f71e5..c8972c0 100644 --- a/t/prefetch/o2m_o2m_order_by_with_limit.t +++ b/t/prefetch/o2m_o2m_order_by_with_limit.t @@ -16,131 +16,117 @@ my ($ROWS, $OFFSET) = ( my $schema = DBICTest->init_schema(); my $artist_rs = $schema->resultset('Artist'); -my $ar = $artist_rs->current_source_alias; my $filtered_cd_rs = $artist_rs->search_related('cds_unordered', - { "$ar.rank" => 13 }, + { "me.rank" => 13 }, { - prefetch => [ 'tracks' ], - order_by => [ 'tracks.position DESC', { -asc => "$ar.name" }, "$ar.artistid DESC" ], - offset => 13, - rows => 3, + prefetch => 'tracks', + join => 'genre', + order_by => [ { -desc => 'genre.name' }, 'tracks.title DESC', { -asc => "me.name" }, { -desc => 'cds_unordered.title' } ], # me. is the artist, *NOT* the cd }, ); -is_same_sql_bind( - $filtered_cd_rs->as_query, - q{( - SELECT cds_unordered.cdid, cds_unordered.artist, cds_unordered.title, cds_unordered.year, cds_unordered.genreid, cds_unordered.single_track, - tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at - FROM artist me - JOIN ( - SELECT cds_unordered.cdid, cds_unordered.artist, cds_unordered.title, cds_unordered.year, cds_unordered.genreid, cds_unordered.single_track - FROM artist me - JOIN cd cds_unordered - ON cds_unordered.artist = me.artistid - LEFT JOIN track tracks - ON tracks.cd = cds_unordered.cdid - WHERE ( me.rank = ? ) - ORDER BY tracks.position DESC, me.name ASC, me.artistid DESC - LIMIT ? - OFFSET ? - ) cds_unordered - ON cds_unordered.artist = me.artistid - LEFT JOIN track tracks - ON tracks.cd = cds_unordered.cdid - WHERE ( me.rank = ? ) - ORDER BY tracks.position DESC, me.name ASC, me.artistid DESC - )}, - [ - [ { sqlt_datatype => 'integer', dbic_colname => 'me.rank' } => 13 ], - [ $ROWS => 3 ], - [ $OFFSET => 13 ], - [ { sqlt_datatype => 'integer', dbic_colname => 'me.rank' } => 13 ], - ], - 'correct SQL on limited prefetch over search_related ordered by root', -); +my $hri_contents = [ + { + artist => 1, cdid => 1, genreid => 1, single_track => undef, title => "Spoonful of bees", year => 1999, tracks => [ + { cd => 1, last_updated_at => undef, last_updated_on => undef, position => 1, title => "The Bees Knees", trackid => 16 }, + { cd => 1, last_updated_at => undef, last_updated_on => undef, position => 3, title => "Beehind You", trackid => 18 }, + { cd => 1, last_updated_at => undef, last_updated_on => undef, position => 2, title => "Apiary", trackid => 17 }, + ], + }, + { + artist => 1, cdid => 3, genreid => undef, single_track => undef, title => "Caterwaulin' Blues", year => 1997, tracks => [ + { cd => 3, last_updated_at => undef, last_updated_on => undef, position => 1, title => "Yowlin", trackid => 7 }, + { cd => 3, last_updated_at => undef, last_updated_on => undef, position => 2, title => "Howlin", trackid => 8 }, + { cd => 3, last_updated_at => undef, last_updated_on => undef, position => 3, title => "Fowlin", trackid => 9 }, + ], + }, + { + artist => 3, cdid => 5, genreid => undef, single_track => undef, title => "Come Be Depressed With Us", year => 1998, tracks => [ + { cd => 5, last_updated_at => undef, last_updated_on => undef, position => 2, title => "Under The Weather", trackid => 14 }, + { cd => 5, last_updated_at => undef, last_updated_on => undef, position => 3, title => "Suicidal", trackid => 15 }, + { cd => 5, last_updated_at => undef, last_updated_on => undef, position => 1, title => "Sad", trackid => 13 }, + ], + }, + { + artist => 1, cdid => 2, genreid => undef, single_track => undef, title => "Forkful of bees", year => 2001, tracks => [ + { cd => 2, last_updated_at => undef, last_updated_on => undef, position => 1, title => "Stung with Success", trackid => 4 }, + { cd => 2, last_updated_at => undef, last_updated_on => undef, position => 2, title => "Stripy", trackid => 5 }, + { cd => 2, last_updated_at => undef, last_updated_on => undef, position => 3, title => "Sticky Honey", trackid => 6 }, + ], + }, + { + artist => 2, cdid => 4, genreid => undef, single_track => undef, title => "Generic Manufactured Singles", year => 2001, tracks => [ + { cd => 4, last_updated_at => undef, last_updated_on => undef, position => 3, title => "No More Ideas", trackid => 12 }, + { cd => 4, last_updated_at => undef, last_updated_on => undef, position => 2, title => "Boring Song", trackid => 11 }, + { cd => 4, last_updated_at => undef, last_updated_on => undef, position => 1, title => "Boring Name", trackid => 10}, + ], + }, +]; -# note: we only requested "get all cds of all artists with rank 13 then order -# by the artist name and give me the fourth, fifth and sixth", consequently the -# cds that belong to the same artist are unordered; fortunately we know that -# the first artist have 3 cds and the second and third artist both have only -# one, so the first 3 cds belong to the first artist and the fourth and fifth -# cds belong to the second and third artist, respectively, and there's no sixth -# row -is_deeply ( +is_deeply( $filtered_cd_rs->all_hri, - [ - { - 'artist' => '2', - 'cdid' => '4', - 'genreid' => undef, - 'single_track' => undef, - 'title' => 'Generic Manufactured Singles', - 'tracks' => [ - { - 'cd' => '4', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '3', - 'title' => 'No More Ideas', - 'trackid' => '12' - }, - { - 'cd' => '4', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '2', - 'title' => 'Boring Song', - 'trackid' => '11' - }, - { - 'cd' => '4', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '1', - 'title' => 'Boring Name', - 'trackid' => '10' - } - ], - 'year' => '2001' - }, - { - 'artist' => '3', - 'cdid' => '5', - 'genreid' => undef, - 'single_track' => undef, - 'title' => 'Come Be Depressed With Us', - 'tracks' => [ - { - 'cd' => '5', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '3', - 'title' => 'Suicidal', - 'trackid' => '15' - }, - { - 'cd' => '5', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '2', - 'title' => 'Under The Weather', - 'trackid' => '14' - }, - { - 'cd' => '5', - 'last_updated_at' => undef, - 'last_updated_on' => undef, - 'position' => '1', - 'title' => 'Sad', - 'trackid' => '13' - } - ], - 'year' => '1998' - } - ], - 'Correctly ordered result', + $hri_contents, + 'Expected ordered unlimited contents', ); +for ( + [ 0, 1 ], + [ 2, 0 ], + [ 20, 2 ], + [ 1, 3 ], + [ 2, 4 ], +) { + my ($limit, $offset) = @$_; + + my $rs = $filtered_cd_rs->search({}, { $limit ? (rows => $limit) : (), offset => $offset }); + + my $used_limit = $limit || DBIx::Class::SQLMaker->__max_int; + my $offset_str = $offset ? 'OFFSET ?' : ''; + + is_same_sql_bind( + $rs->as_query, + "( + SELECT cds_unordered.cdid, cds_unordered.artist, cds_unordered.title, cds_unordered.year, cds_unordered.genreid, cds_unordered.single_track, + tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at + FROM artist me + JOIN ( + SELECT cds_unordered.cdid, cds_unordered.artist, cds_unordered.title, cds_unordered.year, cds_unordered.genreid, cds_unordered.single_track + FROM artist me + JOIN cd cds_unordered + ON cds_unordered.artist = me.artistid + LEFT JOIN genre genre + ON genre.genreid = cds_unordered.genreid + LEFT JOIN track tracks + ON tracks.cd = cds_unordered.cdid + WHERE ( me.rank = ? ) + GROUP BY cds_unordered.cdid, cds_unordered.artist, cds_unordered.title, cds_unordered.year, cds_unordered.genreid, cds_unordered.single_track + ORDER BY MAX(genre.name) DESC, MAX(tracks.title) DESC, MIN(me.name), cds_unordered.title DESC + LIMIT ? + $offset_str + ) cds_unordered + ON cds_unordered.artist = me.artistid + LEFT JOIN genre genre + ON genre.genreid = cds_unordered.genreid + LEFT JOIN track tracks + ON tracks.cd = cds_unordered.cdid + WHERE ( me.rank = ? ) + ORDER BY genre.name DESC, tracks.title DESC, me.name ASC, cds_unordered.title DESC + )", + [ + [ { sqlt_datatype => 'integer', dbic_colname => 'me.rank' } => 13 ], + [ $ROWS => $used_limit ], + $offset ? [ $OFFSET => $offset ] : (), + [ { sqlt_datatype => 'integer', dbic_colname => 'me.rank' } => 13 ], + ], + "correct SQL on prefetch over search_related ordered by external joins with limit '$limit', offset '$offset'", + ); + + is_deeply( + $rs->all_hri, + [ @{$hri_contents}[$offset .. List::Util::min( $used_limit+$offset-1, $#$hri_contents)] ], + "Correct slice of the resultset returned with limit '$limit', offset '$offset'", + ); +} + done_testing; diff --git a/t/prefetch/with_limit.t b/t/prefetch/with_limit.t index 97dffcc..7f633be 100644 --- a/t/prefetch/with_limit.t +++ b/t/prefetch/with_limit.t @@ -80,7 +80,6 @@ is_same_sql_bind ( ON tracks.cd = cds.cdid WHERE artwork.cd_id IS NULL OR tracks.title != ? - GROUP BY me.artistid + ?, me.artistid, me.name, cds.cdid, cds.artist, cds.title, cds.year, cds.genreid, cds.single_track ORDER BY name DESC )', [ @@ -90,7 +89,6 @@ is_same_sql_bind ( $bind_int_resolved->(), # inner group_by [ $ROWS => 3 ], $bind_vc_resolved->(), # outer where - $bind_int_resolved->(), # outer group_by ], 'Expected SQL on complex limited prefetch' ); diff --git a/t/sqlmaker/limit_dialects/torture.t b/t/sqlmaker/limit_dialects/torture.t index 072e9c6..d7a4254 100644 --- a/t/sqlmaker/limit_dialects/torture.t +++ b/t/sqlmaker/limit_dialects/torture.t @@ -39,7 +39,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? LIMIT ? @@ -81,7 +81,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? LIMIT ?, ? @@ -122,7 +122,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? )', @@ -161,7 +161,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? )', @@ -203,7 +203,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me ) me @@ -221,7 +221,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me ) me @@ -305,7 +305,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? %s ) me @@ -334,7 +334,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me ) me @@ -370,7 +370,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? ) me @@ -420,7 +420,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? FETCH FIRST 4 ROWS ONLY )', @@ -440,7 +440,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY me.id FETCH FIRST 7 ROWS ONLY @@ -462,7 +462,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? FETCH FIRST 4 ROWS ONLY @@ -486,7 +486,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? FETCH FIRST 7 ROWS ONLY @@ -534,7 +534,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? )', [ @@ -553,7 +553,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY me.id ) me @@ -573,7 +573,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? )', @@ -596,7 +596,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY ? / ?, ? ) me @@ -641,7 +641,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg(me.id / ?) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ORDER BY me.id SET ROWCOUNT 0 @@ -662,7 +662,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg( me.id / ? ) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me WHERE ( @@ -693,7 +693,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg( me.id / ? ) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me WHERE ( @@ -720,7 +720,7 @@ my $tests = { JOIN owners owner ON owner.id = me.owner WHERE source != ? AND me.title = ? AND source = ? - GROUP BY avg( me.id / ? ) + GROUP BY AVG(me.id / ?), MAX(owner.id) HAVING ? ) me WHERE ( @@ -779,7 +779,7 @@ for my $limtype (sort keys %$tests) { join => 'owner', # single-rel manual prefetch rows => 4, '+columns' => { bar => \['? * ?', [ $attr => 11 ], [ $attr => 12 ]], baz => \[ '?', [ $attr => 13 ]] }, - group_by => \[ 'avg(me.id / ?)', [ $attr => 21 ] ], + group_by => \[ 'AVG(me.id / ?), MAX(owner.id)', [ $attr => 21 ] ], having => \[ '?', [ $attr => 31 ] ], ($limtype =~ /GenericSubQ/ ? ( order_by => 'me.id' ) : () ), # needs a simple-column stable order to be happy });