From: Alexander Hartmaier Date: Mon, 6 Jun 2011 15:10:52 +0000 (+0200) Subject: fixed order of rows difference between first and subsequent pages for Oracle X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6a6394f1;p=dbsrgits%2FDBIx-Class-Historic.git fixed order of rows difference between first and subsequent pages for Oracle --- diff --git a/Changes b/Changes index be62dda..6ddee27 100644 --- a/Changes +++ b/Changes @@ -11,6 +11,8 @@ Revision history for DBIx::Class - Fix issue where the query was becoming overly mangled when trying to use pagination with a query that has a sub-select in the WHERE clause + - Fix possible incorrect pagination on Oracle, when a resultset + is not ordered by a unique column - Revert "Fix incorrect signature of the default sqlt_deploy_hook" from 0.08191 - documentation was in fact incorrect, not the code - Fix Sybase ASE IC::DateTime support (::Storage going out of sync diff --git a/lib/DBIx/Class/SQLMaker/LimitDialects.pm b/lib/DBIx/Class/SQLMaker/LimitDialects.pm index f3b815b..56f2d53 100644 --- a/lib/DBIx/Class/SQLMaker/LimitDialects.pm +++ b/lib/DBIx/Class/SQLMaker/LimitDialects.pm @@ -214,14 +214,31 @@ sub _FirstSkip { ); } + =head2 RowNum +Depending on the resultset attributes one of: + SELECT * FROM ( SELECT *, ROWNUM rownum__index FROM ( SELECT ... ) WHERE ROWNUM <= ($limit+$offset) ) WHERE rownum__index >= ($offset+1) +or + + SELECT * FROM ( + SELECT *, ROWNUM rownum__index FROM ( + SELECT ... + ) + ) WHERE rownum__index BETWEEN ($offset+1) AND ($limit+$offset) + +or + + SELECT * FROM ( + SELECT ... + ) WHERE ROWNUM <= ($limit+1) + Supported by B. =cut @@ -234,33 +251,92 @@ sub _RowNum { my $idx_name = $self->_quote ('rownum__index'); my $order_group_having = $self->_parse_rs_attrs($rs_attrs); + # + # There are two ways to limit in Oracle, one vastly faster than the other + # on large resultsets: https://decipherinfosys.wordpress.com/2007/08/09/paging-and-countstopkey-optimization/ + # However Oracle is retarded and does not preserve stable ROWNUM() values + # when called twice in the same scope. Therefore unless the resultset is + # ordered by a unique set of columns, it is not safe to use the faster + # method, and the slower BETWEEN query is used instead + # + # FIXME - this is quite expensive, and doe snot perform caching of any sort + # as soon as some of the DQ work becomes viable consider switching this + # over + if ( __order_by_is_unique($rs_attrs) ) { + + # if offset is 0 (first page) the we can skip a subquery + if (! $offset) { + push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ]; + + return <{limit_bind}}, [ $self->__total_bindtype => $offset + $rows ], [ $self->__offset_bindtype => $offset + 1 ]; - if ($offset) { - - push @{$self->{limit_bind}}, [ $self->__total_bindtype => $offset + $rows ], [ $self->__offset_bindtype => $offset + 1 ]; - - return <= ? EOS - + } } else { - push @{$self->{limit_bind}}, [ $self->__rows_bindtype => $rows ]; + push @{$self->{limit_bind}}, [ $self->__offset_bindtype => $offset + 1 ], [ $self->__total_bindtype => $offset + $rows ]; return <{_rsroot_rsrc}; + my $order_by = $rs_attrs->{order_by} + || return 0; + + my $storage = $rsrc->schema->storage; + my @order_by_cols = map { $_->[0] } $storage->_extract_order_criteria($order_by) + or return 0; + + my $colinfo = + $storage->_resolve_column_info($rs_attrs->{from}, \@order_by_cols); + + my $sources = { + map {( "$_" => $_ )} map { $_->{-result_source} } values %$colinfo + }; + + my $supplied_order = { + map { $_ => 1 } + grep { exists $colinfo->{$_} and ! $colinfo->{$_}{is_nullable} } + @order_by_cols + }; + + return 0 unless keys %$supplied_order; + + for my $uks ( + map { values %$_ } map { +{ $_->unique_constraints } } values %$sources + ) { + return 1 + unless first { ! exists $supplied_order->{$_} } @$uks; } + + return 0; } -# used by _Top and _FetchFirst +# used by _Top and _FetchFirst below sub _prep_for_skimming_limit { my ( $self, $sql, $rs_attrs ) = @_; diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index 45131c7..d9a97fc 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -4,7 +4,7 @@ package #hide from PAUSE # # This module contains code that should never have seen the light of day, # does not belong in the Storage, or is otherwise unfit for public -# display. The arrival of SQLA2 should immediately oboslete 90% of this +# display. The arrival of SQLA2 should immediately obsolete 90% of this # use strict; diff --git a/t/73oracle_hq.t b/t/73oracle_hq.t index 07c1f40..6759ded 100644 --- a/t/73oracle_hq.t +++ b/t/73oracle_hq.t @@ -316,7 +316,7 @@ do_creates($dbh); my $rs = $schema->resultset('Artist')->search({}, { start_with => { name => 'root' }, connect_by => { parentid => { -prior => { -ident => 'artistid' } } }, - order_by => { -asc => 'name' }, + order_by => [ { -asc => 'name' }, { -desc => 'artistid' } ], rows => 2, }); @@ -329,7 +329,7 @@ do_creates($dbh); FROM artist me START WITH name = ? CONNECT BY parentid = PRIOR artistid - ORDER BY name ASC + ORDER BY name ASC, artistid DESC ) me WHERE ROWNUM <= ? )', @@ -352,17 +352,22 @@ do_creates($dbh); FROM ( SELECT artistid FROM ( - SELECT me.artistid - FROM artist me - START WITH name = ? - CONNECT BY parentid = PRIOR artistid + SELECT artistid, ROWNUM rownum__index + FROM ( + SELECT me.artistid + FROM artist me + START WITH name = ? + CONNECT BY parentid = PRIOR artistid + ) me ) me - WHERE ROWNUM <= ? + WHERE rownum__index BETWEEN ? AND ? ) me )', [ [ { 'sqlt_datatype' => 'varchar', 'dbic_colname' => 'name', 'sqlt_size' => 100 } - => 'root'], [ $ROWS => 2 ] , + => 'root'], + [ $ROWS => 1 ], + [ $TOTAL => 2 ], ], ); diff --git a/t/sqlmaker/limit_dialects/rownum.t b/t/sqlmaker/limit_dialects/rownum.t index d9bc1a4..e792d69 100644 --- a/t/sqlmaker/limit_dialects/rownum.t +++ b/t/sqlmaker/limit_dialects/rownum.t @@ -18,53 +18,121 @@ $s->storage->sql_maker->limit_dialect ('RowNum'); my $rs = $s->resultset ('CD'); -is_same_sql_bind ( - $rs->search ({}, { rows => 1, offset => 3,columns => [ - { id => 'foo.id' }, - { 'bar.id' => 'bar.id' }, - { bleh => \ 'TO_CHAR (foo.womble, "blah")' }, - ]})->as_query, - '( - SELECT id, bar__id, bleh +for my $test_set ( + { + name => 'Rownum subsel aliasing works correctly', + rs => $rs->search_rs(undef, { + rows => 1, + offset => 3, + columns => [ + { id => 'foo.id' }, + { 'bar.id' => 'bar.id' }, + { bleh => \'TO_CHAR (foo.womble, "blah")' }, + ] + }), + sql => '( + SELECT id, bar__id, bleh FROM ( 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 - ) me + FROM ( + SELECT foo.id AS id, bar.id AS bar__id, TO_CHAR (foo.womble, "blah") AS bleh + FROM cd me + ) me + ) me WHERE rownum__index BETWEEN ? AND ? + )', + binds => [ + [ $OFFSET => 4 ], + [ $TOTAL => 4 ], + ], + }, { + name => 'Rownum subsel aliasing works correctly with unique order_by', + rs => $rs->search_rs(undef, { + rows => 1, + offset => 3, + columns => [ + { id => 'foo.id' }, + { 'bar.id' => 'bar.id' }, + { bleh => \'TO_CHAR (foo.womble, "blah")' }, + ], + order_by => [qw( artist title )], + }), + sql => '( + SELECT id, bar__id, bleh + FROM ( + 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 + ORDER BY artist, title + ) me WHERE ROWNUM <= ? ) me - WHERE rownum__index >= ? - )', - [ - [ $TOTAL => 4 ], - [ $OFFSET => 4 ], - ], - 'Rownum subsel aliasing works correctly' -); - -is_same_sql_bind ( - $rs->search ({}, { rows => 2, offset => 3,columns => [ - { id => 'foo.id' }, - { 'ends_with_me.id' => 'ends_with_me.id' }, - ]})->as_query, - '(SELECT id, ends_with_me__id + WHERE rownum__index >= ? + )', + binds => [ + [ $TOTAL => 4 ], + [ $OFFSET => 4 ], + ], + }, { + name => 'Rownum subsel aliasing #2 works correctly', + rs => $rs->search_rs(undef, { + rows => 2, + offset => 3, + columns => [ + { id => 'foo.id' }, + { 'ends_with_me.id' => 'ends_with_me.id' }, + ] + }), + sql => '( + SELECT id, ends_with_me__id FROM ( 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 - ) me + FROM ( + SELECT foo.id AS id, ends_with_me.id AS ends_with_me__id + FROM cd me + ) me + ) me WHERE rownum__index BETWEEN ? AND ? + )', + binds => [ + [ $OFFSET => 4 ], + [ $TOTAL => 5 ], + ], + }, { + name => 'Rownum subsel aliasing #2 works correctly with unique order_by', + rs => $rs->search_rs(undef, { + rows => 2, + offset => 3, + columns => [ + { id => 'foo.id' }, + { 'ends_with_me.id' => 'ends_with_me.id' }, + ], + order_by => [qw( artist title )], + }), + sql => '( + SELECT id, ends_with_me__id + FROM ( + 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 + ORDER BY artist, title + ) me WHERE ROWNUM <= ? ) me - WHERE rownum__index >= ? - )', - [ - [ $TOTAL => 5 ], - [ $OFFSET => 4 ], - ], - 'Rownum subsel aliasing works correctly' -); + WHERE rownum__index >= ? + )', + binds => [ + [ $TOTAL => 5 ], + [ $OFFSET => 4 ], + ], + } +) { + is_same_sql_bind( + $test_set->{rs}->as_query, + $test_set->{sql}, + $test_set->{binds}, + $test_set->{name}); +} { my $subq = $s->resultset('Owners')->search({ @@ -94,15 +162,14 @@ is_same_sql_bind( JOIN owners owner ON owner.id = me.owner WHERE ( source = ? ) ) me - WHERE ROWNUM <= ? ) me - WHERE rownum__index >= ? + WHERE rownum__index BETWEEN ? AND ? )', [ [ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ], - [ $TOTAL => 5 ], [ $OFFSET => 4 ], + [ $TOTAL => 5 ], ], 'pagination with subquery works' @@ -134,14 +201,16 @@ my $rs_selectas_rel = $s->resultset('BooksInLibrary')->search( { -exists => $sub is_same_sql_bind( $rs_selectas_rel->as_query, - '( SELECT id, owner FROM ( - SELECT me.id, me.owner - FROM books me - WHERE ( ( (EXISTS (SELECT COUNT( * ) FROM owners owner WHERE ( books.owner = owner.id ))) AND source = ? ) ) - ) me WHERE ROWNUM <= ? - )', + '( + SELECT id, owner FROM ( + SELECT id, owner, ROWNUM rownum__index FROM ( + SELECT me.id, me.owner FROM books me WHERE ( ( (EXISTS (SELECT COUNT( * ) FROM owners owner WHERE ( books.owner = owner.id ))) AND source = ? ) ) + ) me + ) me WHERE rownum__index BETWEEN ? AND ? + )', [ [ { sqlt_datatype => 'varchar', sqlt_size => 100, dbic_colname => 'source' } => 'Library' ], + [ $OFFSET => 1 ], [ $TOTAL => 1 ], ], 'Pagination with sub-query in WHERE works'