sets quote_char and name_sep appropriate for your RDBMS
- Add retrieve_on_insert column info flag, allowing to retrieve any
column value instead of just autoinc primary keys
+ - Bring back strict ordering of selectors in complex search chains
+ (an ill-fated attempt was made in 0.08127 to order intelligently)
- All limit dialects (except for the older Top and FetchFirst) are
now using bind parameters for the limits/offsets, making DBI's
prepare_cached useful across paged resutsets
attrs => $attrs,
}, $class;
+ # if there is a dark selector, this means we are already in a
+ # chain and the cleanup/sanification was taken care of by
+ # _search_rs already
+ $self->_normalize_selection($attrs)
+ unless $attrs->{_dark_selector};
+
$self->result_class(
$attrs->{result_class} || $source->result_class
);
# take care of call attrs (only if anything is changing)
if (keys %$call_attrs) {
- $self->throw_exception ('_trailing_select is not a public attribute - do not use it in search()')
- if ( exists $call_attrs->{_trailing_select} or exists $call_attrs->{'+_trailing_select'} );
+ my @selector_attrs = qw/select as columns cols +select +as +columns include_columns/;
- my @selector_attrs = qw/select as columns cols +select +as +columns include_columns _trailing_select +_trailing_select/;
+ # reset the current selector list if new selectors are supplied
+ if (List::Util::first { exists $call_attrs->{$_} } qw/columns cols select as/) {
+ delete @{$old_attrs}{(@selector_attrs, '_dark_selector')};
+ }
- # Normalize the selector list (operates on the passed-in attr structure)
+ # Normalize the new selector list (operates on the passed-in attr structure)
# Need to do it on every chain instead of only once on _resolved_attrs, in
- # order to separate 'as'-ed from blind 'select's
+ # order to allow detection of empty vs partial 'as'
+ $call_attrs->{_dark_selector} = $old_attrs->{_dark_selector}
+ if $old_attrs->{_dark_selector};
$self->_normalize_selection ($call_attrs);
# start with blind overwriting merge, exclude selector attrs
$new_attrs = { %{$old_attrs}, %{$call_attrs} };
delete @{$new_attrs}{@selector_attrs};
- # reset the current selector list if new selectors are supplied
- if (List::Util::first { exists $call_attrs->{$_} } qw/columns cols select as/) {
- delete @{$old_attrs}{@selector_attrs};
- }
-
for (@selector_attrs) {
$new_attrs->{$_} = $self->_merge_attr($old_attrs->{$_}, $call_attrs->{$_})
if ( exists $old_attrs->{$_} or exists $call_attrs->{$_} );
return $rs;
}
+my $dark_sel_dumper;
sub _normalize_selection {
my ($self, $attrs) = @_;
$attrs->{'+columns'} = $self->_merge_attr($attrs->{'+columns'}, delete $attrs->{include_columns})
if exists $attrs->{include_columns};
+ # columns are always placed first, however
+
# Keep the X vs +X separation until _resolved_attrs time - this allows to
# delay the decision on whether to use a default select list ($rsrc->columns)
# allowing stuff like the remove_columns helper to work
# supplied at all) - try to infer the alias, either from the -as parameter
# of the selector spec, or use the parameter whole if it looks like a column
# name (ugly legacy heuristic). If all fails - leave the selector bare (which
- # is ok as well), but transport it over a separate attribute to make sure it is
- # the last thing in the select list, thus unable to throw off the corresponding
- # 'as' chain
+ # is ok as well), but make sure no more additions to the 'as' chain take place
for my $pref ('', '+') {
my ($sel, $as) = map {
);
}
elsif( ! @$as ) {
- # no as part supplied at all - try to deduce
+ # no as part supplied at all - try to deduce (unless explicit end of named selection is declared)
# if any @$as has been supplied we assume the user knows what (s)he is doing
# and blindly keep stacking up pieces
- my (@new_sel, @new_trailing);
- for (@$sel) {
- if ( ref $_ eq 'HASH' and exists $_->{-as} ) {
- push @$as, $_->{-as};
- push @new_sel, $_;
- }
- # assume any plain no-space, no-parenthesis string to be a column spec
- # FIXME - this is retarded but is necessary to support shit like 'count(foo)'
- elsif ( ! ref $_ and $_ =~ /^ [^\s\(\)]+ $/x) {
- push @$as, $_;
- push @new_sel, $_;
- }
- # if all else fails - shove the selection to the trailing stack and move on
- else {
- push @new_trailing, $_;
+ unless ($attrs->{_dark_selector}) {
+ SELECTOR:
+ for (@$sel) {
+ if ( ref $_ eq 'HASH' and exists $_->{-as} ) {
+ push @$as, $_->{-as};
+ }
+ # assume any plain no-space, no-parenthesis string to be a column spec
+ # FIXME - this is retarded but is necessary to support shit like 'count(foo)'
+ elsif ( ! ref $_ and $_ =~ /^ [^\s\(\)]+ $/x) {
+ push @$as, $_;
+ }
+ # if all else fails - raise a flag that no more aliasing will be allowed
+ else {
+ $attrs->{_dark_selector} = {
+ plus_stage => $pref,
+ string => ($dark_sel_dumper ||= do {
+ require Data::Dumper::Concise;
+ Data::Dumper::Concise::DumperObject()->Indent(0);
+ })->Values([$_])->Dump
+ ,
+ };
+ last SELECTOR;
+ }
}
}
-
- @$sel = @new_sel;
- $attrs->{"${pref}_trailing_select"} = $self->_merge_attr($attrs->{"${pref}_trailing_select"}, \@new_trailing)
- if @new_trailing;
}
elsif (@$as < @$sel) {
$self->throw_exception(
"Unable to handle an ${pref}as specification (@$as) with less elements than the corresponding ${pref}select"
);
}
-
- # now see what the result for this pair looks like:
- if (@$as == @$sel) {
-
- # if balanced - treat as a columns entry
- $attrs->{"${pref}columns"} = $self->_merge_attr(
- $attrs->{"${pref}columns"},
- [ map { +{ $as->[$_] => $sel->[$_] } } ( 0 .. $#$as ) ]
+ elsif ($pref and $attrs->{_dark_selector}) {
+ $self->throw_exception(
+ "Unable to process named '+select', resultset contains an unnamed selector $attrs->{_dark_selector}{string}"
);
}
- else {
- # unbalanced - shove in select/as, not subject to deduplication in _resolved_attrs
- $attrs->{"${pref}select"} = $self->_merge_attr($attrs->{"${pref}select"}, $sel);
- $attrs->{"${pref}as"} = $self->_merge_attr($attrs->{"${pref}as"}, $as);
- }
- }
+
+ # merge result
+ $attrs->{"${pref}select"} = $self->_merge_attr($attrs->{"${pref}select"}, $sel);
+ $attrs->{"${pref}as"} = $self->_merge_attr($attrs->{"${pref}as"}, $as);
+ }
}
sub _stack_cond {
# overwrite the selector (supplied by the storage)
$tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $attrs);
$tmp_attrs->{as} = 'count';
- delete @{$tmp_attrs}{qw/columns _trailing_select/};
+ delete @{$tmp_attrs}{qw/columns/};
my $tmp_rs = $rsrc->resultset_class->new($rsrc, $tmp_attrs)->get_column ('count');
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 _trailing_select order_by for/};
+ delete @{$sub_attrs}{qw/collapse columns as select _prefetch_selector_range order_by for/};
# if we multi-prefetch we group_by primary keys only as this is what we would
# get out of the rs via ->next/->all. We *DO WANT* to clobber old group_by regardless
my $source = $self->result_source;
my $alias = $attrs->{alias};
- # one last pass of normalization
- $self->_normalize_selection($attrs);
-
# default selection list
$attrs->{columns} = [ $source->columns ]
- unless List::Util::first { exists $attrs->{$_} } qw/columns cols select as _trailing_select/;
+ unless List::Util::first { exists $attrs->{$_} } qw/columns cols select as/;
# merge selectors together
- for (qw/columns select as _trailing_select/) {
- $attrs->{$_} = $self->_merge_attr($attrs->{$_}, $attrs->{"+$_"})
+ for (qw/columns select as/) {
+ $attrs->{$_} = $self->_merge_attr($attrs->{$_}, delete $attrs->{"+$_"})
if $attrs->{$_} or $attrs->{"+$_"};
}
}
else {
# distinct affects only the main selection part, not what prefetch may
- # add below. However trailing is not yet a part of the selection as
- # prefetch must insert before it
+ # add below.
$attrs->{group_by} = $source->storage->_group_over_selection (
$attrs->{from},
- [ @{$attrs->{select}||[]}, @{$attrs->{_trailing_select}||[]} ],
+ $attrs->{select},
$attrs->{order_by},
);
}
$attrs->{collapse} ||= {};
if ($attrs->{prefetch}) {
+
+ $self->throw_exception("Unable to prefetch, resultset contains an unnamed selector $attrs->{_dark_selector}{string}")
+ if $attrs->{_dark_selector};
+
my $prefetch = $self->_merge_joinpref_attr( {}, delete $attrs->{prefetch} );
my $prefetch_ordering = [];
}
- push @{ $attrs->{select} }, @{$attrs->{_trailing_select}}
- if $attrs->{_trailing_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
$schema->storage->debug($orig_debug);
$schema->storage->debugcb(undef);
-# try to unbalance the select
-
-# first add a lone non-as-ed select
-# it should be reordered to appear at the end without throwing prefetch/bind off
-$c_rs = $c_rs->search({}, { '+select' => \[ 'me.cdid + ?', [ \ 'inTEger' => 1 ] ] });
-
# now add an unbalanced select/as pair
$c_rs = $c_rs->search ({}, {
'+select' => $cdrs->search(
'+as' => [qw/active_from active_to/],
});
-
is_same_sql_bind(
$c_rs->as_query,
'(
WHERE siblings.artist = me.artist
AND me.artist != ?
),
- tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at,
- me.cdid + ?
+ tracks.trackid, tracks.cd, tracks.position, tracks.title, tracks.last_updated_on, tracks.last_updated_at
FROM cd me
LEFT JOIN track tracks
ON tracks.cd = me.cdid
[ { sqlt_datatype => 'integer', dbic_colname => 'me.artist' }
=> 2 ],
- # the addition
- [ { sqlt_datatype => 'inTEger' }
- => 1 ],
-
# outher WHERE
[ { sqlt_datatype => 'integer', dbic_colname => 'me.artist' }
=> 2 ],
}
);
-# add a floating +select to make sure it does not throw things off
+# add an extra +select to make sure it does not throw things off
# we also expect it to appear in both selectors, as we can not know
# for sure which part of the query it applies to (may be order_by,
# maybe something else)
# is_deeply picks up this difference too (not sure if bug or
# feature)
$use_prefetch = $use_prefetch->search({}, {
- '+select' => \[ 'me.artistid + ?', [ \ 'inTEger' => 1 ] ],
+ '+columns' => { monkeywrench => \[ 'me.artistid + ?', [ \ 'inTEger' => 1 ] ] },
});
my $bind_int_resolved = sub { [ { sqlt_datatype => 'inTEger' } => 1 ] };
is_same_sql_bind (
$use_prefetch->as_query,
'(
- SELECT me.artistid, me.name,
- cds.cdid, cds.artist, cds.title, cds.year, cds.genreid, cds.single_track,
- me.artistid + ?
+ SELECT me.artistid + ?,
+ me.artistid, me.name,
+ cds.cdid, cds.artist, cds.title, cds.year, cds.genreid, cds.single_track
FROM (
- SELECT me.artistid, me.name,
- me.artistid + ?
+ SELECT me.artistid + ?,
+ me.artistid, me.name
FROM artist me
LEFT JOIN cd cds
ON cds.artist = me.artistid
ON tracks.cd = cds.cdid
WHERE artwork.cd_id IS NULL
OR tracks.title != ?
- GROUP BY me.artistid, me.name, me.artistid + ?
+ GROUP BY me.artistid + ?, me.artistid, me.name
ORDER BY name DESC LIMIT ?
) me
LEFT JOIN cd cds
ON tracks.cd = cds.cdid
WHERE artwork.cd_id IS NULL
OR tracks.title != ?
- GROUP BY me.artistid, me.name, cds.cdid, cds.artist, cds.title, cds.year, cds.genreid, cds.single_track, me.artistid + ?
+ 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, cds.artist, cds.year ASC
)',
[
'+columns' => [ { max_year => { max => 'me.year', -as => 'last_y' }}, ],
'+select' => [ { count => 'me.cdid' }, ],
'+as' => [ 'cnt' ],
- } => 'SELECT me.cdid, LOWER( title ) AS lctitle, me.genreid, MAX( me.year ) AS last_y, COUNT( me.cdid ) FROM cd me',
+ } => 'SELECT me.cdid, LOWER( title ) AS lctitle, MAX( me.year ) AS last_y, me.genreid, COUNT( me.cdid ) FROM cd me',
{
select => [ { min => 'me.cdid' }, ],
{
'+columns' => [ { cnt => { count => 'cdid', -as => 'cnt' } } ],
- } => 'SELECT MIN( me.cdid ), COUNT ( cdid ) AS cnt FROM cd me',
+ } => 'SELECT COUNT ( cdid ) AS cnt, MIN( me.cdid ) FROM cd me',
{
columns => [ { foo => { coalesce => [qw/a b c/], -as => 'firstfound' } } ],
{
'+select' => [ 'me.year' ],
'+as' => [ 'year' ],
- } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt FROM cd me',
+ } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt, me.year FROM cd me',
{
'+columns' => [ 'me.year' ],
- } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt FROM cd me',
+ } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt, me.year FROM cd me',
{
'+columns' => 'me.year',
- } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt FROM cd me',
+ } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt, me.year FROM cd me',
+
+ # naked selector at the end should just work
+ {
+ '+select' => 'me.moar_stuff',
+ } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt, me.year, me.moar_stuff FROM cd me',
+
+ {
+ '+select' => [ { MOAR => 'f', -as => 'func' } ],
+ } => 'SELECT COALESCE( a, b, c ) AS firstfound, me.year, MAX( me.year ) AS last_y, COUNT( me.cdid ) AS cnt, me.year, me.moar_stuff, MOAR(f) AS func FROM cd me',
);
# Make sure we don't lose bits even with weird selector specs
# also check that the default selector list is lazy
+# and make sure that unaliased +select does not go crazy
$rs = $schema->resultset('CD');
for my $attr (
{ '+columns' => [ 'me.title' ] }, # this one should be de-duplicated but not the select's
me.year,
me.genreid,
me.single_track,
- COUNT( artistid ) AS baz,
me.year AS foo,
me.year AS foo,
- me.artistid AS bar
+ me.artistid AS bar,
+ COUNT( artistid ) AS baz
FROM cd me
)',
[],
'Correct order of selected columns'
);
+# Test bare +select with as from root of resultset
+$rs = $schema->resultset('CD')->search ({}, {
+ '+select' => [
+ \ 'foo',
+ { MOAR => 'f', -as => 'func' },
+ ],
+});
+
+is_same_sql_bind (
+ $rs->as_query,
+ '( SELECT
+ me.cdid,
+ me.artist,
+ me.title,
+ me.year,
+ me.genreid,
+ me.single_track,
+ foo,
+ MOAR( f ) AS func
+ FROM cd me
+ )',
+ [],
+ 'Correct order of selected columns'
+);
+
done_testing;
use warnings;
use Test::More;
+use Test::Exception;
use lib qw(t/lib);
use DBIC::SqlMakerTest;
my @chain = (
{
- select => 'title',
- as => 'title',
- columns => [ 'cdid' ],
+ select => 'cdid',
+ as => 'cd_id',
+ columns => [ 'title' ],
} => 'SELECT
- me.cdid,
- me.title
+ me.title,
+ me.cdid
FROM cd me'
- => [qw/cdid title/],
+ => [qw/title cd_id/],
{
'+select' => \ 'DISTINCT(foo, bar)',
'+as' => [qw/foo bar/],
} => 'SELECT
- me.cdid,
me.title,
+ me.cdid,
DISTINCT(foo, bar)
FROM cd me'
- => [qw/cdid title foo bar/],
+ => [qw/title cd_id foo bar/],
{
- '+select' => \'unaliased randomness',
- } => 'SELECT
- me.cdid,
- me.title,
- DISTINCT(foo, bar),
- unaliased randomness
- FROM cd me'
- => [qw/cdid title foo bar/],
- {
'+select' => [ 'genreid', $multicol_rs->as_query ],
'+as' => [qw/genreid name rank/],
} => 'SELECT
- me.cdid,
me.title,
+ me.cdid,
DISTINCT(foo, bar),
me.genreid,
(SELECT me.name, me.rank FROM artist me WHERE ( artistid 1 )),
- unaliased randomness
FROM cd me'
- => [qw/cdid title foo bar genreid name rank/],
+ => [qw/title cd_id foo bar genreid name rank/],
{
'+select' => { count => 'me.cdid', -as => 'cnt' }, # lack of 'as' infers from '-as'
'+columns' => { len => { length => 'me.title' } },
} => 'SELECT
- me.cdid,
me.title,
LENGTH( me.title ),
+ me.cdid,
+ DISTINCT(foo, bar),
+ me.genreid,
+ (SELECT me.name, me.rank FROM artist me WHERE ( artistid 1 )),
COUNT( me.cdid ) AS cnt,
+ FROM cd me'
+ => [qw/title len cd_id foo bar genreid name rank cnt/],
+ {
+ '+select' => \'unaliased randomness',
+ } => 'SELECT
+ me.title,
+ LENGTH( me.title ),
+ me.cdid,
DISTINCT(foo, bar),
me.genreid,
(SELECT me.name, me.rank FROM artist me WHERE ( artistid 1 )),
+ COUNT( me.cdid ) AS cnt,
unaliased randomness
FROM cd me'
- => [qw/cdid title len cnt foo bar genreid name rank/],
-
-
+ => [qw/title len cd_id foo bar genreid name rank cnt/],
+ {
+ '+select' => \'MOAR unaliased randomness',
+ } => 'SELECT
+ me.title,
+ LENGTH( me.title ),
+ me.cdid,
+ DISTINCT(foo, bar),
+ me.genreid,
+ (SELECT me.name, me.rank FROM artist me WHERE ( artistid 1 )),
+ COUNT( me.cdid ) AS cnt,
+ unaliased randomness,
+ MOAR unaliased randomness
+ FROM cd me'
+ => [qw/title len cd_id foo bar genreid name rank cnt/],
);
my $rs = $schema->resultset('CD');
$testno++;
}
+# make sure proper exceptions are thrown on unbalanced use
+{
+ my $rs = $schema->resultset('CD')->search({}, { select => \'count(me.cdid)'});
+
+ lives_ok(sub {
+ $rs->search({}, { '+select' => 'me.cdid' })->next
+ }, 'Two dark selectors are ok');
+
+ throws_ok(sub {
+ $rs->search({}, { '+select' => 'me.cdid', '+as' => 'cdid' })->next
+ }, qr/resultset contains an unnamed selector/, 'Unnamed followed by named is not');
+
+ throws_ok(sub {
+ $rs->search_rs({}, { prefetch => 'tracks' })->next
+ }, qr/resultset contains an unnamed selector/, 'Throw on unaliased selector followed by prefetch');
+
+ throws_ok(sub {
+ $rs->search_rs({}, { '+select' => 'me.title', '+as' => 'title' })->next
+ }, qr/resultset contains an unnamed selector/, 'Throw on unaliased selector followed by +select/+as');
+}
+
+
done_testing;
projects / dbsrgits/DBIx-Class.git / commitdiff