Skip to content

Commit

Permalink
Fix and test first_skip/skip_first limit dialects
Browse files Browse the repository at this point in the history
Codebase didn't take in account the fact that limit bindvals for
SELECT FIRST x SKIP y and SELECT FIRST x SKIP y need to be inserted
at the head of the bind-assembly chain. Hence introducing a new
bind-chunk position 'pre_select'. While at it move the TOP dialect
to use it as well.

Extensive tests for both dialects, and also augment the test for
the Oracle RowNum dialect (no fixes necessary)
  • Loading branch information
ribasushi committed Dec 5, 2011
1 parent 10419db commit 8b31f62
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 16 deletions.
3 changes: 3 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
Revision history for DBIx::Class

* Fixes
- Fix SkipFirst and FirstSkip limit dialects (Informix and Firebird)

0.08196 2011-11-29 05:35 (UTC)
* Fixes
- Fix tests for DBD::SQLite >= 1.34.
Expand Down
2 changes: 1 addition & 1 deletion lib/DBIx/Class/SQLMaker.pm
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ sub select {

sub _assemble_binds {
my $self = shift;
return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/select from where group having order limit/);
return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/pre_select select from where group having order limit/);
}

my $for_syntax = {
Expand Down
16 changes: 7 additions & 9 deletions lib/DBIx/Class/SQLMaker/LimitDialects.pm
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ sub _SkipFirst {
return sprintf ('SELECT %s%s%s%s',
$offset
? do {
push @{$self->{limit_bind}}, [ $self->__offset_bindtype => $offset];
push @{$self->{pre_select_bind}}, [ $self->__offset_bindtype => $offset];
'SKIP ? '
}
: ''
,
do {
push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ];
push @{$self->{pre_select_bind}}, [ $self->__rows_bindtype => $rows ];
'FIRST ? '
},
$sql,
Expand All @@ -199,12 +199,12 @@ sub _FirstSkip {

return sprintf ('SELECT %s%s%s%s',
do {
push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ];
push @{$self->{pre_select_bind}}, [ $self->__rows_bindtype => $rows ];
'FIRST ? '
},
$offset
? do {
push @{$self->{limit_bind}}, [ $self->__offset_bindtype => $offset];
push @{$self->{pre_select_bind}}, [ $self->__offset_bindtype => $offset];
'SKIP ? '
}
: ''
Expand Down Expand Up @@ -390,11 +390,9 @@ sub _prep_for_skimming_limit {
$r{mid_sel} .= ', ' . $extra_order_sel->{$extra_col};
}

# since whatever order bindvals there are, they will be realiased
# and need to show up in front of the entire initial inner subquery
# *unshift* the selector bind stack to make this happen (horrible,
# horrible, but we don't have another mechanism yet)
unshift @{$self->{select_bind}}, @{$self->{order_bind}};
# Whatever order bindvals there are, they will be realiased and
# need to show up in front of the entire initial inner subquery
push @{$self->{pre_select_bind}}, @{$self->{order_bind}};
}

# and this is order re-alias magic
Expand Down
2 changes: 1 addition & 1 deletion lib/DBIx/Class/SQLMaker/Oracle.pm
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ sub new {

sub _assemble_binds {
my $self = shift;
return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/select from where oracle_connect_by group having order limit/);
return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/pre_select select from where oracle_connect_by group having order limit/);
}


Expand Down
151 changes: 151 additions & 0 deletions t/sqlmaker/limit_dialects/first_skip.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
use strict;
use warnings;

use Test::More;
use lib qw(t/lib);
use DBICTest;
use DBIC::SqlMakerTest;
use DBIx::Class::SQLMaker::LimitDialects;

my ($LIMIT, $OFFSET) = (
DBIx::Class::SQLMaker::LimitDialects->__rows_bindtype,
DBIx::Class::SQLMaker::LimitDialects->__offset_bindtype,
);

my $schema = DBICTest->init_schema;

$schema->storage->_sql_maker->limit_dialect ('FirstSkip');

my $rs_selectas_col = $schema->resultset ('BooksInLibrary')->search ({}, {
'+select' => ['owner.name'],
'+as' => ['owner.name'],
join => 'owner',
rows => 1,
offset => 2,
});

is_same_sql_bind(
$rs_selectas_col->as_query,
'(
SELECT FIRST ? SKIP ? me.id, me.source, me.owner, me.title, me.price, owner.name
FROM books me
JOIN owners owner ON owner.id = me.owner
WHERE ( source = ? )
)',
[
[ $LIMIT => 1 ],
[ $OFFSET => 2 ],
[ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ],
],
);

$schema->storage->_sql_maker->quote_char ([qw/ [ ] /]);
$schema->storage->_sql_maker->name_sep ('.');

my $rs_selectas_rel = $schema->resultset ('BooksInLibrary')->search ({}, {
'+select' => ['owner.name'],
'+as' => ['owner_name'],
join => 'owner',
rows => 1,
offset => 2,
});

is_same_sql_bind(
$rs_selectas_rel->as_query,
'(
SELECT FIRST ? SKIP ? [me].[id], [me].[source], [me].[owner], [me].[title], [me].[price], [owner].[name]
FROM [books] [me]
JOIN [owners] [owner] ON [owner].[id] = [me].[owner]
WHERE ( [source] = ? )
)',
[
[ $LIMIT => 1 ],
[ $OFFSET => 2 ],
[ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ],
],
);

{
my $subq = $schema->resultset('Owners')->search({
'count.id' => { -ident => 'owner.id' },
'count.name' => 'fail', # no one would do this in real life, the rows makes even less sense
}, { alias => 'owner', rows => 1 })->count_rs;

my $rs_selectas_rel = $schema->resultset('BooksInLibrary')->search ({}, {
columns => [
{ owner_name => 'owner.name' },
{ owner_books => $subq->as_query },
],
join => 'owner',
rows => 1,
offset => 2,
});

is_same_sql_bind(
$rs_selectas_rel->as_query,
'(
SELECT FIRST ? SKIP ?
[owner].[name],
( SELECT COUNT(*) FROM
( SELECT FIRST ? [owner].[id] FROM [owners] [owner]
WHERE [count].[id] = [owner].[id] and [count].[name] = ?
) [owner]
)
FROM [books] [me]
JOIN [owners] [owner] ON [owner].[id] = [me].[owner]
WHERE ( [source] = ? )
)',
[
[ $LIMIT => 1 ], # outer
[ $OFFSET => 2 ], # outer
[ {%$LIMIT} => 1 ], # inner
[ { dbic_colname => 'count.name' } => 'fail' ],
[ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ],
],
)
};

{
my $rs = $schema->resultset('Artist')->search({}, {
columns => 'name',
offset => 1,
order_by => 'name',
});
local $rs->result_source->{name} = "weird \n newline/multi \t \t space containing \n table";

like (
${$rs->as_query}->[0],
qr| weird \s \n \s newline/multi \s \t \s \t \s space \s containing \s \n \s table|x,
'Newlines/spaces preserved in final sql',
);
}

{
my $subq = $schema->resultset('Owners')->search({
'books.owner' => { -ident => 'owner.id' },
}, { alias => 'owner', select => ['id'], offset => 3, rows => 4 });

my $rs_selectas_rel = $schema->resultset('BooksInLibrary')->search( { -exists => $subq->as_query }, { select => ['id','owner'], rows => 1, offset => 2 } );

is_same_sql_bind(
$rs_selectas_rel->as_query,
'(
SELECT FIRST ? SKIP ? [me].[id], [me].[owner]
FROM [books] [me]
WHERE ( ( (EXISTS (
SELECT FIRST ? SKIP ? [owner].[id] FROM [owners] [owner] WHERE ( [books].[owner] = [owner].[id] )
)) AND [source] = ? ) )
)',
[
[ $LIMIT => 1 ], #outer
[ $OFFSET => 2 ], #outer
[ {%$LIMIT} => 4 ], #inner
[ {%$OFFSET} => 3 ], #inner
[ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ],
],
'Pagination with sub-query in WHERE works'
);

}

done_testing;
20 changes: 15 additions & 5 deletions t/sqlmaker/limit_dialects/rownum.t
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ my ($TOTAL, $OFFSET) = (
my $s = DBICTest->init_schema (no_deploy => 1, );
$s->storage->sql_maker->limit_dialect ('RowNum');

my $rs = $s->resultset ('CD');
my $rs = $s->resultset ('CD')->search({ id => 1 });

my $where_bind = [ { dbic_colname => 'id' }, 1 ];

for my $test_set (
{
Expand All @@ -36,11 +38,13 @@ for my $test_set (
SELECT id, bar__id, bleh, ROWNUM rownum__index
FROM (
SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR (foo.womble, "blah") AS bleh
FROM cd me
FROM cd me
WHERE id = ?
) me
) me WHERE rownum__index BETWEEN ? AND ?
)',
binds => [
$where_bind,
[ $OFFSET => 4 ],
[ $TOTAL => 4 ],
],
Expand All @@ -62,14 +66,16 @@ for my $test_set (
SELECT id, bar__id, bleh, ROWNUM rownum__index
FROM (
SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR(foo.womble, "blah") AS bleh
FROM cd me
FROM cd me
WHERE id = ?
ORDER BY artist, title
) me
WHERE ROWNUM <= ?
) me
WHERE rownum__index >= ?
)',
binds => [
$where_bind,
[ $TOTAL => 4 ],
[ $OFFSET => 4 ],
],
Expand All @@ -89,11 +95,13 @@ for my $test_set (
SELECT id, ends_with_me__id, ROWNUM rownum__index
FROM (
SELECT foo.id AS id, ends_with_me.id AS ends_with_me__id
FROM cd me
FROM cd me
WHERE id = ?
) me
) me WHERE rownum__index BETWEEN ? AND ?
)',
binds => [
$where_bind,
[ $OFFSET => 4 ],
[ $TOTAL => 5 ],
],
Expand All @@ -114,14 +122,16 @@ for my $test_set (
SELECT id, ends_with_me__id, ROWNUM rownum__index
FROM (
SELECT foo.id AS id, ends_with_me.id AS ends_with_me__id
FROM cd me
FROM cd me
WHERE id = ?
ORDER BY artist, title
) me
WHERE ROWNUM <= ?
) me
WHERE rownum__index >= ?
)',
binds => [
$where_bind,
[ $TOTAL => 5 ],
[ $OFFSET => 4 ],
],
Expand Down
Loading

0 comments on commit 8b31f62

Please sign in to comment.