Merge 'trunk' into 'sybase'
Rafael Kitover [Wed, 5 Aug 2009 08:46:51 +0000 (08:46 +0000)]
r9075@hlagh (orig r7205):  ribasushi | 2009-08-05 02:34:25 -0400
Bump dependencies:
Test::More for the new no_plan/done_testing goodies
File::Temp as per RT#48431
r9077@hlagh (orig r7207):  ribasushi | 2009-08-05 02:36:32 -0400
 r7156@Thesaurus (orig r7153):  robkinyon | 2009-07-30 20:06:04 +0200
 Create prefetch_redux branch
 r7164@Thesaurus (orig r7161):  robkinyon | 2009-07-31 22:41:01 +0200
 Added MooseX::Traits to Makefile.PL
 r7172@Thesaurus (orig r7169):  robkinyon | 2009-08-03 05:49:59 +0200
 Added two tests and marked one todo_skip
 r7187@Thesaurus (orig r7184):  ribasushi | 2009-08-03 17:24:41 +0200
 Use goto to preserve correct error-at-line reporting
 r7189@Thesaurus (orig r7186):  ribasushi | 2009-08-04 12:34:58 +0200
 Add an extra test specifically for distinct/prefetch
 Remove duplicate test in count/prefetch

 Switch to as_query instead of debug overloading
 r7190@Thesaurus (orig r7187):  ribasushi | 2009-08-04 12:35:57 +0200
 Fix how a distinct-induced group_by is calculated, taking in consideration the new prefetch mechanism
 r7197@Thesaurus (orig r7194):  ribasushi | 2009-08-04 17:31:33 +0200
 Traits not needed by anything currently in dbic
 r7198@Thesaurus (orig r7195):  ribasushi | 2009-08-04 17:41:14 +0200
 Move around tests a bit
 r7199@Thesaurus (orig r7196):  mo | 2009-08-04 21:10:57 +0200
 prefetch-grouped fails, again
 r7204@Thesaurus (orig r7201):  ribasushi | 2009-08-04 22:50:51 +0200
 Split the search_related prefetch tests into a standalone testfile
 r7205@Thesaurus (orig r7202):  ribasushi | 2009-08-04 23:05:03 +0200
 Move norbi's test to prefetch_redux - it's the same idea
 r7209@Thesaurus (orig r7206):  ribasushi | 2009-08-05 08:35:48 +0200
 Tadaaaa (even more prefetch insanity)

r9079@hlagh (orig r7209):  ribasushi | 2009-08-05 02:38:41 -0400
 r7107@Thesaurus (orig r7104):  caelum | 2009-07-24 06:51:57 +0200
 new branch to move common mssql functionality into the base class, and other tweaks
 r7109@Thesaurus (orig r7106):  caelum | 2009-07-24 07:28:11 +0200
 moved code to ::DBI::MSSQL and added DT inflation test
 r7112@Thesaurus (orig r7109):  caelum | 2009-07-24 08:46:16 +0200
 merge in some more MSSQL code, including odbc dynamic cursor support
 r7113@Thesaurus (orig r7110):  caelum | 2009-07-24 08:49:54 +0200
 fix a warning in SQLAHacks
 r7114@Thesaurus (orig r7111):  caelum | 2009-07-24 09:22:33 +0200
 add placeholder support detection for mssql through dbd::sybase
 r7118@Thesaurus (orig r7115):  caelum | 2009-07-24 16:39:06 +0200
 minor doc clarification
 r7122@Thesaurus (orig r7119):  caelum | 2009-07-25 16:10:30 +0200
 move placeholder support detection into ::Sybase::Base
 r7123@Thesaurus (orig r7120):  caelum | 2009-07-25 16:12:01 +0200
 add a comment
 r7127@Thesaurus (orig r7124):  caelum | 2009-07-26 18:04:29 +0200
 SAVEPOINT methods for MSSQL
 r7140@Thesaurus (orig r7137):  caelum | 2009-07-30 10:12:45 +0200
 better tests for "smalldatetime" support in MSSQL
 r7142@Thesaurus (orig r7139):  caelum | 2009-07-30 13:29:19 +0200
 MSSQL GUID support
 r7147@Thesaurus (orig r7144):  caelum | 2009-07-30 15:38:33 +0200
 update sqlite test schema
 r7150@Thesaurus (orig r7147):  caelum | 2009-07-30 16:26:47 +0200
 make sure the new mssql insert method works on an un-reblessed storage
 r7151@Thesaurus (orig r7148):  caelum | 2009-07-30 16:55:35 +0200
 better rebless check for insert
 r7154@Thesaurus (orig r7151):  caelum | 2009-07-30 18:57:22 +0200
 add missing file
 r7155@Thesaurus (orig r7152):  caelum | 2009-07-30 19:00:40 +0200
 fix syntax error
 r7163@Thesaurus (orig r7160):  caelum | 2009-07-31 15:52:41 +0200
 fix a bug in _determine_driver
 r7166@Thesaurus (orig r7163):  caelum | 2009-08-01 18:10:23 +0200
 default collist for storage _resolve_column_info
 r7182@Thesaurus (orig r7179):  caelum | 2009-08-03 13:42:31 +0200
 check that dynamic cursors are functional if enabled
 r7184@Thesaurus (orig r7181):  ribasushi | 2009-08-03 14:23:37 +0200
 Adjust expected sql to match the new 'Track' table definition
 r7186@Thesaurus (orig r7183):  ribasushi | 2009-08-03 15:16:10 +0200
 Simplify code and add some comments
 r7200@Thesaurus (orig r7197):  caelum | 2009-08-04 21:31:16 +0200
 update oracle tests for new "track" table
 r7203@Thesaurus (orig r7200):  caelum | 2009-08-04 22:39:57 +0200
 update Changes

r9081@hlagh (orig r7211):  ribasushi | 2009-08-05 02:40:39 -0400
 r7213@Thesaurus (orig r7210):  ribasushi | 2009-08-05 08:40:20 +0200
 Really sanify _resolve_column_info

r9083@hlagh (orig r7213):  ribasushi | 2009-08-05 04:19:37 -0400
Reminder about discard_changes and friends
r9084@hlagh (orig r7214):  ribasushi | 2009-08-05 04:26:20 -0400
Reformat and fill-in changes
r9085@hlagh (orig r7215):  caelum | 2009-08-05 04:37:12 -0400
rename connect_call_use_mars to connect_call_use_MARS

1  2 
Changes
Makefile.PL
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm
t/746mssql.t
t/lib/sqlite.sql

diff --combined Changes
+++ b/Changes
@@@ -1,24 -1,34 +1,44 @@@
  Revision history for DBIx::Class
  
-         - support for MSSQL 'money' type
-         - support for 'smalldatetime' type used in MSSQL and Sybase for
+         - Replication updates:
+           - Improved the replication tests so that they are more reliable
+             and accurate, and hopefully solve some cross platform issues.
+           - Bugfixes related to naming particular replicants in a
+             'force_pool' attribute.
+           - Lots of documentation updates, including a new Introduction.pod
+             file.
+           - Fixed the way we detect transaction to make this more reliable
+             and forward looking.
+           - Fixed some trouble with the way Moose Types are used.
+         - Refactor of MSSQL storage drivers, with some new features:
+           - Support for placeholders for MSSQL via DBD::Sybase with proper
+             autodetection
+           - 'uniqueidentifier' support with auto newid()
+           - Dynamic cursor support and other MARS options for ODBC
++          - savepoints with auto_savepoint => 1
+         - Support for MSSQL 'money' type
+         - Support for 'smalldatetime' type used in MSSQL and Sybase for
            InflateColumn::DateTime
--        - support for Postgres 'timestamp without timezone' type in
 -          InflateColumn::DateTime (RT#48389)
++        - Support for Postgres 'timestamp without timezone' type in
 +          InflateColumn::DateTime
-         - much improved Sybase support, including support for TEXT/IMAGE
++        - Much improved Sybase support, including support for TEXT/IMAGE
 +          columns and connecting via FreeTDS
 +        - Replication updates: Improved the replication tests so that they are
 +          more reliable and accurate, and hopefully solve some cross platform
 +          issues.  Bugfixes related to naming particular replicants in a
 +          'force_pool' attribute.  Lots of documentation updates, including a
 +          new Introduction.pod file. Fixed the way we detect transaction to 
 +          make this more reliable and forward looking. Fixed some trouble with
 +          the way Moose Types are used.
          - Added new MySQL specific on_connect_call macro 'set_strict_mode'
            (also known as make_mysql_not_suck_as_much)
-         - Added call to Pod::Inherit in Makefile.PL -
-           currently at author-time only, so we need to add the produced
-           .pod files to the MANIFEST
+         - Multiple prefetch-related fixes:
+           - Adjust overly agressive subquery join-chain pruning
+           - Always preserve the outer join-chain - fixes numerous
+             problems with search_related chaining
+           - Deal with the distinct => 1 attribute properly when using
+             prefetch
+         - Multiple POD improvements
  
  
  0.08108 2009-07-05 23:15:00 (UTC)
diff --combined Makefile.PL
@@@ -13,9 -13,11 +13,11 @@@ all_from 'lib/DBIx/Class.pm'
  test_requires 'Test::Builder'       => 0.33;
  test_requires 'Test::Deep'          => 0;
  test_requires 'Test::Exception'     => 0;
- test_requires 'Test::More'          => 0.82;
+ test_requires 'Test::More'          => 0.92;
  test_requires 'Test::Warn'          => 0.11;
  
+ test_requires 'File::Temp'          => 0.22;
  # Core
  requires 'List::Util'               => 0;
  requires 'Scalar::Util'             => 0;
@@@ -111,12 -113,6 +113,12 @@@ my %force_requires_if_author = 
        'DateTime::Format::Oracle' => 0,
      ) : ()
    ,
 +
 +  $ENV{DBICTEST_SYBASE_DSN}
 +    ? (
 +      'DateTime::Format::Sybase' => 0,
 +    ) : ()
 +  ,
  );
  
  if ($Module::Install::AUTHOR) {
@@@ -1264,7 -1264,7 +1264,7 @@@ sub _count_subq_rs 
    my $sub_attrs = { %$attrs };
  
    # extra selectors do not go in the subquery and there is no point of ordering it
-   delete $sub_attrs->{$_} for qw/collapse prefetch_select select as order_by/;
+   delete $sub_attrs->{$_} for qw/collapse select _prefetch_select as order_by/;
  
    # if we prefetch, we group_by primary keys only as this is what we would get out of the rs via ->next/->all
    # clobber old group_by regardless
@@@ -2780,14 -2780,7 +2780,14 @@@ sub _resolved_attrs 
                        : "${alias}.$_"
                    )
              }
 -      } ( ref($attrs->{columns}) eq 'ARRAY' ) ? @{ delete $attrs->{columns}} : (delete $attrs->{columns} || $source->columns );
 +      } ( ref($attrs->{columns}) eq 'ARRAY' ) ?
 +          @{ delete $attrs->{columns}} :
 +            (delete $attrs->{columns} ||
 +              $source->storage->order_columns_for_select(
 +                $source,
 +                [ $source->columns ]
 +              )
 +            );
    }
    # add the additional columns on
    foreach ( 'include_columns', '+columns' ) {
      $attrs->{group_by} = [ $attrs->{group_by} ];
    }
  
+   # generate the distinct induced group_by early, as prefetch will be carried via a
+   # subquery (since a group_by is present)
+   if (delete $attrs->{distinct}) {
+     $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ];
+   }
    $attrs->{collapse} ||= {};
    if ( my $prefetch = delete $attrs->{prefetch} ) {
      $prefetch = $self->_merge_attr( {}, $prefetch );
      my @prefetch =
        $source->_resolve_prefetch( $prefetch, $alias, $join_map, $prefetch_ordering, $attrs->{collapse} );
  
-     $attrs->{prefetch_select} = [ map { $_->[0] } @prefetch ];
-     push @{ $attrs->{select} }, @{$attrs->{prefetch_select}};
+     # we need to somehow mark which columns came from prefetch
+     $attrs->{_prefetch_select} = [ map { $_->[0] } @prefetch ];
+     push @{ $attrs->{select} }, @{$attrs->{_prefetch_select}};
      push @{ $attrs->{as} }, (map { $_->[1] } @prefetch);
  
      push( @{$attrs->{order_by}}, @$prefetch_ordering );
      $attrs->{_collapse_order_by} = \@$prefetch_ordering;
    }
  
-   if (delete $attrs->{distinct}) {
-     $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ];
-   }
    # if both page and offset are specified, produce a combined offset
    # even though it doesn't make much sense, this is what pre 081xx has
    # been doing
@@@ -15,8 -15,8 +15,8 @@@ use Scalar::Util()
  use List::Util();
  
  __PACKAGE__->mk_group_accessors('simple' =>
-   qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts
-      _conn_pid _conn_tid transaction_depth _dbh_autocommit savepoints/
+   qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts _conn_pid
+      _conn_tid transaction_depth _dbh_autocommit _driver_determined savepoints/
  );
  
  # the values for these accessors are picked out (and deleted) from
@@@ -629,8 -629,7 +629,8 @@@ sub disconnect 
  
      $self->_do_connection_actions(disconnect_call_ => $_) for @actions;
  
 -    $self->_dbh->rollback unless $self->_dbh_autocommit;
 +    $self->_dbh_rollback unless $self->_dbh_autocommit;
 +
      $self->_dbh->disconnect;
      $self->_dbh(undef);
      $self->{_dbh_gen}++;
@@@ -764,23 -763,27 +764,27 @@@ sub _populate_dbh 
  sub _determine_driver {
    my ($self) = @_;
  
-   if (ref $self eq 'DBIx::Class::Storage::DBI') {
-     my $driver;
+   if (not $self->_driver_determined) {
+     if (ref($self) eq __PACKAGE__) {
+       my $driver;
  
-     if ($self->_dbh) { # we are connected
-       $driver = $self->_dbh->{Driver}{Name};
-     } else {
-       # try to use dsn to not require being connected, the driver may still
-       # force a connection in _rebless to determine version
-       ($driver) = $self->_dbi_connect_info->[0] =~ /dbi:([^:]+):/i;
-     }
+       if ($self->_dbh) { # we are connected
+         $driver = $self->_dbh->{Driver}{Name};
+       } else {
+         # try to use dsn to not require being connected, the driver may still
+         # force a connection in _rebless to determine version
+         ($driver) = $self->_dbi_connect_info->[0] =~ /dbi:([^:]+):/i;
+       }
  
-     my $storage_class = "DBIx::Class::Storage::DBI::${driver}";
-     if ($self->load_optional_class($storage_class)) {
-       mro::set_mro($storage_class, 'c3');
-       bless $self, $storage_class;
-       $self->_rebless();
+       my $storage_class = "DBIx::Class::Storage::DBI::${driver}";
+       if ($self->load_optional_class($storage_class)) {
+         mro::set_mro($storage_class, 'c3');
+         bless $self, $storage_class;
+         $self->_rebless();
+       }
      }
+     $self->_driver_determined(1);
    }
  }
  
@@@ -987,25 -990,20 +991,25 @@@ sub txn_begin 
      # this isn't ->_dbh-> because
      #  we should reconnect on begin_work
      #  for AutoCommit users
 -    $self->dbh->begin_work;
 +    $self->_dbh_begin_work;
    } elsif ($self->auto_savepoint) {
      $self->svp_begin;
    }
    $self->{transaction_depth}++;
  }
  
 +sub _dbh_begin_work {
 +  my $self = shift;
 +  $self->dbh->begin_work;
 +}
 +
  sub txn_commit {
    my $self = shift;
    if ($self->{transaction_depth} == 1) {
      my $dbh = $self->_dbh;
      $self->debugobj->txn_commit()
        if ($self->debug);
 -    $dbh->commit;
 +    $self->_dbh_commit;
      $self->{transaction_depth} = 0
        if $self->_dbh_autocommit;
    }
    }
  }
  
 +sub _dbh_commit {
 +  my $self = shift;
 +  $self->_dbh->commit;
 +}
 +
  sub txn_rollback {
    my $self = shift;
    my $dbh = $self->_dbh;
          if ($self->debug);
        $self->{transaction_depth} = 0
          if $self->_dbh_autocommit;
 -      $dbh->rollback;
 +      $self->_dbh_rollback;
      }
      elsif($self->{transaction_depth} > 1) {
        $self->{transaction_depth}--;
    }
  }
  
 +sub _dbh_rollback {
 +  my $self = shift;
 +  $self->_dbh->rollback;
 +}
 +
  # This used to be the top-half of _execute.  It was split out to make it
  #  easier to override in NoBindVars without duping the rest.  It takes up
  #  all of _execute's args, and emits $sql, @bind.
@@@ -1158,12 -1146,17 +1162,17 @@@ sub _execute 
  sub insert {
    my ($self, $source, $to_insert) = @_;
  
+ # redispatch to insert method of storage we reblessed into, if necessary
+   if (not $self->_driver_determined) {
+     $self->_determine_driver;
+     goto $self->can('insert');
+   }
    my $ident = $source->from;
    my $bind_attributes = $self->source_bind_attributes($source);
  
    my $updated_cols = {};
  
-   $self->ensure_connected;
    foreach my $col ( $source->columns ) {
      if ( !defined $to_insert->{$col} ) {
        my $col_info = $source->column_info($col);
@@@ -1459,7 -1452,7 +1468,7 @@@ sub _select_args 
      ( $attrs->{rows} && keys %{$attrs->{collapse}} )
         ||
      ( $attrs->{group_by} && @{$attrs->{group_by}} &&
-       $attrs->{prefetch_select} && @{$attrs->{prefetch_select}} )
+       $attrs->{_prefetch_select} && @{$attrs->{_prefetch_select}} )
    ) {
      ($ident, $select, $where, $attrs)
        = $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs);
@@@ -1504,14 -1497,15 +1513,15 @@@ sub _adjust_select_args_for_complex_pre
    # separate attributes
    my $sub_attrs = { %$attrs };
    delete $attrs->{$_} for qw/where bind rows offset group_by having/;
-   delete $sub_attrs->{$_} for qw/for collapse prefetch_select _collapse_order_by select as/;
+   delete $sub_attrs->{$_} for qw/for collapse _prefetch_select _collapse_order_by select as/;
  
-   my $alias = $attrs->{alias};
+   my $select_root_alias = $attrs->{alias};
    my $sql_maker = $self->sql_maker;
  
    # create subquery select list - consider only stuff *not* brought in by the prefetch
    my $sub_select = [];
-   for my $i (0 .. @{$attrs->{select}} - @{$attrs->{prefetch_select}} - 1) {
+   my $sub_group_by;
+   for my $i (0 .. @{$attrs->{select}} - @{$attrs->{_prefetch_select}} - 1) {
      my $sel = $attrs->{select}[$i];
  
      # alias any functions to the dbic-side 'as' label
      ];
    }
  
-   # mangle {from}
+   # mangle {from}, keep in mind that $from is "headless" from here on
    my $join_root = shift @$from;
-   my @outer_from = @$from;
  
    my %inner_joins;
    my %join_info = map { $_->[0]{-alias} => $_->[0] } (@$from);
  
-   # in complex search_related chains $alias may *not* be 'me'
-   # so always include it in the inner join, and also shift away
-   # from the outer stack, so that the two datasets actually do
-   # meet
-   if ($join_root->{-alias} ne $alias) {
-     $inner_joins{$alias} = 1;
-     while (@outer_from && $outer_from[0][0]{-alias} ne $alias) {
-       shift @outer_from;
-     }
-     if (! @outer_from) {
-       $self->throw_exception ("Unable to find '$alias' in the {from} stack, something is wrong");
-     }
-     shift @outer_from; # the new subquery will represent this alias, so get rid of it
-   }
+   # in complex search_related chains $select_root_alias may *not* be
+   # 'me' so always include it in the inner join
+   $inner_joins{$select_root_alias} = 1 if ($join_root->{-alias} ne $select_root_alias);
  
  
    # decide which parts of the join will remain on the inside
  
    # if a multi-type join was needed in the subquery ("multi" is indicated by
    # presence in {collapse}) - add a group_by to simulate the collapse in the subq
-   for my $alias (keys %inner_joins) {
-     # the dot comes from some weirdness in collapse
-     # remove after the rewrite
-     if ($attrs->{collapse}{".$alias"}) {
-       $sub_attrs->{group_by} ||= $sub_select;
-       last;
+   unless ($sub_attrs->{group_by}) {
+     for my $alias (keys %inner_joins) {
+       # the dot comes from some weirdness in collapse
+       # remove after the rewrite
+       if ($attrs->{collapse}{".$alias"}) {
+         $sub_attrs->{group_by} ||= $sub_select;
+         last;
+       }
      }
    }
  
      $where,
      $sub_attrs
    );
-   # put it in the new {from}
-   unshift @outer_from, {
-     -alias => $alias,
+   my $subq_joinspec = {
+     -alias => $select_root_alias,
      -source_handle => $join_root->{-source_handle},
-     $alias => $subq,
+     $select_root_alias => $subq,
    };
  
+   # Generate a new from (really just replace the join slot with the subquery)
+   # Before we would start the outer chain from the subquery itself (i.e.
+   # SELECT ... FROM (SELECT ... ) alias JOIN ..., but this turned out to be
+   # a bad idea for search_related, as the root of the chain was effectively
+   # lost (i.e. $artist_rs->search_related ('cds'... ) would result in alias
+   # of 'cds', which would prevent from doing things like order_by artist.*)
+   # See t/prefetch/via_search_related.t for a better idea
+   my @outer_from;
+   if ($join_root->{-alias} eq $select_root_alias) { # just swap the root part and we're done
+     @outer_from = (
+       $subq_joinspec,
+       @$from,
+     )
+   }
+   else {  # this is trickier
+     @outer_from = ($join_root);
+     for my $j (@$from) {
+       if ($j->[0]{-alias} eq $select_root_alias) {
+         push @outer_from, [
+           $subq_joinspec,
+           @{$j}[1 .. $#$j],
+         ];
+       }
+       else {
+         push @outer_from, $j;
+       }
+     }
+   }
    # This is totally horrific - the $where ends up in both the inner and outer query
    # Unfortunately not much can be done until SQLA2 introspection arrives, and even
    # then if where conditions apply to the *right* side of the prefetch, you may have
@@@ -1698,7 -1708,7 +1724,7 @@@ sub _resolve_ident_sources 
  # also note: this adds -result_source => $rsrc to the column info
  #
  # usage:
- #   my $col_sources = $self->_resolve_column_info($ident, [map $_->[0], @{$bind}]);
+ #   my $col_sources = $self->_resolve_column_info($ident, @column_names);
  sub _resolve_column_info {
    my ($self, $ident, $colnames) = @_;
    my ($alias2src, $root_alias) = $self->_resolve_ident_sources($ident);
    my $sep = $self->_sql_maker_opts->{name_sep} || '.';
    $sep = "\Q$sep\E";
  
-   my (%return, %converted);
+   my (%return, %seen_cols);
+   # compile a global list of column names, to be able to properly
+   # disambiguate unqualified column names (if at all possible)
+   for my $alias (keys %$alias2src) {
+     my $rsrc = $alias2src->{$alias};
+     for my $colname ($rsrc->columns) {
+       push @{$seen_cols{$colname}}, $alias;
+     }
+   }
+   COLUMN:
    foreach my $col (@$colnames) {
      my ($alias, $colname) = $col =~ m/^ (?: ([^$sep]+) $sep)? (.+) $/x;
  
-     # deal with unqualified cols - we assume the main alias for all
-     # unqualified ones, ugly but can't think of anything better right now
-     $alias ||= $root_alias;
+     unless ($alias) {
+       # see if the column was seen exactly once (so we know which rsrc it came from)
+       if ($seen_cols{$colname} and @{$seen_cols{$colname}} == 1) {
+         $alias = $seen_cols{$colname}[0];
+       }
+       else {
+         next COLUMN;
+       }
+     }
  
      my $rsrc = $alias2src->{$alias};
-     $return{$col} = $rsrc && { %{$rsrc->column_info($colname)}, -result_source => $rsrc };
+     $return{$col} = $rsrc && {
+       %{$rsrc->column_info($colname)},
+       -result_source => $rsrc,
+       -source_alias => $alias,
+     };
    }
    return \%return;
  }
  
@@@ -2306,23 -2338,6 +2354,23 @@@ sub lag_behind_master 
      return;
  }
  
 +=head2 order_columns_for_select
 +
 +Returns an ordered list of column names for use with a C<SELECT> when the column
 +list is not explicitly specified.
 +By default returns the result of L<DBIx::Class::ResultSource/columns>.
 +
 +This may be overridden in a specific storage when there are requirements such
 +as moving C<BLOB> columns to the end of the list.
 +
 +=cut
 +
 +sub order_columns_for_select {
 +  my ($self, $source, $columns) = @_;
 +
 +  return @$columns;
 +}
 +
  sub DESTROY {
    my $self = shift;
    return if !$self->_dbh;
@@@ -5,48 -5,51 +5,52 @@@ use warnings
  
  use base qw/
    DBIx::Class::Storage::DBI::Sybase::Base
-   DBIx::Class::Storage::DBI::ODBC::Microsoft_SQL_Server
+   DBIx::Class::Storage::DBI::MSSQL
 +  DBIx::Class::Storage::DBI::NoBindVars
  /;
  use mro 'c3';
  
  sub _rebless {
    my $self = shift;
-   $self->disable_sth_caching(1);
+   my $dbh  = $self->_dbh;
+   if (not $self->_placeholders_supported) {
+     bless $self,
+       'DBIx::Class::Storage::DBI::Sybase::Microsoft_SQL_Server::NoBindVars';
+     $self->_rebless;
+   }
  
  # LongReadLen doesn't work with MSSQL through DBD::Sybase, and the default is
  # huge on some versions of SQL server and can cause memory problems, so we
  # fix it up here.
-   $self->set_textsize(
-     eval { $self->_dbi_connect_info->[-1]->{LongReadLen} } ||
-     32768 # the DBD::Sybase default
-   );
+   my $text_size = eval { $self->_dbi_connect_info->[-1]->{LongReadLen} } ||
+     32768; # the DBD::Sybase default
+   $dbh->do("set textsize $text_size");
  }
  
  1;
  
  =head1 NAME
  
- DBIx::Class::Storage::DBI::Sybase::Microsoft_SQL_Server - Storage::DBI subclass for MSSQL via
- DBD::Sybase
+ DBIx::Class::Storage::DBI::Sybase::Microsoft_SQL_Server - Support for Microsoft
+ SQL Server via DBD::Sybase
  
  =head1 SYNOPSIS
  
  This subclass supports MSSQL server connections via L<DBD::Sybase>.
  
- =head1 CAVEATS
- This storage driver uses L<DBIx::Class::Storage::DBI::NoBindVars> as a base.
- This means that bind variables will be interpolated (properly quoted of course)
- into the SQL query itself, without using bind placeholders.
+ =head1 DESCRIPTION
  
- More importantly this means that caching of prepared statements is explicitly
- disabled, as the interpolation renders it useless.
+ This driver tries to determine whether your version of L<DBD::Sybase> and
+ supporting libraries (usually FreeTDS) support using placeholders, if not the
+ storage will be reblessed to
+ L<DBIx::Class::Storage::DBI::Sybase::Microsoft_SQL_Server::NoBindVars>.
  
- The actual driver code for MSSQL is in
- L<DBIx::Class::Storage::DBI::ODBC::Microsoft_SQL_Server>.
+ The MSSQL specific functionality is provided by
+ L<DBIx::Class::Storage::DBI::MSSQL>.
  
- =head1 AUTHORS
+ =head1 AUTHOR
  
  See L<DBIx::Class/CONTRIBUTORS>.
  
diff --combined t/746mssql.t
@@@ -12,8 -12,9 +12,9 @@@ my ($dsn, $user, $pass) = @ENV{map { "D
  plan skip_all => 'Set $ENV{DBICTEST_MSSQL_ODBC_DSN}, _USER and _PASS to run this test'
    unless ($dsn && $user);
  
- plan tests => 33;
+ plan tests => 39;
  
+ DBICTest::Schema->load_classes('ArtistGUID');
  my $schema = DBICTest::Schema->connect($dsn, $user, $pass);
  
  {
@@@ -41,17 -42,37 +42,37 @@@ CREATE TABLE artist 
     primary key(artistid)
  )
  SQL
  });
  
  my %seen_id;
  
- # fresh $schema so we start unconnected
- $schema = DBICTest::Schema->connect($dsn, $user, $pass);
+ my @opts = (
+   { on_connect_call => 'use_dynamic_cursors' },
+   {},
+ );
+ my $new;
+ # test Auto-PK with different options
+ for my $opts (@opts) {
+   SKIP: {
+     $schema = DBICTest::Schema->connect($dsn, $user, $pass, $opts);
+     eval {
+       $schema->storage->ensure_connected
+     };
+     if ($@ =~ /dynamic cursors/) {
+       skip
+ 'Dynamic Cursors not functional, tds_version 8.0 or greater required if using'.
+ ' FreeTDS', 1;
+     }
  
- # test primary key handling
- my $new = $schema->resultset('Artist')->create({ name => 'foo' });
- ok($new->artistid > 0, "Auto-PK worked");
+     $schema->resultset('Artist')->search({ name => 'foo' })->delete;
+     $new = $schema->resultset('Artist')->create({ name => 'foo' });
+     ok($new->artistid > 0, "Auto-PK worked");
+   }
+ }
  
  $seen_id{$new->artistid}++;
  
@@@ -73,21 -94,69 +94,66 @@@ $it->next
  is( $it->next->name, "Artist 2", "iterator->next ok" );
  is( $it->next, undef, "next past end of resultset ok" );
  
+ # test GUID columns
+ $schema->storage->dbh_do (sub {
+     my ($storage, $dbh) = @_;
+     eval { $dbh->do("DROP TABLE artist") };
+     $dbh->do(<<'SQL');
+ CREATE TABLE artist (
+    artistid UNIQUEIDENTIFIER NOT NULL,
+    name VARCHAR(100),
+    rank INT NOT NULL DEFAULT '13',
+    charfield CHAR(10) NULL,
+    a_guid UNIQUEIDENTIFIER,
+    primary key(artistid)
+ )
+ SQL
+ });
+ # start disconnected to make sure insert works on an un-reblessed storage
+ $schema = DBICTest::Schema->connect($dsn, $user, $pass);
+ my $row;
+ lives_ok {
+   $row = $schema->resultset('ArtistGUID')->create({ name => 'mtfnpy' })
+ } 'created a row with a GUID';
+ ok(
+   eval { $row->artistid },
+   'row has GUID PK col populated',
+ );
+ diag $@ if $@;
+ ok(
+   eval { $row->a_guid },
+   'row has a GUID col with auto_nextval populated',
+ );
+ diag $@ if $@;
+ my $row_from_db = $schema->resultset('ArtistGUID')
+   ->search({ name => 'mtfnpy' })->first;
+ is $row_from_db->artistid, $row->artistid,
+   'PK GUID round trip';
+ is $row_from_db->a_guid, $row->a_guid,
+   'NON-PK GUID round trip';
  # test MONEY type
  $schema->storage->dbh_do (sub {
      my ($storage, $dbh) = @_;
      eval { $dbh->do("DROP TABLE money_test") };
      $dbh->do(<<'SQL');
 -
  CREATE TABLE money_test (
     id INT IDENTITY PRIMARY KEY,
     amount MONEY NULL
  )
 -
  SQL
 -
  });
  
  my $rs = $schema->resultset('Money');
  
- my $row;
  lives_ok {
    $row = $rs->create({ amount => 100 });
  } 'inserted a money value';
diff --combined t/lib/sqlite.sql
@@@ -1,6 -1,6 +1,4 @@@
---- 
---- Created by SQL::Translator::Producer::SQLite
- -- Created on Thu Jul 30 09:36:16 2009
+ -- Created on Thu Jul 30 09:37:43 2009
  --