From: Peter Rabbitson Date: Sat, 9 Jan 2010 10:26:46 +0000 (+0000) Subject: Close the book on mssql ordered subqueries X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6de07ea386016c9a45b51ad919ac22563bb4d9d6;p=dbsrgits%2FDBIx-Class-Historic.git Close the book on mssql ordered subqueries --- diff --git a/lib/DBIx/Class/Storage/DBI/MSSQL.pm b/lib/DBIx/Class/Storage/DBI/MSSQL.pm index 178a007..ea1269a 100644 --- a/lib/DBIx/Class/Storage/DBI/MSSQL.pm +++ b/lib/DBIx/Class/Storage/DBI/MSSQL.pm @@ -191,6 +191,9 @@ sub _select_args_to_query { # see if this is an ordered subquery my $attrs = $_[3]; if ( scalar $self->sql_maker->_order_by_chunks ($attrs->{order_by}) ) { + $self->throw_exception( + 'An ordered subquery encountered. Please see "Ordered Subqueries" in DBIx::Class::Storage::DBI::MSSQL + ') unless $attrs->{unsafe_subquery}; my $max = 2 ** 32; $sql =~ s/^ \s* SELECT \s/SELECT TOP $max /xi; } @@ -305,6 +308,45 @@ $table_name ON>. Unfortunately this operation in MSSQL requires the C privilege, which is normally not included in the standard write-permissions. +=head2 Ordered Subqueries + + # this is deemed unsafe and throws under MSSQL + $rs->search ({}, { + prefetch => 'relation', + rows => 2, + offset => 3, + }); + + # however this should work (but please check what comes back from the db) + $rs->search ({}, { + unsafe_subquery => 1, + prefetch => 'relation', + rows => 2, + offset => 3, + }); + +DBIC can do truly wonderful things with the aid of subqueries, and does so +automatically when necessary. Especially useful are ordered subqueries, +which allow things like "Give me things number 4 to 6 (ordered by name), and +prefetch all their relationss, no matter how many". In its pursuit of standards +Microsft SQL Server goes to great lengths to forbid the use of ordered +subqueries. While there is a hack which fools the syntax checker, the optimizer +may B. Testing has determined that while +such breakage does occur (the test suite contains an explicit test which +demonstrates the problem), it is relative rare. The benefits of ordered +subqueries are on the other hand too great to be outright disabled for MSSQL. + +Thus compromise between usability and perfection is the MSSQL-specific +L C. +It is deliberately not possible to set this on the Storage level, as the user +should inspect (and preferrably regression-test) the return of every such +ResultSet individually. + +If it is possible to rewrite the search() in a way that will avoid the need +for this flag - you are urged to do so. If DBIC internals insist that an +ordered subquery is necessary for an operation, and you believe there is a +differnt way to express the query - please file a bugreport. + =head1 AUTHOR See L. diff --git a/t/746mssql.t b/t/746mssql.t index c32797a..f73f5ff 100644 --- a/t/746mssql.t +++ b/t/746mssql.t @@ -255,9 +255,14 @@ lives_ok ( sub { ]); }, 'populate without PKs supplied ok' ); -# make sure ordered subselects work +# plain ordered subqueries throw +throws_ok (sub { + $schema->resultset('Owners')->search ({}, { order_by => 'name' })->as_query +}, qr/ordered subquery encountered/, 'Ordered Subquery detection throws ok'); + +# make sure ordered subselects *somewhat* work { - my $owners = $schema->resultset ('Owners')->search ({}, { order_by => 'name', offset => 2, rows => 3 }); + my $owners = $schema->resultset ('Owners')->search ({}, { order_by => 'name', offset => 2, rows => 3, unsafe_subquery => 1 }); my $al = $owners->current_source_alias; my $sealed_owners = $owners->result_source->resultset->search ( @@ -277,29 +282,13 @@ lives_ok ( sub { [ map { $_->name } ($owners->all) ], 'Sort preserved from within a subquery', ); - - - my $corelated_owners = $owners->result_source->resultset->search ( - { - id => { -in => $owners->get_column('id')->as_query }, - }, - { - order_by => 'name' - }, - ); - - is_deeply ( - [ map { $_->name } ($corelated_owners->all) ], - [ map { $_->name } ($owners->all) ], - 'Sort preserved from within a corelated subquery', - ); } TODO: { local $TODO = "This porbably will never work, but it isn't critical either afaik"; my $book_owner_ids = $schema->resultset ('BooksInLibrary') - ->search ({}, { join => 'owner', distinct => 1, order_by => 'owner.name' }) + ->search ({}, { join => 'owner', distinct => 1, order_by => 'owner.name', unsafe_subquery => 1 }) ->get_column ('owner'); my $book_owners = $schema->resultset ('Owners')->search ({ @@ -313,6 +302,27 @@ TODO: { ); } +# This is known not to work - thus the negative test +{ + my $owners = $schema->resultset ('Owners')->search ({}, { order_by => 'name', offset => 2, rows => 3, unsafe_subquery => 1 }); + my $corelated_owners = $owners->result_source->resultset->search ( + { + id => { -in => $owners->get_column('id')->as_query }, + }, + { + order_by => 'name' #reorder because of what is shown above + }, + ); + + cmp_ok ( + join ("\x00", map { $_->name } ($corelated_owners->all) ), + 'ne', + join ("\x00", map { $_->name } ($owners->all) ), + 'Sadly sort not preserved from within a corelated subquery', + ); +} + + # make sure right-join-side single-prefetch ordering limit works { my $rs = $schema->resultset ('BooksInLibrary')->search ( @@ -334,7 +344,7 @@ TODO: { 'Rows were properly ordered' ); - my $limited_rs = $rs->search ({}, {rows => 7, offset => 2}); + my $limited_rs = $rs->search ({}, {rows => 7, offset => 2, unsafe_subquery => 1}); is ($limited_rs->count, 6, 'Correct count of limited right-sorted joined resultset'); is ($limited_rs->count_rs->next, 6, 'Correct count_rs of limited right-sorted joined resultset'); @@ -380,6 +390,7 @@ $schema->storage->_sql_maker->{name_sep} = '.'; prefetch => 'books', order_by => { -asc => \['name + ?', [ test => 'xxx' ]] }, # test bindvar propagation rows => 3, # 8 results total + unsafe_subquery => 1, }, ); @@ -408,6 +419,7 @@ $schema->storage->_sql_maker->{name_sep} = '.'; prefetch => 'owner', rows => 2, # 3 results total order_by => { -desc => 'owner' }, + unsafe_subquery => 1, }, );