Remove idiotic RowCountOrGenericSubQ - it will never work as part of as_query
Peter Rabbitson [Thu, 21 Mar 2013 08:07:32 +0000 (09:07 +0100)]
I am not sure how I overlooked this, blame is on me :(

Replacing with a ROWCOUNT fallback at the appropriate place

Changes
lib/DBIx/Class/SQLMaker/LimitDialects.pm
lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
t/sqlmaker/limit_dialects/torture.t

diff --git a/Changes b/Changes
index 416776c..819dc5b 100644 (file)
--- a/Changes
+++ b/Changes
@@ -4,6 +4,8 @@ Revision history for DBIx::Class
         - Fix update/delete operations on resultsets *joining* the updated
           table failing on MySQL. Resolves oversights in the fixes for
           RT#81378 and RT#81897
+        - Stop Sybase ASE storage from generating invalid SQL in subselects
+          when a limit without offset is encountered
 
 0.08210 2013-04-04 15:30 (UTC)
     * New Features / Changes
index 2062021..a5ac467 100644 (file)
@@ -509,32 +509,6 @@ sub _FetchFirst {
   return $sql;
 }
 
-=head2 RowCountOrGenericSubQ
-
-This is not exactly a limit dialect, but more of a proxy for B<Sybase ASE>.
-If no $offset is supplied the limit is simply performed as:
-
- SET ROWCOUNT $limit
- SELECT ...
- SET ROWCOUNT 0
-
-Otherwise we fall back to L</GenericSubQ>
-
-=cut
-
-sub _RowCountOrGenericSubQ {
-  my $self = shift;
-  my ($sql, $rs_attrs, $rows, $offset) = @_;
-
-  return $self->_GenericSubQ(@_) if $offset;
-
-  return sprintf <<"EOF", $rows, $sql, $self->_parse_rs_attrs( $rs_attrs );
-SET ROWCOUNT %d
-%s %s
-SET ROWCOUNT 0
-EOF
-}
-
 =head2 GenericSubQ
 
  SELECT * FROM (
index 346dcd9..29563f0 100644 (file)
@@ -18,7 +18,7 @@ use Try::Tiny;
 use Context::Preserve 'preserve_context';
 use namespace::clean;
 
-__PACKAGE__->sql_limit_dialect ('RowCountOrGenericSubQ');
+__PACKAGE__->sql_limit_dialect ('GenericSubQ');
 __PACKAGE__->sql_quote_char ([qw/[ ]/]);
 __PACKAGE__->datetime_parser_type(
   'DBIx::Class::Storage::DBI::Sybase::ASE::DateTime::Format'
@@ -254,8 +254,7 @@ sub _is_lob_column {
 }
 
 sub _prep_for_execute {
-  my $self = shift;
-  my $ident = $_[1];
+  my ($self, $op, $ident, $args) = @_;
 
   #
 ### This is commented out because all tests pass. However I am leaving it
@@ -274,7 +273,20 @@ sub _prep_for_execute {
   #  = $self->_parent_storage->_perform_autoinc_retrieval
   #if ($op eq 'insert' or $op eq 'update') and $self->_parent_storage;
 
-  my ($sql, $bind) = $self->next::method (@_);
+  my $limit;  # extract and use shortcut on limit without offset
+  if ($op eq 'select' and ! $args->[4] and $limit = $args->[3]) {
+    $args = [ @$args ];
+    $args->[3] = undef;
+  }
+
+  my ($sql, $bind) = $self->next::method($op, $ident, $args);
+
+  # $limit is already sanitized by now
+  $sql = join( "\n",
+    "SET ROWCOUNT $limit",
+    $sql,
+    "SET ROWCOUNT 0",
+  ) if $limit;
 
   if (my $identity_col = $self->_perform_autoinc_retrieval) {
     $sql .= "\n" . $self->_fetch_identity_sql($ident, $identity_col)
index 44df440..1a2a699 100644 (file)
@@ -640,57 +640,6 @@ my $tests = {
     ],
   },
 
-  RowCountOrGenericSubQ => {
-    limit => [
-      '(
-        SET ROWCOUNT 4
-        SELECT me.id, owner.id, owner.name, ? * ?, ?
-          FROM books me
-          JOIN owners owner
-            ON owner.id = me.owner
-        WHERE source != ? AND me.title = ? AND source = ?
-        GROUP BY avg(me.id / ?)
-        HAVING ?
-        ORDER BY me.id
-        SET ROWCOUNT 0
-      )',
-      [
-        @select_bind,
-        @where_bind,
-        @group_bind,
-        @having_bind,
-      ],
-    ],
-    limit_offset => [
-      '(
-        SELECT me.id, owner__id, owner__name, bar, baz
-          FROM (
-            SELECT me.id, owner.id AS owner__id, owner.name AS owner__name, ? * ? AS bar, ? AS baz
-              FROM books me
-              JOIN owners owner
-                ON owner.id = me.owner
-            WHERE source != ? AND me.title = ? AND source = ?
-            GROUP BY avg( me.id / ? )
-            HAVING ?
-          ) me
-        WHERE (
-          SELECT COUNT( * )
-            FROM books rownum__emulation
-          WHERE rownum__emulation.id < me.id
-        ) BETWEEN ? AND ?
-        ORDER BY me.id
-      )',
-      [
-        @select_bind,
-        @where_bind,
-        @group_bind,
-        @having_bind,
-        [ { sqlt_datatype => 'integer' } => 3 ],
-        [ { sqlt_datatype => 'integer' } => 6 ],
-      ],
-    ],
-  },
-
   GenericSubQ => {
     limit => [
       '(