From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sat, 14 Aug 2010 10:50:08 +0000 (+0200)
Subject: SQLA::Limit is no more \o/
X-Git-Tag: v0.08124~101^2~3
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6a247f3368100ac0557e33cc534bc8ad0ccb1175;p=dbsrgits%2FDBIx-Class.git

SQLA::Limit is no more \o/
---

diff --git a/Changes b/Changes
index e9e8654..d7ffe6a 100644
--- a/Changes
+++ b/Changes
@@ -29,6 +29,8 @@ Revision history for DBIx::Class
         - Bumped minimum Module::Install for developers
         - Bumped DBD::SQLite dependency and removed some TODO markers
           from tests
+        - No longer depend on SQL::Abstract::Limit - DBIC has been doing
+          most of the heavy lifting for a while anyway
 
 0.08123 2010-06-12 14:46 (UTC)
     * Fixes
diff --git a/Makefile.PL b/Makefile.PL
index fad044a..8fa4ddb 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -57,7 +57,6 @@ my $runtime_requires = {
   'Module::Find'             => '0.06',
   'Path::Class'              => '0.18',
   'SQL::Abstract'            => '1.67',
-  'SQL::Abstract::Limit'     => '0.13',
   'Sub::Name'                => '0.04',
   'Data::Dumper::Concise'    => '1.000',
   'Scope::Guard'             => '0.03',
diff --git a/lib/DBIx/Class/SQLAHacks.pm b/lib/DBIx/Class/SQLAHacks.pm
index adb06a9..bf6d148 100644
--- a/lib/DBIx/Class/SQLAHacks.pm
+++ b/lib/DBIx/Class/SQLAHacks.pm
@@ -1,11 +1,17 @@
 package # Hide from PAUSE
   DBIx::Class::SQLAHacks;
 
-# This module is a subclass of SQL::Abstract::Limit and includes a number
-# of DBIC-specific workarounds, not yet suitable for inclusion into the
-# SQLA core
-
-use base qw/SQL::Abstract::Limit/;
+# This module is a subclass of SQL::Abstract and includes a number of
+# DBIC-specific workarounds, not yet suitable for inclusion into the
+# SQLA core.
+# It also provides all (and more than) the functionality of
+# SQL::Abstract::Limit, which proved to be very hard to keep updated
+
+use base qw/
+  SQL::Abstract
+  Class::Accessor::Grouped
+/;
+use mro 'c3';
 use strict;
 use warnings;
 use List::Util 'first';
@@ -13,6 +19,8 @@ use Sub::Name 'subname';
 use Carp::Clan qw/^DBIx::Class|^SQL::Abstract|^Try::Tiny/;
 use namespace::clean;
 
+__PACKAGE__->mk_group_accessors (simple => qw/quote_char name_sep limit_dialect/);
+
 BEGIN {
   # reinstall the carp()/croak() functions imported into SQL::Abstract
   # as Carp and Carp::Clan do not like each other much
@@ -35,25 +43,13 @@ BEGIN {
 }
 
 # the "oh noes offset/top without limit" constant
-# limited to 32 bits for sanity (and since it is fed
-# to sprintf %u)
+# limited to 32 bits for sanity (and consistency,
+# since it is ultimately handed to sprintf %u)
+# Implemented as a method, since ::Storage::DBI also
+# refers to it (i.e. for the case of software_limit or
+# as the value to abuse with MSSQL ordered subqueries)
 sub __max_int { 0xFFFFFFFF };
 
-
-# Tries to determine limit dialect.
-#
-sub new {
-  my $self = shift->SUPER::new(@_);
-
-  # This prevents the caching of $dbh in S::A::L, I believe
-  # If limit_dialect is a ref (like a $dbh), go ahead and replace
-  #   it with what it resolves to:
-  $self->{limit_dialect} = $self->_find_syntax($self->{limit_dialect})
-    if ref $self->{limit_dialect};
-
-  $self;
-}
-
 # !!! THIS IS ALSO HORRIFIC !!! /me ashamed
 #
 # Generates inner/outer select lists for various limit dialects
@@ -167,6 +163,27 @@ sub _unqualify_colname {
   return $fqcn;
 }
 
+#
+# Follow limit dialect implementations
+#
+
+# PostgreSQL and SQLite
+sub _LimitOffset {
+    my ( $self, $sql, $order, $rows, $offset ) = @_;
+    $sql .= $self->_order_by( $order ) . " LIMIT $rows";
+    $sql .= " OFFSET $offset" if +$offset;
+    return $sql;
+}
+
+# MySQL and any SQL::Statement based DBD
+sub _LimitXY {
+    my ( $self, $sql, $order, $rows, $offset ) = @_;
+    $sql .= $self->_order_by( $order ) . " LIMIT ";
+    $sql .= "$offset, " if +$offset;
+    $sql .= $rows;
+    return $sql;
+}
+
 # ANSI standard Limit/Offset implementation. DB2 and MSSQL >= 2005 use this
 sub _RowNumberOver {
   my ($self, $sql, $rs_attrs, $rows, $offset ) = @_;
@@ -233,6 +250,8 @@ sub _rno_default_order {
 }
 
 # Informix specific limit, almost like LIMIT/OFFSET
+# According to SQLA::Limit informix also supports
+# SKIP X LIMIT Y (in addition to SKIP X FIRST Y)
 sub _SkipFirst {
   my ($self, $sql, $rs_attrs, $rows, $offset) = @_;
 
@@ -251,6 +270,8 @@ sub _SkipFirst {
 }
 
 # Firebird specific limit, reverse of _SkipFirst for Informix
+# According to SQLA::Limit firebird/interbase also supports
+# ROWS X TO Y (in addition to FIRST X SKIP Y)
 sub _FirstSkip {
   my ($self, $sql, $rs_attrs, $rows, $offset) = @_;
 
@@ -527,30 +548,55 @@ EOS
   return $sql;
 }
 
+#
+# Actual SQL::Abstract overrid^Whacks
+#
 
-# While we're at it, this should make LIMIT queries more efficient,
-#  without digging into things too deeply
-sub _find_syntax {
-  my ($self, $syntax) = @_;
-  return $self->{_cached_syntax} ||= $self->SUPER::_find_syntax($syntax);
-}
-
-# Quotes table names, handles "limit" dialects (e.g. where rownum between x and
-# y)
+# Handle limit-dialect selection
 sub select {
-  my ($self, $table, $fields, $where, $rs_attrs, @rest) = @_;
+  my ($self, $table, $fields, $where, $rs_attrs, $limit, $offset) = @_;
+
+
+  $fields = $self->_recurse_fields($fields);
+
+  if (defined $offset) {
+    croak ('A supplied offset must be a non-negative integer')
+      if ( $offset =~ /\D/ or $offset < 0 );
+  }
+  $offset ||= 0;
 
-  if (not ref($table) or ref($table) eq 'SCALAR') {
-    $table = $self->_quote($table);
+  if (defined $limit) {
+    croak ('A supplied limit must be a positive integer')
+      if ( $limit =~ /\D/ or $limit <= 0 );
+  }
+  elsif ($offset) {
+    $limit = $self->__max_int;
   }
 
-  @rest = (-1) unless defined $rest[0];
-  croak "LIMIT 0 Does Not Compute" if $rest[0] == 0;
-    # and anyway, SQL::Abstract::Limit will cause a barf if we don't first
 
-  my ($sql, @bind) = $self->SUPER::select(
-    $table, $self->_recurse_fields($fields), $where, $rs_attrs, @rest
-  );
+  my ($sql, @bind);
+  if ($limit) {
+    # this is legacy code-flow from SQLA::Limit, it is not set in stone
+
+    ($sql, @bind) = $self->next::method ($table, $fields, $where);
+
+    my $limiter =
+      $self->can ('emulate_limit')  # also backcompat hook from SQLA::Limit
+        ||
+      do {
+        my $dialect = $self->limit_dialect
+          or croak "Unable to generate SQL-limit - no limit dialect specified on $self, and no emulate_limit method found";
+        $self->can ("_$dialect")
+          or croak "SQLAHacks does not implement the requested dialect '$dialect'";
+      }
+    ;
+
+    $sql = $self->$limiter ($sql, $rs_attrs, $limit, $offset);
+  }
+  else {
+    ($sql, @bind) = $self->next::method ($table, $fields, $where, $rs_attrs);
+  }
+
   push @{$self->{where_bind}}, @bind;
 
 # this *must* be called, otherwise extra binds will remain in the sql-maker
@@ -564,53 +610,25 @@ sub _assemble_binds {
   return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/from where having order/);
 }
 
-# Quotes table names, and handles default inserts
+# Handle default inserts
 sub insert {
-  my $self = shift;
-  my $table = shift;
-  $table = $self->_quote($table);
+# optimized due to hotttnesss
+#  my ($self, $table, $data, $options) = @_;
 
   # SQLA will emit INSERT INTO $table ( ) VALUES ( )
   # which is sadly understood only by MySQL. Change default behavior here,
   # until SQLA2 comes with proper dialect support
-  if (! $_[0] or (ref $_[0] eq 'HASH' and !keys %{$_[0]} ) ) {
-    my $sql = "INSERT INTO ${table} DEFAULT VALUES";
+  if (! $_[2] or (ref $_[2] eq 'HASH' and !keys %{$_[2]} ) ) {
+    my $sql = "INSERT INTO $_[1] DEFAULT VALUES";
 
-    if (my $ret = ($_[1]||{})->{returning} ) {
-      $sql .= $self->_insert_returning ($ret);
+    if (my $ret = ($_[3]||{})->{returning} ) {
+      $sql .= $_[0]->_insert_returning ($ret);
     }
 
     return $sql;
   }
 
-  $self->SUPER::insert($table, @_);
-}
-
-# Just quotes table names.
-sub update {
-  my $self = shift;
-  my $table = shift;
-  $table = $self->_quote($table);
-  $self->SUPER::update($table, @_);
-}
-
-# Just quotes table names.
-sub delete {
-  my $self = shift;
-  my $table = shift;
-  $table = $self->_quote($table);
-  $self->SUPER::delete($table, @_);
-}
-
-sub _emulate_limit {
-  my $self = shift;
-  # my ( $syntax, $sql, $order, $rows, $offset ) = @_;
-
-  if ($_[3] == -1) {
-    return $_[1] . $self->_parse_rs_attrs($_[2]);
-  } else {
-    return $self->SUPER::_emulate_limit(@_);
-  }
+  next::method(@_);
 }
 
 sub _recurse_fields {
@@ -708,36 +726,25 @@ sub _order_by {
     return $self->_parse_rs_attrs ($arg);
   }
   else {
-    my ($sql, @bind) = $self->SUPER::_order_by ($arg);
+    my ($sql, @bind) = $self->next::method($arg);
     push @{$self->{order_bind}}, @bind;
     return $sql;
   }
 }
 
-sub _order_directions {
-  my ($self, $order) = @_;
-
-  # strip bind values - none of the current _order_directions users support them
-  return $self->SUPER::_order_directions( [ map
-    { ref $_ ? $_->[0] : $_ }
-    $self->_order_by_chunks ($order)
-  ]);
-}
-
 sub _table {
-  my ($self, $from) = @_;
-  if (ref $from eq 'ARRAY') {
-    return $self->_recurse_from(@$from);
-  } elsif (ref $from eq 'HASH') {
-    return $self->_make_as($from);
-  } else {
-    return $from; # would love to quote here but _table ends up getting called
-                  # twice during an ->select without a limit clause due to
-                  # the way S::A::Limit->select works. should maybe consider
-                  # bypassing this and doing S::A::select($self, ...) in
-                  # our select method above. meantime, quoting shims have
-                  # been added to select/insert/update/delete here
+# optimized due to hotttnesss
+#  my ($self, $from) = @_;
+  if (my $ref = ref $_[1] ) {
+    if ($ref eq 'ARRAY') {
+      return $_[0]->_recurse_from(@{$_[1]});
+    }
+    elsif ($ref eq 'HASH') {
+      return $_[0]->_make_as($_[1]);
+    }
   }
+
+  return $_[0]->next::method ($_[1]);
 }
 
 sub _generate_join_clause {
@@ -827,28 +834,5 @@ sub _join_condition {
   }
 }
 
-sub limit_dialect {
-    my $self = shift;
-    if (@_) {
-      $self->{limit_dialect} = shift;
-      undef $self->{_cached_syntax};
-    }
-    return $self->{limit_dialect};
-}
-
-# Set to an array-ref to specify separate left and right quotes for table names.
-# A single scalar is equivalen to [ $char, $char ]
-sub quote_char {
-    my $self = shift;
-    $self->{quote_char} = shift if @_;
-    return $self->{quote_char};
-}
-
-# Character separating quoted table names.
-sub name_sep {
-    my $self = shift;
-    $self->{name_sep} = shift if @_;
-    return $self->{name_sep};
-}
 
 1;
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index c26525d..b47c7cd 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -7,7 +7,7 @@ use warnings;
 use base qw/DBIx::Class::Storage::DBIHacks DBIx::Class::Storage/;
 use mro 'c3';
 
-use Carp::Clan qw/^DBIx::Class/;
+use Carp::Clan qw/^DBIx::Class|^Try::Tiny/;
 use DBI;
 use DBIx::Class::Storage::DBI::Cursor;
 use DBIx::Class::Storage::Statistics;
@@ -18,11 +18,11 @@ use Try::Tiny;
 use File::Path 'make_path';
 use namespace::clean;
 
+
 # default cursor class, overridable in connect_info attributes
 __PACKAGE__->cursor_class('DBIx::Class::Storage::DBI::Cursor');
 
-__PACKAGE__->mk_group_accessors('inherited' => qw/sql_maker_class/);
-# default
+__PACKAGE__->mk_group_accessors('inherited' => qw/sql_maker_class sql_limit_dialect/);
 __PACKAGE__->sql_maker_class('DBIx::Class::SQLAHacks');
 
 __PACKAGE__->mk_group_accessors('simple' => qw/
@@ -973,23 +973,37 @@ sub _get_dbh {
   return $self->_dbh;
 }
 
-sub _sql_maker_args {
-    my ($self) = @_;
-
-    return (
-      bindtype=>'columns',
-      array_datatypes => 1,
-      limit_dialect => $self->_get_dbh,
-      %{$self->_sql_maker_opts}
-    );
-}
-
 sub sql_maker {
   my ($self) = @_;
   unless ($self->_sql_maker) {
     my $sql_maker_class = $self->sql_maker_class;
     $self->ensure_class_loaded ($sql_maker_class);
-    $self->_sql_maker($sql_maker_class->new( $self->_sql_maker_args ));
+
+    my %opts = %{$self->_sql_maker_opts||{}};
+    my $dialect =
+      $opts{limit_dialect}
+        ||
+      $self->sql_limit_dialect
+        ||
+      do {
+        my $s_class = (ref $self) || $self;
+        carp (
+          "Your storage class ($s_class) does not set sql_limit_dialect and you "
+        . 'have not supplied an explicit limit_dialect in your connection_info. '
+        . 'DBIC will attempt to use the GenericSubQ dialect, which works on most '
+        . 'databases but can be (and often is) painfully slow.'
+        );
+
+        'GenericSubQ';
+      }
+    ;
+
+    $self->_sql_maker($sql_maker_class->new(
+      bindtype=>'columns',
+      array_datatypes => 1,
+      limit_dialect => $dialect,
+      %opts,
+    ));
   }
   return $self->_sql_maker;
 }
@@ -2005,12 +2019,19 @@ sub _select_args {
     }
   }
 
-  # adjust limits
+  # Sanity check the attributes (SQLAHacks does it too, but
+  # in case of a software_limit we'll never reach there)
+  if (defined $attrs->{offset}) {
+    $self->throw_exception('A supplied offset attribute must be a non-negative integer')
+      if ( $attrs->{offset} =~ /\D/ or $attrs->{offset} < 0 );
+  }
+  $attrs->{offset} ||= 0;
+
   if (defined $attrs->{rows}) {
-    $self->throw_exception("rows attribute must be positive if present")
-      unless $attrs->{rows} > 0;
+    $self->throw_exception("The rows attribute must be a positive integer if present")
+      if ( $attrs->{rows} =~ /\D/ or $attrs->{rows} <= 0 );
   }
-  elsif (defined $attrs->{offset}) {
+  elsif ($attrs->{offset}) {
     # MySQL actually recommends this approach.  I cringe.
     $attrs->{rows} = $sql_maker->__max_int;
   }
diff --git a/lib/DBIx/Class/Storage/DBI/DB2.pm b/lib/DBIx/Class/Storage/DBI/DB2.pm
index 3bad8e0..a5b98c3 100644
--- a/lib/DBIx/Class/Storage/DBI/DB2.pm
+++ b/lib/DBIx/Class/Storage/DBI/DB2.pm
@@ -6,6 +6,8 @@ use warnings;
 use base qw/DBIx::Class::Storage::DBI/;
 use mro 'c3';
 
+__PACKAGE__->sql_limit_dialect ('RowNumberOver');
+
 sub _dbh_last_insert_id {
     my ($self, $dbh, $source, $col) = @_;
 
@@ -19,16 +21,6 @@ sub _dbh_last_insert_id {
 
 sub datetime_parser_type { "DateTime::Format::DB2"; }
 
-sub _sql_maker_opts {
-    my ( $self, $opts ) = @_;
-
-    if ( $opts ) {
-        $self->{_sql_maker_opts} = { %$opts };
-    }
-
-    return { limit_dialect => 'RowNumberOver', %{$self->{_sql_maker_opts}||{}} };
-}
-
 1;
 
 =head1 NAME
diff --git a/lib/DBIx/Class/Storage/DBI/Informix.pm b/lib/DBIx/Class/Storage/DBI/Informix.pm
index 414c391..4c475d7 100644
--- a/lib/DBIx/Class/Storage/DBI/Informix.pm
+++ b/lib/DBIx/Class/Storage/DBI/Informix.pm
@@ -9,6 +9,8 @@ use Scope::Guard ();
 use Context::Preserve 'preserve_context';
 use namespace::clean;
 
+__PACKAGE__->sql_limit_dialect ('SkipFirst');
+
 __PACKAGE__->mk_group_accessors('simple' => '__last_insert_id');
 
 =head1 NAME
@@ -37,16 +39,6 @@ sub last_insert_id {
   shift->__last_insert_id;
 }
 
-sub _sql_maker_opts {
-  my ( $self, $opts ) = @_;
-
-  if ( $opts ) {
-    $self->{_sql_maker_opts} = { %$opts };
-  }
-
-  return { limit_dialect => 'SkipFirst', %{$self->{_sql_maker_opts}||{}} };
-}
-
 sub _svp_begin {
     my ($self, $name) = @_;
 
diff --git a/lib/DBIx/Class/Storage/DBI/InterBase.pm b/lib/DBIx/Class/Storage/DBI/InterBase.pm
index 115a3ee..cd50531 100644
--- a/lib/DBIx/Class/Storage/DBI/InterBase.pm
+++ b/lib/DBIx/Class/Storage/DBI/InterBase.pm
@@ -33,6 +33,7 @@ L</connect_call_datetime_setup>.
 
 # set default
 __PACKAGE__->_use_insert_returning (1);
+__PACKAGE__->sql_limit_dialect ('FirstSkip');
 
 sub _sequence_fetch {
   my ($self, $nextval, $sequence) = @_;
@@ -90,18 +91,6 @@ EOF
   return undef;
 }
 
-# this sub stolen from DB2
-
-sub _sql_maker_opts {
-  my ( $self, $opts ) = @_;
-
-  if ( $opts ) {
-    $self->{_sql_maker_opts} = { %$opts };
-  }
-
-  return { limit_dialect => 'FirstSkip', %{$self->{_sql_maker_opts}||{}} };
-}
-
 sub _svp_begin {
     my ($self, $name) = @_;
 
diff --git a/lib/DBIx/Class/Storage/DBI/MSSQL.pm b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
index 3c6e911..73298d7 100644
--- a/lib/DBIx/Class/Storage/DBI/MSSQL.pm
+++ b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
@@ -144,7 +144,7 @@ sub last_insert_id { shift->_identity }
 
 #
 # MSSQL is retarded wrt ordered subselects. One needs to add a TOP
-# to *all* subqueries, but one also can't use TOP 100 PERCENT
+# to *all* subqueries, but one also *can't* use TOP 100 PERCENT
 # http://sqladvice.com/forums/permalink/18496/22931/ShowThread.aspx#22931
 #
 sub _select_args_to_query {
@@ -196,37 +196,26 @@ sub datetime_parser_type {
 
 sub sqlt_type { 'SQLServer' }
 
-sub sql_maker {
+sub sql_limit_dialect {
   my $self = shift;
 
-  unless ($self->_sql_maker) {
-    unless ($self->{_sql_maker_opts}{limit_dialect}) {
-      my $have_rno = 0;
+  my $supports_rno = 0;
 
-      if (exists $self->_server_info->{normalized_dbms_version}) {
-        $have_rno = 1 if $self->_server_info->{normalized_dbms_version} >= 9;
-      }
-      else {
-        # User is connecting via DBD::Sybase and has no permission to run
-        # stored procedures like xp_msver, or version detection failed for some
-        # other reason.
-        # So, we use a query to check if RNO is implemented.
-        try {
-          $self->_get_dbh->selectrow_array('SELECT row_number() OVER (ORDER BY rand())');
-          $have_rno = 1;
-        };
-      }
-
-      $self->{_sql_maker_opts} = {
-        limit_dialect => ($have_rno ? 'RowNumberOver' : 'Top'),
-        %{$self->{_sql_maker_opts}||{}}
-      };
-    }
-
-    my $maker = $self->next::method (@_);
+  if (exists $self->_server_info->{normalized_dbms_version}) {
+    $supports_rno = 1 if $self->_server_info->{normalized_dbms_version} >= 9;
+  }
+  else {
+    # User is connecting via DBD::Sybase and has no permission to run
+    # stored procedures like xp_msver, or version detection failed for some
+    # other reason.
+    # So, we use a query to check if RNO is implemented.
+    try {
+      $self->_get_dbh->selectrow_array('SELECT row_number() OVER (ORDER BY rand())');
+      $supports_rno = 1;
+    };
   }
 
-  return $self->_sql_maker;
+  return $supports_rno ? 'RowNumberOver' : 'Top';
 }
 
 sub _ping {
diff --git a/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm b/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm
index 625498a..036c550 100644
--- a/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm
+++ b/lib/DBIx/Class/Storage/DBI/ODBC/ACCESS.pm
@@ -9,6 +9,8 @@ use DBI;
 
 my $ERR_MSG_START = __PACKAGE__ . ' failed: ';
 
+__PACKAGE__->sql_limit_dialect ('Top');
+
 sub insert {
     my $self = shift;
     my ( $source, $to_insert ) = @_;
diff --git a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
index 4d8b781..30d885b 100644
--- a/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
+++ b/lib/DBIx/Class/Storage/DBI/Oracle/Generic.pm
@@ -7,6 +7,8 @@ use Context::Preserve 'preserve_context';
 use Try::Tiny;
 use namespace::clean;
 
+__PACKAGE__->sql_limit_dialect ('RowNum');
+
 =head1 NAME
 
 DBIx::Class::Storage::DBI::Oracle::Generic - Oracle Support for DBIx::Class
diff --git a/lib/DBIx/Class/Storage/DBI/Pg.pm b/lib/DBIx/Class/Storage/DBI/Pg.pm
index ade8c2c..36c9c42 100644
--- a/lib/DBIx/Class/Storage/DBI/Pg.pm
+++ b/lib/DBIx/Class/Storage/DBI/Pg.pm
@@ -13,6 +13,8 @@ use Scope::Guard ();
 use Context::Preserve 'preserve_context';
 use namespace::clean;
 
+__PACKAGE__->sql_limit_dialect ('LimitOffset');
+
 # Ask for a DBD::Pg with array support
 warn __PACKAGE__.": DBD::Pg 2.9.2 or greater is strongly recommended\n"
   if ($DBD::Pg::VERSION < 2.009002);  # pg uses (used?) version::qv()
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated.pm b/lib/DBIx/Class/Storage/DBI/Replicated.pm
index 259fc1b..46ad462 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated.pm
@@ -328,7 +328,6 @@ has 'write_handler' => (
     _dbh
     _select_args
     _dbh_execute_array
-    _sql_maker_args
     _sql_maker
     _query_start
     _sqlt_version_error
@@ -379,6 +378,8 @@ my @unimplemented = qw(
   get_dbms_capability
   set_dbms_capability
 
+  sql_limit_dialect
+
   _dbh_details
 
   _use_insert_returning
diff --git a/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm b/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm
index 85b5614..2addde0 100644
--- a/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm
+++ b/lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm
@@ -8,9 +8,8 @@ use List::Util 'first';
 use Try::Tiny;
 use namespace::clean;
 
-__PACKAGE__->mk_group_accessors(simple => qw/
-  _identity
-/);
+__PACKAGE__->mk_group_accessors(simple => qw/_identity/);
+__PACKAGE__->sql_limit_dialect ('RowNumberOver');
 
 =head1 NAME
 
@@ -98,18 +97,6 @@ sub _select_args {
   return $self->next::method(@_);
 }
 
-# this sub stolen from DB2
-
-sub _sql_maker_opts {
-  my ( $self, $opts ) = @_;
-
-  if ( $opts ) {
-    $self->{_sql_maker_opts} = { %$opts };
-  }
-
-  return { limit_dialect => 'RowNumberOver', %{$self->{_sql_maker_opts}||{}} };
-}
-
 # this sub stolen from MSSQL
 
 sub build_datetime_parser {
diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm
index bffd133..3bc3e2b 100644
--- a/lib/DBIx/Class/Storage/DBI/SQLite.pm
+++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm
@@ -11,6 +11,7 @@ use File::Copy;
 use File::Spec;
 
 __PACKAGE__->sql_maker_class('DBIx::Class::SQLAHacks::SQLite');
+__PACKAGE__->sql_limit_dialect ('LimitOffset');
 
 sub backup
 {
diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
index 637d4ec..e380740 100644
--- a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
+++ b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
@@ -16,6 +16,8 @@ use Data::Dumper::Concise 'Dumper';
 use Try::Tiny;
 use namespace::clean;
 
+__PACKAGE__->sql_limit_dialect ('RowCountOrGenericSubQ');
+
 __PACKAGE__->mk_group_accessors('simple' =>
     qw/_identity _blob_log_on_update _writer_storage _is_extra_storage
        _bulk_storage _is_bulk_storage _began_bulk_work
@@ -152,16 +154,6 @@ for my $method (@also_proxy_to_extra_storages) {
   };
 }
 
-sub _sql_maker_opts {
-  my ( $self, $opts ) = @_;
-
-  if ( $opts ) {
-    $self->{_sql_maker_opts} = { %$opts };
-  }
-
-  return { limit_dialect => 'RowCountOrGenericSubQ', %{$self->{_sql_maker_opts}||{}} };
-}
-
 sub disconnect {
   my $self = shift;
 
diff --git a/lib/DBIx/Class/Storage/DBI/mysql.pm b/lib/DBIx/Class/Storage/DBI/mysql.pm
index 7d80187..e50c487 100644
--- a/lib/DBIx/Class/Storage/DBI/mysql.pm
+++ b/lib/DBIx/Class/Storage/DBI/mysql.pm
@@ -10,6 +10,7 @@ use base qw/
 use mro 'c3';
 
 __PACKAGE__->sql_maker_class('DBIx::Class::SQLAHacks::MySQL');
+__PACKAGE__->sql_limit_dialect ('LimitXY');
 
 sub with_deferred_fk_checks {
   my ($self, $sub) = @_;
diff --git a/t/74mssql.t b/t/74mssql.t
index 3397f3c..99eb601 100644
--- a/t/74mssql.t
+++ b/t/74mssql.t
@@ -173,15 +173,10 @@ SQL
 
     my $have_rno = $version >= 9 ? 1 : 0;
 
-    local $storage->{_sql_maker}        = undef;
-    local $storage->{_sql_maker_opts}   = undef;
-
     local $storage->{_dbh_details}{info} = {}; # delete cache
 
-    $storage->sql_maker;
-
     my $rno_detected =
-      ($storage->{_sql_maker_opts}{limit_dialect} eq 'RowNumberOver') ? 1 : 0;
+      ($storage->sql_limit_dialect eq 'RowNumberOver') ? 1 : 0;
 
     ok (($have_rno == $rno_detected),
       'row_number() over support detected correctly');