From: Peter Rabbitson Date: Thu, 1 Dec 2011 16:50:57 +0000 (+0100) Subject: Fix and test first_skip/skip_first limit dialects X-Git-Tag: v0.08197~139 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=8b31f62e2b395fba451c32e0bbfecbd3e0f673f8;p=dbsrgits%2FDBIx-Class.git Fix and test first_skip/skip_first limit dialects 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) --- diff --git a/Changes b/Changes index 1590ce6..7076eb0 100644 --- a/Changes +++ b/Changes @@ -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. diff --git a/lib/DBIx/Class/SQLMaker.pm b/lib/DBIx/Class/SQLMaker.pm index fd42594..705c569 100644 --- a/lib/DBIx/Class/SQLMaker.pm +++ b/lib/DBIx/Class/SQLMaker.pm @@ -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 = { diff --git a/lib/DBIx/Class/SQLMaker/LimitDialects.pm b/lib/DBIx/Class/SQLMaker/LimitDialects.pm index 56f2d53..8e85ac8 100644 --- a/lib/DBIx/Class/SQLMaker/LimitDialects.pm +++ b/lib/DBIx/Class/SQLMaker/LimitDialects.pm @@ -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, @@ -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 ? ' } : '' @@ -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 diff --git a/lib/DBIx/Class/SQLMaker/Oracle.pm b/lib/DBIx/Class/SQLMaker/Oracle.pm index d144113..7548c2a 100644 --- a/lib/DBIx/Class/SQLMaker/Oracle.pm +++ b/lib/DBIx/Class/SQLMaker/Oracle.pm @@ -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/); } diff --git a/t/sqlmaker/limit_dialects/first_skip.t b/t/sqlmaker/limit_dialects/first_skip.t new file mode 100644 index 0000000..c9e93bb --- /dev/null +++ b/t/sqlmaker/limit_dialects/first_skip.t @@ -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; diff --git a/t/sqlmaker/limit_dialects/rownum.t b/t/sqlmaker/limit_dialects/rownum.t index e792d69..14ac5cd 100644 --- a/t/sqlmaker/limit_dialects/rownum.t +++ b/t/sqlmaker/limit_dialects/rownum.t @@ -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 ( { @@ -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 ], ], @@ -62,7 +66,8 @@ 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 <= ? @@ -70,6 +75,7 @@ for my $test_set ( WHERE rownum__index >= ? )', binds => [ + $where_bind, [ $TOTAL => 4 ], [ $OFFSET => 4 ], ], @@ -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 ], ], @@ -114,7 +122,8 @@ 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 <= ? @@ -122,6 +131,7 @@ for my $test_set ( WHERE rownum__index >= ? )', binds => [ + $where_bind, [ $TOTAL => 5 ], [ $OFFSET => 4 ], ], diff --git a/t/sqlmaker/limit_dialects/skip_first.t b/t/sqlmaker/limit_dialects/skip_first.t new file mode 100644 index 0000000..897ed2e --- /dev/null +++ b/t/sqlmaker/limit_dialects/skip_first.t @@ -0,0 +1,152 @@ +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 ('SkipFirst'); + +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 SKIP ? FIRST ? 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 = ? ) + )', + [ + [ $OFFSET => 2 ], + [ $LIMIT => 1 ], + [ { 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 SKIP ? FIRST ? [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] = ? ) + )', + [ + [ $OFFSET => 2 ], + [ $LIMIT => 1 ], + [ { 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 SKIP ? FIRST ? + [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] = ? ) + )', + [ + [ $OFFSET => 2 ], # outer + [ $LIMIT => 1 ], # 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 SKIP ? FIRST ? [me].[id], [me].[owner] + FROM [books] [me] + WHERE ( ( (EXISTS ( + SELECT SKIP ? FIRST ? [owner].[id] FROM [owners] [owner] WHERE ( [books].[owner] = [owner].[id] ) + )) AND [source] = ? ) ) + )', + [ + [ $OFFSET => 2 ], #outer + [ $LIMIT => 1 ], #outer + [ {%$OFFSET} => 3 ], #inner + [ {%$LIMIT} => 4 ], #inner + [ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ], + ], + 'Pagination with sub-query in WHERE works' +); + +} + + +done_testing;