[Jifty-commit] r4425 - in Jifty-DBI/trunk: . lib/Jifty/DBI t

jifty-commit at lists.jifty.org jifty-commit at lists.jifty.org
Tue Nov 13 13:32:27 EST 2007


Author: alexmv
Date: Tue Nov 13 13:32:26 2007
New Revision: 4425

Modified:
   Jifty-DBI/trunk/   (props changed)
   Jifty-DBI/trunk/lib/Jifty/DBI/Collection.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Informix.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle/ODBC.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Oracle.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Pg.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Sybase.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm
   Jifty-DBI/trunk/t/12prefetch.t
   Jifty-DBI/trunk/t/14handle-pg.t

Log:
 r24692 at zoq-fot-pik:  chmrr | 2007-11-13 13:31:57 -0500
  * perltidy
  * '$sb' -> '$collection' because we aren't SearchBuilder anymore
  * Remove Jifty::DBI::Collection->preload_columns, which was old and
    unused
  * add derived => 1 for prefetched collections
  * derived collections can't be relimited
  * Don't grovel theough columns to find possible joins, just look at
    the joins
  * Fix preload for records
  * Standardize on "prefetch" not "preload"
  * $collection->{leftjoins} becomes ->{joins}, to not lie
  * $collection->{aliases} rolled into ->{joins}, to also not lie
  * $collection->_preload_columns becomes ->query_columns, to have a
    better name
  * $collection->prefetch is now smarter, stealing code from future
    tisql branch
  * Remove broken ->_normal_join
  * (re-)wrapped POD
  * Factor out ->_new_record_args and ->_new_collection_args on both
    Record and Collection; makes for easier subclassing in Jifty


Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Collection.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Collection.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Collection.pm	Tue Nov 13 13:32:26 2007
@@ -27,11 +27,11 @@
   my $handle = Jifty::DBI::Handle->new();
   $handle->connect( driver => 'SQLite', database => "my_test_db" );
 
-  my $sb = My::ThingCollection->new( handle => $handle );
+  my $collection = My::ThingCollection->new( handle => $handle );
 
-  $sb->limit( column => "column_1", value => "matchstring" );
+  $collection->limit( column => "column_1", value => "matchstring" );
 
-  while ( my $record = $sb->next ) {
+  while ( my $record = $collection->next ) {
       print $record->id;
   }
 
@@ -60,7 +60,7 @@
 use Clone;
 use Carp qw/croak/;
 use base qw/Class::Accessor::Fast/;
-__PACKAGE__->mk_accessors(qw/pager preload_columns preload_related/);
+__PACKAGE__->mk_accessors(qw/pager prefetch_related derived/);
 
 =head1 METHODS
 
@@ -72,7 +72,7 @@
 should pass in a L<Jifty::DBI::Handle> (or one of its subclasses) like
 this:
 
-   my $sb = My::Jifty::DBI::Subclass->new( handle => $handle );
+   my $collection = My::Jifty::DBI::Subclass->new( handle => $handle );
 
 However, if your subclass overrides L</_init> you do not need to take
 a handle argument, as long as your subclass takes care of calling the
@@ -103,10 +103,12 @@
 sub _init {
     my $self = shift;
     my %args = (
-        handle => undef,
+        handle  => undef,
+        derived => undef,
         @_
     );
-    $self->_handle( $args{'handle'} ) if ( $args{'handle'} );
+    $self->_handle( $args{'handle'} )  if ( $args{'handle'} );
+    $self->derived( $args{'derived'} ) if ( $args{'derived'} );
     $self->table( $self->new_item->table() );
     $self->clean_slate(%args);
 }
@@ -143,11 +145,10 @@
     $self->{'alias_count'}      = 0;
     $self->{'first_row'}        = 0;
     $self->{'show_rows'}        = 0;
-    @{ $self->{'aliases'} } = ();
 
     delete $self->{$_} for qw(
         items
-        leftjoins
+        joins
         raw_rows
         count_all
         subclauses
@@ -196,82 +197,75 @@
     my $self = shift;
 
     my $query_string = $self->build_select_query();
+
     # If we're about to redo the search, we need an empty set of items
     delete $self->{'items'};
 
     my $records = $self->_handle->simple_query($query_string);
     return 0 unless $records;
     my @names = @{ $records->{NAME_lc} };
-    my $data = {};
-    my $column_map = {};
-    foreach my $column (@names) {
-        if ($column =~ /^((\w+)_?(?:\d*))_(.*?)$/) {
-            $column_map->{$1}->{$2} =$column;
-        }
-    }
-    my @tables = keys %$column_map;
+    my $data  = {};
 
+    my @tables = (
+        "main", map { $_->{alias} } values %{ $self->prefetch_related || {} }
+    );
 
     my @order;
     while ( my $base_row = $records->fetchrow_hashref() ) {
-        my $main_pkey = $base_row->{$names[0]};
-        push @order, $main_pkey unless ( $order[0] && $order[-1] eq $main_pkey);
+        my $main_pkey = $base_row->{ $names[0] };
+        push @order, $main_pkey
+            unless ( $order[0] && $order[-1] eq $main_pkey );
 
-            # let's chop the row into subrows;
+        # let's chop the row into subrows;
         foreach my $table (@tables) {
             for ( keys %$base_row ) {
-                if ( $_ =~ /$table\_(.*)$/ ) {
-                    $data->{$main_pkey}->{$table} ->{ ($base_row->{ $table . '_id' } ||$main_pkey )}->{$1} = $base_row->{$_};
+                if (/^$table\_(.*)$/) {
+                    $data->{$main_pkey}->{$table}
+                        ->{ ( $base_row->{ $table . '_id' } || $main_pkey ) }
+                        ->{$1} = $base_row->{$_};
                 }
             }
         }
-
     }
 
-    # For related "record" values, we can simply prepopulate the
-    # Jifty::DBI::Record cache and life will be good. (I suspect we want
-    # to do this _before_ doing the initial primary record load on the
-    # off chance that the primary record will try to do the relevant
-    # prefetch manually For related "collection" values, our job is a bit
-    # harder. we need to create a new empty collection object, set it's
-    # "must search" to 0 and manually add the records to it for each of
-    # the items we find. Then we need to ram it into place.
-
-    foreach my $row_id ( @order) {
+    foreach my $row_id (@order) {
         my $item;
         foreach my $row ( values %{ $data->{$row_id}->{'main'} } ) {
             $item = $self->new_item();
             $item->load_from_hash($row);
         }
-        foreach my $alias ( grep { $_ ne 'main' } keys %{ $data->{$row_id} } ) {
+        foreach my $alias ( grep { $_ ne 'main' } keys %{ $data->{$row_id} } )
+        {
 
             my $related_rows = $data->{$row_id}->{$alias};
-            my ( $class, $col_name ) = $self->class_and_column_for_alias($alias);
-            if ($class) {
+            my ( $class, $col_name )
+                = $self->class_and_column_for_alias($alias);
+            next unless $class;
+
+            my @rows = sort { $a->{id} <=> $b->{id} }
+                grep { $_->{id} } values %$related_rows;
 
             if ( $class->isa('Jifty::DBI::Collection') ) {
-                my $collection = $class->new( handle => $self->_handle );
-                foreach my $row( sort { $a->{id} <=> $b->{id} } grep {$_->{id}} values %$related_rows ) {
-                    my $entry
-                        = $collection->new_item( handle => $self->_handle );
+                my $collection = $class->new( $self->_new_collection_args,
+                    derived => 1 );
+                foreach my $row (@rows) {
+                    my $entry = $collection->new_item;
                     $entry->load_from_hash($row);
                     $collection->add_record($entry);
                 }
 
-                $item->_prefetched_collection( $col_name => $collection );
+                $item->prefetched( $col_name => $collection );
             } elsif ( $class->isa('Jifty::DBI::Record') ) {
-                foreach my $related_row ( values %$related_rows ) {
-                    my $item = $class->new( handle => $self->_handle );
-                    $item->load_from_hash($related_row);
-                }
+                warn "Multiple rows returned for $class in prefetch"
+                    if @rows > 1;
+                my $entry = $class->new( $self->_new_record_args );
+                $entry->load_from_hash( shift @rows ) if @rows;
+                $item->prefetched( $col_name => $entry );
             } else {
                 Carp::cluck(
-                    "Asked to preload $alias as a $class. Don't know how to handle $class"
+                    "Asked to prefetch $alias as a $class. Don't know how to handle $class"
                 );
             }
-            }
-
-
         }
         $self->add_record($item);
 
@@ -283,6 +277,16 @@
     return $self->_record_count;
 }
 
+sub _new_record_args {
+    my $self = shift;
+    return ( handle => $self->_handle );
+}
+
+sub _new_collection_args {
+    my $self = shift;
+    return ( handle => $self->_handle );
+}
+
 =head2 add_record RECORD
 
 Adds a record object to this collection.
@@ -323,7 +327,7 @@
 
 sub _do_count {
     my $self = shift;
-    my $all  = shift || 0;
+    my $all = shift || 0;
 
     my $query_string = $self->build_select_count_query();
     my $records      = $self->_handle->simple_query($query_string);
@@ -393,10 +397,10 @@
 
 sub _is_joined {
     my $self = shift;
-    if ( $self->{'leftjoins'} && keys %{ $self->{'leftjoins'} } ) {
+    if ( $self->{'joins'} && keys %{ $self->{'joins'} } ) {
         return (1);
     } else {
-        return ( @{ $self->{'aliases'} } );
+        return 0;
     }
 }
 
@@ -411,12 +415,12 @@
 
 sub _is_distinctly_joined {
     my $self = shift;
-    if ( $self->{'leftjoins'} ) {
-	for (values %{ $self->{'leftjoins'} } ) {
-	    return 0 unless $_->{is_distinct};
-	}
+    if ( $self->{'joins'} ) {
+        for ( values %{ $self->{'joins'} } ) {
+            return 0 unless $_->{is_distinct};
+        }
 
-	return 1;
+        return 1;
     }
 }
 
@@ -468,6 +472,8 @@
 sub build_select_query {
     my $self = shift;
 
+    return "" if $self->derived;
+
     # The initial SELECT or SELECT DISTINCT is decided later
 
     my $query_string = $self->_build_joins . " ";
@@ -481,7 +487,7 @@
         $self->_distinct_query( \$query_string );
     } else {
         $query_string
-            = "SELECT " . $self->_preload_columns . " FROM $query_string";
+            = "SELECT " . $self->query_columns . " FROM $query_string";
         $query_string .= $self->_group_clause;
         $query_string .= $self->_order_clause;
     }
@@ -492,73 +498,55 @@
 
 }
 
-=head2 preload_columns
-
-The columns that the query would load for result items.  By default it's everything.
+=head2 query_columns
 
-XXX TODO: in the case of distinct, it needs to work as well.
+The columns that the query would load for result items.  By default
+it's everything.
 
 =cut
 
-sub _preload_columns {
+sub query_columns {
     my $self = shift;
 
-    my @cols            = ();
-    my $item            = $self->new_item;
-    if( $self->{columns} and @{ $self->{columns} } ) {
-         push @cols, @{$self->{columns}};
-         # push @cols, map { warn "Preloading $_"; "main.$_ as main_" . $_ } @{$preload_columns};
+    my @cols = ();
+    my $item = $self->new_item;
+    if ( $self->{columns} and @{ $self->{columns} } ) {
+        push @cols, @{ $self->{columns} };
     } else {
         push @cols, $self->_qualified_record_columns( 'main' => $item );
     }
-    my %preload_related = %{ $self->preload_related || {} };
-    foreach my $alias ( keys %preload_related ) {
-        my $related_obj = $preload_related{$alias};
-        if ( my $col_obj = $item->column($related_obj) ) {
-            my $reference_type = $col_obj->refers_to;
-
-            my $reference_item;
-
-            if ( !$reference_type ) {
-                Carp::cluck(
-                    "Asked to prefetch $col_obj->name for $self. But $col_obj->name isn't a known reference"
-                );
-            } elsif ( $reference_type->isa('Jifty::DBI::Collection') ) {
-                $reference_item = $reference_type->new->new_item();
-            } elsif ( $reference_type->isa('Jifty::DBI::Record') ) {
-                $reference_item = $reference_type->new;
-            } else {
-                Carp::cluck(
-                    "Asked to prefetch $col_obj->name for $self. But $col_obj->name isn't a known type"
-                );
-            }
-
-            push @cols,
-                $self->_qualified_record_columns( $alias => $reference_item );
+    my %prefetch_related = %{ $self->prefetch_related || {} };
+    foreach my $alias ( keys %prefetch_related ) {
+        my $class = $prefetch_related{$alias}{class};
+
+        my $reference;
+        if ( $class->isa('Jifty::DBI::Collection') ) {
+            $reference = $class->new->new_item( $self->_new_collection_args );
+        } elsif ( $class->isa('Jifty::DBI::Record') ) {
+            $reference = $class->new( $self->_new_record_args );
         }
 
-   #     push @cols, map { $_ . ".*" } keys %{ $self->preload_related || {} };
-
+        push @cols, $self->_qualified_record_columns( $alias => $reference );
     }
     return CORE::join( ', ', @cols );
 }
 
 =head2 class_and_column_for_alias
 
-Takes the alias you've assigned to a prefetched related object. Returns the class
-of the column we've declared that alias preloads.
+Takes the alias you've assigned to a prefetched related
+object. Returns the class of the column we've declared that alias
+prefetches.
 
 =cut
 
 sub class_and_column_for_alias {
-    my $self            = shift;
-    my $alias           = shift;
-    my %preload_related = %{ $self->preload_related || {} };
-    my $related_colname = $preload_related{$alias};
-    if ( my $col_obj = $self->new_item->column($related_colname) ) {
-        return ( $col_obj->refers_to => $related_colname );
-    }
-    return undef;
+    my $self     = shift;
+    my $alias    = shift;
+    my %prefetch = %{ $self->prefetch_related || {} };
+    my $related  = $prefetch{$alias};
+    return unless $related;
+
+    return $related->{class}, $related->{name};
 }
 
 sub _qualified_record_columns {
@@ -567,7 +555,7 @@
     my $item  = shift;
     grep {$_} map {
         my $col = $_;
-        if ( $col->virtual ) {
+        if ( $col->virtual or $col->aliased_as ) {
             undef;
         } else {
             $col = $col->name;
@@ -576,32 +564,228 @@
     } $item->columns;
 }
 
-=head2  prefetch ALIAS_NAME ATTRIBUTE
+=head2 prefetch PARAMHASH
+
+Prefetches properties of a related table, in the same query.  Possible
+keys in the paramhash are:
+
+=over
 
-prefetches all related rows from alias ALIAS_NAME into the record attribute ATTRIBUTE of the
-sort of item this collection is.
+=item name
+
+This argument is required; it specifies the name of the collection or
+record that is to be prefetched.  If the name matches a column with a
+C<refers_to> relationship, the other arguments can be inferred, and
+this is the only parameter which needs to be passed.
+
+It is possible to pass values for C<name> which are not real columns
+in the model; these, while they won't be accessible by calling 
+C<< $record-> I<columnname> >> on records in this collection, will
+still be accessible by calling C<< $record->prefetched( I<columnname> ) >>.
+
+=item reference
+
+Specifies the series of column names to traverse to extract the
+information.  For instance, if groups referred to multiple users, and
+users referred to multiple phone numbers, then providing
+C<users.phones> would do the two necessary joins to produce a phone
+collection for all users in each group.
+
+This option defaults to the name, and is irrelevant if an C<alias> is
+provided.
+
+=item alias
 
-If you have employees who have many phone numbers, this method will let you search for all your employees
-    and prepopulate their phone numbers.
+Specifies an alias which has already been joined to this collection as
+the source of the prefetched data.  C<class> will also need to be
+specified.
+
+=item class
+
+Specifies the class of the data to preload.  This is only necessary if
+C<alias> is provided, and C<name> is not the name of a column which
+provides C<refers_to> information.
 
-Right now, in order to make this work, you need to do an explicit join between your primary table and the subsidiary tables AND then specify the name of the attribute you want to prefetch related data into.
-This method could be a LOT smarter. since we already know what the relationships between our tables are, that could all be precomputed.
+=back
 
-XXX TODO: in the future, this API should be extended to let you specify columns.
+For backwards compatibility, C<prefetch> can instead be called with
+C<alias> and C<name> as its two arguments, instead of a paramhash.
 
 =cut
 
 sub prefetch {
-    my $self           = shift;
-    my $alias          = shift;
-    my $into_attribute = shift;
+    my $self = shift;
+
+    # Back-compat
+    if ( @_ and $self->{joins}{ $_[0] } ) {
+
+        # First argument appears to be an alias
+        @_ = ( alias => $_[0], name => $_[1] );
+    }
+
+    my %args = (
+        alias     => undef,
+        name      => undef,
+        class     => undef,
+        reference => undef,
+        @_,
+    );
+
+    die "Must at least provide name to prefetch"
+        unless $args{name};
+
+    # Reference defaults to name
+    $args{reference} ||= $args{name};
+
+    # If we don't have an alias, do the join
+    if ( not $args{alias} ) {
+        my ( $class, @columns )
+            = $self->find_class( split /\./, $args{reference} );
+        $args{class} = ref $class;
+        ( $args{alias} ) = $self->resolve_join(@columns);
+    }
+
+    if ( not $args{class} ) {
+
+        # Check the column
+        my $column = $self->new_item->column( $args{name} );
+        $args{class} = $column->refers_to if $column;
 
-    my $preload_related = $self->preload_related() || {};
+        die "Don't know class" unless $args{class};
+    }
+
+    # Check that the class is a Jifty::DBI::Record or Jifty::DBI::Collection
+    unless ( UNIVERSAL::isa( $args{class}, "Jifty::DBI::Record" )
+        or UNIVERSAL::isa( $args{class}, "Jifty::DBI::Collection" ) )
+    {
+        warn
+            "Class ($args{class}) isn't a Jifty::DBI::Record or Jifty::DBI::Collection";
+        return undef;
+    }
+
+    $self->prefetch_related( {} ) unless $self->prefetch_related;
+    $self->prefetch_related->{ $args{alias} } = {};
+    $self->prefetch_related->{ $args{alias} }{$_} = $args{$_}
+        for qw/alias class name/;
+
+    # Return the alias, in case we made it
+    return $args{alias};
+}
+
+=head2 find_column NAMES
+
+Tales a chained list of column names, where all but the last element
+is the name of a column on the previous class which refers to the next
+collection or record.  Returns a list of L<Jifty::DBI::Column> objects
+for the list.
+
+=cut
+
+sub find_column {
+    my $self  = shift;
+    my @names = @_;
+
+    my $last = pop @names;
+    my ( $class, @columns ) = $self->find_class(@names);
+    $class = $class->new_item
+        if UNIVERSAL::isa( $class, "Jifty::DBI::Collection" );
+    my $column = $class->column($last);
+    die "$class has no column '$last'" unless $column;
+    return @columns, $column;
+}
+
+=head2 find_class NAMES
+
+Tales a chained list of column names, where each element is the name
+of a column on the previous class which refers to the next collection
+or record.  Returns an instance of the ending class, followed by the
+list of L<Jifty::DBI::Column> objects traversed to get there.
+
+=cut
+
+sub find_class {
+    my $self  = shift;
+    my @names = @_;
+
+    my @res;
+    my $object = $self;
+    my $item   = $self->new_item;
+    while ( my $name = shift @names ) {
+        my $column = $item->column($name);
+        die "$item has no column '$name'" unless $column;
+
+        push @res, $column;
+
+        my $classname = $column->refers_to;
+        unless ($classname) {
+            die "column '$name' of $item is not a reference";
+        }
+
+        if ( UNIVERSAL::isa( $classname, 'Jifty::DBI::Collection' ) ) {
+            $object = $classname->new( $self->_new_collection_args );
+            $item   = $object->new_item;
+        } elsif ( UNIVERSAL::isa( $classname, 'Jifty::DBI::Record' ) ) {
+            $object = $item = $classname->new( $self->_new_record_args );
+        } else {
+            die
+                "Column '$name' refers to '$classname' which is not record or collection";
+        }
+    }
+
+    return $object, @res;
+}
+
+=head2 resolve_join COLUMNS
 
-    $preload_related->{$alias} = $into_attribute;
+Takes a chained list of L<Jifty::DBI::Column> objects, and performs
+the requisite joins to join all of them.  Returns the alias of the
+last join.
 
-    $self->preload_related($preload_related);
+=cut
+
+sub resolve_join {
+    my $self  = shift;
+    my @chain = @_;
+
+    my $last_alias = 'main';
 
+    foreach my $column (@chain) {
+        my $name = $column->name;
+
+        my $classname = $column->refers_to;
+        unless ($classname) {
+            die "column '$name' of is not a reference";
+        }
+
+        if ( UNIVERSAL::isa( $classname, 'Jifty::DBI::Collection' ) ) {
+            my $item
+                = $classname->new( $self->_new_collection_args )->new_item;
+            my $right_alias = $self->new_alias($item);
+            $self->join(
+                type    => 'left',
+                alias1  => $last_alias,
+                column1 => 'id',
+                alias2  => $right_alias,
+                column2 => $column->by || 'id',
+            );
+            $last_alias = $right_alias;
+        } elsif ( UNIVERSAL::isa( $classname, 'Jifty::DBI::Record' ) ) {
+            my $item        = $classname->new( $self->_new_record_args );
+            my $right_alias = $self->new_alias($item);
+            $self->join(
+                type    => 'left',
+                alias1  => $last_alias,
+                column1 => $name,
+                alias2  => $right_alias,
+                column2 => $column->by || 'id',
+            );
+            $last_alias = $right_alias;
+        } else {
+            die
+                "Column '$name' refers to '$classname' which is not record or collection";
+        }
+    }
+    return $last_alias;
 }
 
 =head2 distinct_required
@@ -621,7 +805,7 @@
 
 sub distinct_required {
     my $self = shift;
-    return( $self->_is_joined ? !$self->_is_distinctly_joined : 0 );
+    return ( $self->_is_joined ? !$self->_is_distinctly_joined : 0 );
 }
 
 =head2 build_select_count_query
@@ -634,6 +818,8 @@
 sub build_select_count_query {
     my $self = shift;
 
+    return "" if $self->derived;
+
     my $query_string = $self->_build_joins . " ";
 
     if ( $self->_is_limited ) {
@@ -663,6 +849,7 @@
 
 sub do_search {
     my $self = shift;
+    return              if $self->derived;
     $self->_do_search() if $self->{'must_redo_search'};
 
 }
@@ -682,8 +869,7 @@
 
     my $item = $self->peek;
 
-    if ( $self->{'itemscount'} < $self->_record_count )
-    {
+    if ( $self->{'itemscount'} < $self->_record_count ) {
         $self->{'itemscount'}++;
     } else {    #we've gone through the whole list. reset the count.
         $self->goto_first_item();
@@ -788,7 +974,6 @@
 =head2 new_item
 
 Should return a new object of the correct type for the current collection.
-Must be overridden by a subclassed.
 
 =cut
 
@@ -799,8 +984,9 @@
     die "Jifty::DBI::Collection needs to be subclassed; override new_item\n"
         unless $class;
 
+    warn "$self $class\n" if $class =~ /::$/;
     $class->require();
-    return $class->new( handle => $self->_handle );
+    return $class->new( $self->_new_record_args );
 }
 
 =head2 record_class
@@ -828,7 +1014,8 @@
             if ref $self->{record_class};
     } elsif ( not $self->{record_class} ) {
         my $class = ref($self);
-         $class =~ s/(Collection|s)$// || die "Can't guess record class from $class";
+        $class =~ s/(Collection|s)$//
+            || die "Can't guess record class from $class";
         $self->{record_class} = $class;
     }
     return $self->{record_class};
@@ -872,7 +1059,6 @@
 
 =cut
 
-
 sub find_all_rows {
     my $self = shift;
     $self->_is_limited(-1);
@@ -971,20 +1157,26 @@
         @_    # get the real argumentlist
     );
 
-
+    return if $self->derived;
 
     # make passing in an object DTRT
     my $value_ref = ref( $args{value} );
-    if ( $value_ref ) {
-        if ( ( $value_ref ne 'ARRAY' ) 
-                && $args{value}->isa('Jifty::DBI::Record') ) {
+    if ($value_ref) {
+        if ( ( $value_ref ne 'ARRAY' )
+            && $args{value}->isa('Jifty::DBI::Record') )
+        {
             $args{value} = $args{value}->id;
-        }
-        elsif ( $value_ref eq 'ARRAY' ) {
+        } elsif ( $value_ref eq 'ARRAY' ) {
+
             # Don't modify the original reference, it isn't polite
-            $args{value} = [ @{$args{value}} ];
-            map {$_ = ( ( ref $_ && $_->isa('Jifty::DBI::Record') ) ?
-                        ( $_->id ) : $_ ) } @{$args{value}};
+            $args{value} = [ @{ $args{value} } ];
+            map {
+                $_ = (
+                      ( ref $_ && $_->isa('Jifty::DBI::Record') )
+                    ? ( $_->id )
+                    : $_
+                    )
+            } @{ $args{value} };
         }
     }
 
@@ -995,11 +1187,11 @@
 
         #If it's a like, we supply the %s around the search term
         if ( $args{'operator'} =~ /MATCHES/i ) {
-            $args{'value'}    = "%" . $args{'value'} . "%";
+            $args{'value'} = "%" . $args{'value'} . "%";
         } elsif ( $args{'operator'} =~ /STARTSWITH/i ) {
-            $args{'value'}    = $args{'value'} . "%";
+            $args{'value'} = $args{'value'} . "%";
         } elsif ( $args{'operator'} =~ /ENDSWITH/i ) {
-            $args{'value'}    = "%" . $args{'value'};
+            $args{'value'} = "%" . $args{'value'};
         }
         $args{'operator'} =~ s/(?:MATCHES|ENDSWITH|STARTSWITH)/LIKE/i;
 
@@ -1008,9 +1200,8 @@
 
         if ( $args{'quote_value'} && $args{'operator'} !~ /IS/i ) {
             if ( $value_ref eq 'ARRAY' ) {
-                map {$_ = $self->_quote_value( $_ )} @{$args{'value'}};
-            }
-            else {
+                map { $_ = $self->_quote_value($_) } @{ $args{'value'} };
+            } else {
                 $args{'value'} = $self->_quote_value( $args{'value'} );
             }
         }
@@ -1027,7 +1218,7 @@
 
     # {{{ if there's no alias set, we need to set it
 
-    unless (defined  $args{'alias'} ) {
+    unless ( defined $args{'alias'} ) {
 
         #if the table we're looking at is the same as the main table
         if ( $args{'table'} eq $self->table ) {
@@ -1049,18 +1240,22 @@
     # Set this to the name of the column and the alias, unless we've been
     # handed a subclause name
 
-    my $qualified_column = $args{'alias'} ? $args{'alias'} . "." . $args{'column'} : $args{'column'};
+    my $qualified_column
+        = $args{'alias'}
+        ? $args{'alias'} . "." . $args{'column'}
+        : $args{'column'};
     my $clause_id = $args{'subclause'} || $qualified_column;
 
     # If we're trying to get a leftjoin restriction, lets set
-    # $restriction to point htere. otherwise, lets construct normally
+    # $restriction to point there. otherwise, lets construct normally
 
     my $restriction;
     if ( $args{'leftjoin'} ) {
-        $restriction = $self->{'leftjoins'}{ $args{'leftjoin'} }{'criteria'}
-            { $clause_id } ||= [];
+        $restriction
+            = $self->{'joins'}{ $args{'leftjoin'} }{'criteria'}{$clause_id}
+            ||= [];
     } else {
-        $restriction = $self->{'restrictions'}{ $clause_id } ||= [];
+        $restriction = $self->{'restrictions'}{$clause_id} ||= [];
     }
 
     # If it's a new value or we're overwriting this sort of restriction,
@@ -1068,7 +1263,8 @@
     # XXX: when is column_obj undefined?
     my $column_obj = $self->new_item()->column( $args{column} );
     my $case_sensitive = $column_obj ? $column_obj->case_sensitive : 0;
-    $case_sensitive = $args{'case_sensitive'} if defined $args{'case_sensitive'};
+    $case_sensitive = $args{'case_sensitive'}
+        if defined $args{'case_sensitive'};
     if (   $self->_handle->case_sensitive
         && defined $args{'value'}
         && $args{'quote_value'}
@@ -1084,9 +1280,10 @@
     }
 
     if ( $value_ref eq 'ARRAY' ) {
-        croak 'Limits with an array ref are only allowed with operator \'IN\' or \'=\''
+        croak
+            'Limits with an array ref are only allowed with operator \'IN\' or \'=\''
             unless $args{'operator'} =~ /^(IN|=)$/i;
-        $args{'value'} = '( '. join(',', @{$args{'value'}}) .' )';
+        $args{'value'} = '( ' . join( ',', @{ $args{'value'} } ) . ' )';
         $args{'operator'} = 'IN';
     }
 
@@ -1098,14 +1295,14 @@
 
     # Juju because this should come _AFTER_ the EA
     my @prefix;
-    if ( $self->{'_open_parens'}{ $clause_id } ) {
-        @prefix = ('(') x delete $self->{'_open_parens'}{ $clause_id };
+    if ( $self->{'_open_parens'}{$clause_id} ) {
+        @prefix = ('(') x delete $self->{'_open_parens'}{$clause_id};
     }
 
     if ( lc( $args{'entry_aggregator'} || "" ) eq 'none' || !@$restriction ) {
-        @$restriction = (@prefix, $clause);
+        @$restriction = ( @prefix, $clause );
     } else {
-        push @$restriction, $args{'entry_aggregator'}, @prefix , $clause;
+        push @$restriction, $args{'entry_aggregator'}, @prefix, $clause;
     }
 
     # We're now limited. people can do searches.
@@ -1147,7 +1344,7 @@
 # Immediate Action
 sub close_paren {
     my ( $self, $clause ) = @_;
-    my $restriction = $self->{'restrictions'}{ $clause } ||= [];
+    my $restriction = $self->{'restrictions'}{$clause} ||= [];
     push @$restriction, ')';
 }
 
@@ -1173,7 +1370,8 @@
     #Go through all restriction types. Build the where clause from the
     #Various subclauses.
 
-    my @subclauses = grep defined && length, values %{ $self->{'subclauses'} };
+    my @subclauses = grep defined && length,
+        values %{ $self->{'subclauses'} };
 
     $where_clause = " WHERE " . CORE::join( ' AND ', @subclauses )
         if (@subclauses);
@@ -1189,22 +1387,23 @@
 
     delete $self->{'subclauses'}{'generic_restrictions'};
 
-    # Go through all the restrictions of this type. Buld up the generic subclause
+ # Go through all the restrictions of this type. Buld up the generic subclause
     my $result = '';
-    foreach my $restriction ( grep $_ && @$_, values %{ $self->{'restrictions'} } ) {
+    foreach my $restriction ( grep $_ && @$_,
+        values %{ $self->{'restrictions'} } )
+    {
         $result .= ' AND ' if $result;
         $result .= '(';
-        foreach my $entry ( @$restriction ) {
+        foreach my $entry (@$restriction) {
             unless ( ref $entry ) {
-                $result .= ' '. $entry . ' ';
-            }
-            else {
+                $result .= ' ' . $entry . ' ';
+            } else {
                 $result .= join ' ', @{$entry}{qw(column operator value)};
             }
         }
         $result .= ')';
     }
-    return ($self->{'subclauses'}{'generic_restrictions'} = $result);
+    return ( $self->{'subclauses'}{'generic_restrictions'} = $result );
 }
 
 # set $self->{$type .'_clause'} to new value
@@ -1222,16 +1421,16 @@
 # quote the search value
 sub _quote_value {
     my $self = shift;
-    my ( $value ) = @_;
+    my ($value) = @_;
 
-    my $tmp = $self->_handle->dbh->quote( $value );
+    my $tmp = $self->_handle->dbh->quote($value);
 
     # Accomodate DBI drivers that don't understand UTF8
     if ( $] >= 5.007 ) {
-      require Encode;
-      if ( Encode::is_utf8( $tmp ) ) {
-        Encode::_utf8_on($tmp);
-      }
+        require Encode;
+        if ( Encode::is_utf8($tmp) ) {
+            Encode::_utf8_on($tmp);
+        }
     }
     return $tmp;
 
@@ -1274,6 +1473,7 @@
 
 sub order_by {
     my $self = shift;
+    return if $self->derived;
     if (@_) {
         my @args = @_;
 
@@ -1283,7 +1483,7 @@
         $self->{'order_by'} = \@args;
         $self->redo_search();
     }
-    return ( $self->{'order_by'} || []);
+    return ( $self->{'order_by'} || [] );
 }
 
 =head2 _order_clause
@@ -1317,7 +1517,7 @@
             $clause .= $rowhash{'function'} . ' ';
             $clause .= $rowhash{'order'};
 
-        } elsif ( (defined $rowhash{'alias'} )
+        } elsif ( ( defined $rowhash{'alias'} )
             and ( $rowhash{'column'} ) )
         {
 
@@ -1365,6 +1565,7 @@
 sub group_by {
     my $self = shift;
 
+    return if $self->derived;
     my @args = @_;
 
     unless ( UNIVERSAL::isa( $args[0], 'HASH' ) ) {
@@ -1436,17 +1637,20 @@
 
     my $alias = $self->_get_alias($table);
 
-    my $subclause = "$table $alias";
-
-    push( @{ $self->{'aliases'} }, $subclause );
+    $self->{'joins'}{$alias} = {
+        alias        => $alias,
+        table        => $table,
+        type         => 'CROSS',
+        alias_string => " CROSS JOIN $table $alias ",
+    };
 
     return $alias;
 }
 
 # _get_alias is a private function which takes an tablename and
-# returns a new alias for that table without adding something
-# to self->{'aliases'}.  This function is used by new_alias
-# and the as-yet-unnamed left join code
+# returns a new alias for that table without adding something to
+# self->{'joins'}.  This function is used by new_alias and the
+# as-yet-unnamed left join code
 
 sub _get_alias {
     my $self  = shift;
@@ -1478,6 +1682,10 @@
 The parameter C<operator> defaults C<=>, but you can specify other
 operators to join with.
 
+Passing a true value for the C<is_distinct> parameter allows one to
+specify that, despite the join, the original table's rows are will all
+still be distinct.
+
 Instead of C<alias1>/C<column1>, it's possible to specify expression, to join
 C<alias2>/C<table2> on an arbitrary expression.
 
@@ -1495,6 +1703,7 @@
         @_
     );
 
+    return if $self->derived;
     $self->_handle->join( collection => $self, %args );
 
 }
@@ -1517,6 +1726,7 @@
         current_page => undef,    # 1-based
         @_
     );
+    return if $self->derived;
 
     $self->pager->total_entries( $self->count_all )
         ->entries_per_page( $args{'per_page'} )
@@ -1766,7 +1976,7 @@
 
     my $column = "col" . @{ $self->{columns} ||= [] };
     $column = $args{column} if $table eq $self->table and !$args{alias};
-    $column = ($args{'alias'}||'main')."_".$column;
+    $column = ( $args{'alias'} || 'main' ) . "_" . $column;
     push @{ $self->{columns} }, "$name AS \L$column";
     return $column;
 }
@@ -1856,8 +2066,8 @@
 
     $obj->redo_search();    # clean out the object of data
 
-    $obj->{$_} = Clone::clone( $obj->{$_} ) for
-        grep exists $self->{ $_ }, $self->_cloned_attributes;
+    $obj->{$_} = Clone::clone( $obj->{$_} )
+        for grep exists $self->{$_}, $self->_cloned_attributes;
     return $obj;
 }
 
@@ -1873,8 +2083,7 @@
 
 sub _cloned_attributes {
     return qw(
-        aliases
-        leftjoins
+        joins
         subclauses
         restrictions
     );
@@ -1904,7 +2113,12 @@
 
 =head1 AUTHOR
 
-Copyright (c) 2001-2005 Jesse Vincent, jesse at fsck.com.
+Jesse Vincent <jesse at bestpractical.com>, Alex Vandiver
+<alexmv at bestpractical.com>, Ruslan Zakirov <ruslan.zakirov at gmail.com>
+
+Based on DBIx::SearchBuilder::Collection, whose credits read:
+
+ Jesse Vincent, <jesse at fsck.com> 
 
 All rights reserved.
 
@@ -1914,7 +2128,6 @@
 
 =head1 SEE ALSO
 
-Jifty::DBI::Handle, Jifty::DBI::Record.
+L<Jifty::DBI>, L<Jifty::DBI::Handle>, L<Jifty::DBI::Record>.
 
 =cut
-

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle.pm	Tue Nov 13 13:32:26 2007
@@ -13,13 +13,16 @@
 
 our $VERSION = '0.01';
 
-if (my $pattern = $ENV{JIFTY_DBQUERY_CALLER}) {
+if ( my $pattern = $ENV{JIFTY_DBQUERY_CALLER} ) {
     require Hook::LexWrap;
-    Hook::LexWrap::wrap('Jifty::DBI::Handle::simple_query', pre => sub {
-        return unless $_[1] =~ m/$pattern/;
-        warn $_[1].'   '.join(',', @_[2..$#_])."\n";
-        Carp::cluck;
-    });
+    Hook::LexWrap::wrap(
+        'Jifty::DBI::Handle::simple_query',
+        pre => sub {
+            return unless $_[1] =~ m/$pattern/;
+            warn $_[1] . '   ' . CORE::join( ',', @_[ 2 .. $#_ ] ) . "\n";
+            Carp::cluck;
+        }
+    );
 }
 
 =head1 NAME
@@ -188,12 +191,12 @@
         @_
     );
 
-
     my $driver = delete $args{'driver'};
     $args{'dbname'} ||= delete $args{'database'};
 
-    $self->{'dsn'} =
-    "dbi:$driver:" . join(';', map { $_ ."=".$args{$_} } grep { defined $args{$_} } keys %args);
+    $self->{'dsn'} = "dbi:$driver:"
+        . CORE::join( ';',
+        map { $_ . "=" . $args{$_} } grep { defined $args{$_} } keys %args );
 }
 
 =head2 dsn
@@ -346,7 +349,7 @@
 
     my @bind  = ();
     my $where = 'WHERE ';
-    while (my $key = shift @pairs) {
+    while ( my $key = shift @pairs ) {
         $where .= $key . "=?" . " AND ";
         push( @bind, shift(@pairs) );
     }
@@ -364,7 +367,7 @@
 
 sub insert {
     my ( $self, $table, @pairs ) = @_;
-    my ( @cols, @vals,  @bind );
+    my ( @cols, @vals, @bind );
 
 #my %seen; #only the *first* value is used - allows drivers to specify default
     while ( my $key = shift @pairs ) {
@@ -376,7 +379,8 @@
         push @bind, $value;
     }
 
-    my $query_string = "INSERT INTO $table ("
+    my $query_string
+        = "INSERT INTO $table ("
         . CORE::join( ", ", @cols )
         . ") VALUES " . "("
         . CORE::join( ", ", @vals ) . ")";
@@ -407,7 +411,7 @@
         @_
     );
 
-    return 1 unless grep {defined} values %{$args{primary_keys}};
+    return 1 unless grep {defined} values %{ $args{primary_keys} };
 
     my @bind  = ();
     my $query = 'UPDATE ' . $args{'table'} . ' ';
@@ -523,8 +527,9 @@
                 . $self->dbh->errstr . "\n";
 
         } else {
-            # XXX: This warn doesn't show up because we mask logging in Jifty::Test::END.
-            # and it usually fails because the test server is still running.
+
+ # XXX: This warn doesn't show up because we mask logging in Jifty::Test::END.
+ # and it usually fails because the test server is still running.
             warn "$self couldn't execute the query '$query_string'";
 
             my $ret = Class::ReturnValue->new();
@@ -609,7 +614,7 @@
         my $ver = '';
         $ver = ( $sth->fetchrow_arrayref->[0] || '' ) if $sth;
         $ver =~ /(\d+(?:\.\d+)*(?:-[a-z0-9]+)?)/i;
-        $self->{'database_version'}       = $ver;
+        $self->{'database_version'} = $ver;
         $self->{'database_version_short'} = $1 || $ver;
 
         $self->raise_error($re);
@@ -648,10 +653,11 @@
     my $value    = shift;
 
     return $value ne ''
-      && $value   ne "''"
-      && ( $operator !~ /IS/ && $value !~ /^null$/i )
-      # don't downcase integer values
-      && $value !~ /^['"]?\d+['"]?$/;
+        && $value ne "''"
+        && ( $operator !~ /IS/ && $value !~ /^null$/i )
+
+        # don't downcase integer values
+        && $value !~ /^['"]?\d+['"]?$/;
 }
 
 sub _make_clause_case_insensitive {
@@ -660,13 +666,12 @@
     my $operator = shift;
     my $value    = shift;
 
-    if ($self->_case_insensitivity_valid($column, $operator, $value)) {
+    if ( $self->_case_insensitivity_valid( $column, $operator, $value ) ) {
         $column = "lower($column)";
-        if (ref $value eq 'ARRAY') {
-            map {$_ = "lower($_)"} @{$value};
-        }
-        else {
-            $value  = "lower($value)";
+        if ( ref $value eq 'ARRAY' ) {
+            map { $_ = "lower($_)" } @{$value};
+        } else {
+            $value = "lower($value)";
         }
     }
     return ( $column, $operator, $value );
@@ -811,16 +816,16 @@
 
     my $self = shift;
     my %args = (
-        collection => undef,
-        type       => 'normal',
-        alias1     => 'main',
-        column1    => undef,
-        table2     => undef,
-        alias2     => undef,
-        column2    => undef,
-        expression => undef,
-        operator   => '=',
-        is_distinct=> 0,
+        collection  => undef,
+        type        => 'normal',
+        alias1      => 'main',
+        column1     => undef,
+        table2      => undef,
+        alias2      => undef,
+        column2     => undef,
+        expression  => undef,
+        operator    => '=',
+        is_distinct => 0,
         @_
     );
 
@@ -829,236 +834,204 @@
     # If we're handed in a table2 as a Collection object, make notes
     # about if the result of the join is still distinct for the
     # calling collection
-    if ( $args{'table2'} &&
-         UNIVERSAL::isa($args{'table2'}, 'Jifty::DBI::Collection') ) {
-	if ($args{'operator'} eq '=' ) {
-	    my $x = $args{'table2'}->new_item->column($args{column2});
-	    if ($x->type eq 'serial' || $x->distinct) {
-		$args{'is_distinct'} = 1;
-	    }
-	}
+    if ( $args{'table2'}
+        && UNIVERSAL::isa( $args{'table2'}, 'Jifty::DBI::Collection' ) )
+    {
+        if ( $args{'operator'} eq '=' ) {
+            my $x = $args{'table2'}->new_item->column( $args{column2} );
+            if ( $x->type eq 'serial' || $x->distinct ) {
+                $args{'is_distinct'} = 1;
+            }
+        }
 
         $args{'table2'} = $args{'table2'}->table;
     }
 
-    #If we're handed in an alias2, we need to go remove it from the
-    # Aliases array.  Basically, if anyone generates an alias and then
-    # tries to use it in a join later, we want to be smart about creating
-    # joins, so we need to go rip it out of the old aliases table and drop
-    # it in as an explicit join
     if ( $args{'alias2'} ) {
-
-        # this code is slow and wasteful, but it's clear.
-        my @aliases = @{ $args{'collection'}->{'aliases'} };
-        my @new_aliases;
-        foreach my $old_alias (@aliases) {
-            if ( $old_alias =~ /^(.*?) (\Q$args{'alias2'}\E)$/ ) {
-                $args{'table2'} = $1;
-                $alias = $2;
-
+        if ( $args{'collection'}{'joins'}{ $args{alias2} } ) {
+            my $join = $args{'collection'}{'joins'}{ $args{alias2} };
+            if ( lc $join->{type} eq 'cross' ) {
+                $args{'table2'} = $join->{table};
+                $alias = $join->{alias};
             } else {
-                push @new_aliases, $old_alias;
+                warn "Already joined?";
+                return;
             }
-        }
-
-# If we found an alias, great. let's just pull out the table and alias for the other item
-        unless ($alias) {
+        } else {
 
             # if we can't do that, can we reverse the join and have it work?
-            my $a1 = $args{'alias1'};
-            my $f1 = $args{'column1'};
-            $args{'alias1'}  = $args{'alias2'};
-            $args{'column1'} = $args{'column2'};
-            $args{'alias2'}  = $a1;
-            $args{'column2'} = $f1;
-
-            @aliases     = @{ $args{'collection'}->{'aliases'} };
-            @new_aliases = ();
-            foreach my $old_alias (@aliases) {
-                if ( $old_alias =~ /^(.*?) ($args{'alias2'})$/ ) {
-                    $args{'table2'} = $1;
-                    $alias = $2;
+            @args{qw/alias1 alias2/}   = @args{qw/alias2 $alias2/};
+            @args{qw/column1 column2/} = @args{qw/column2 column1/};
 
+            if ( $args{'collection'}{'joins'}{ $args{alias2} } ) {
+                my $join = $args{'collection'}{'joins'}{ $args{alias2} };
+                if ( lc $join->{type} eq 'cross' ) {
+                    $args{'table2'} = $join->{table};
+                    $alias = $join->{alias};
                 } else {
-                    push @new_aliases, $old_alias;
+                    warn "Already joined?";
+                    return;
                 }
-            }
-
-        }
+            } else {
 
-        unless ( $alias ) {
-            return $self->_normal_join(%args);
+                # Swap back
+                @args{qw/alias1 alias2/}   = @args{qw/alias2 $alias2/};
+                @args{qw/column1 column2/} = @args{qw/column2 column1/};
+
+                return $self->Jifty::DBI::Collection::limit(
+                    entry_aggregator => 'AND',
+                    @_,
+                    quote_value => 0,
+                    alias       => $args{'alias1'},
+                    column      => $args{'column1'},
+                    value       => $args{'alias2'} . "." . $args{'column2'},
+                );
+            }
         }
-
-        $args{'collection'}->{'aliases'} = \@new_aliases;
-    }
-
-    else {
+    } else {
         $alias = $args{'collection'}->_get_alias( $args{'table2'} );
-
     }
 
-    my $meta = $args{'collection'}->{'leftjoins'}{ $alias } ||= {};
+    my $meta = $args{'collection'}->{'joins'}{$alias} ||= {};
+    $meta->{alias} = $alias;
     if ( $args{'type'} =~ /LEFT/i ) {
-        $meta->{'alias_string'} = " LEFT JOIN " . $args{'table2'} . " $alias ";
+        $meta->{'alias_string'}
+            = " LEFT JOIN " . $args{'table2'} . " $alias ";
         $meta->{'type'} = 'LEFT';
 
     } else {
         $meta->{'alias_string'} = " JOIN " . $args{'table2'} . " $alias ";
-        $meta->{'type'} = 'NORMAL';
+        $meta->{'type'}         = 'NORMAL';
     }
-    $meta->{'depends_on'} = $args{'alias1'};
+    $meta->{'depends_on'}       = $args{'alias1'};
     $meta->{'entry_aggregator'} = $args{'entry_aggregator'}
         if $args{'entry_aggregator'};
     $meta->{'is_distinct'} = $args{'is_distinct'};
 
     my $criterion = $args{'expression'} || "$args{'alias1'}.$args{'column1'}";
-    $meta->{'criteria'}{ 'base_criterion' } = [{
-        column   => $criterion,
-        operator => $args{'operator'},
-        value    => "$alias.$args{'column2'}",
-    }];
-
-    return ($alias);
-}
-
-sub _normal_join {
-
-    my $self = shift;
-    my %args = (
-        collection => undef,
-        type       => 'normal',
-        column1    => undef,
-        alias1     => undef,
-        table2     => undef,
-        column2    => undef,
-        alias2     => undef,
-        operator   => '=',
-        @_
-    );
-
-    my $sb = $args{'collection'};
-
-    if ( $args{'type'} =~ /LEFT/i ) {
-        my $alias = $sb->_get_alias( $args{'table2'} );
-        my $meta  = $sb->{'leftjoins'}{ $alias } ||= {};
-        $meta->{'alias_string'} = " LEFT JOIN $args{'table2'} $alias ";
-        $meta->{'depends_on'}   = $args{'alias1'};
-        $meta->{'type'}         = 'LEFT';
-        $meta->{'base_criterion'} = [ {
-            column   => "$args{'alias1'}.$args{'column1'}",
+    $meta->{'criteria'}{'base_criterion'} = [
+        {   column   => $criterion,
             operator => $args{'operator'},
             value    => "$alias.$args{'column2'}",
-        } ];
+        }
+    ];
 
-        return ($alias);
-    } else {
-        $sb->Jifty::DBI::Collection::limit(
-            entry_aggregator => 'AND',
-            @_,
-            quote_value      => 0,
-            alias            => $args{'alias1'},
-            column           => $args{'column1'},
-            value            => $args{'alias2'} . "." . $args{'column2'},
-        );
-    }
+    return ($alias);
 }
 
 # this code is all hacky and evil. but people desperately want _something_ and I'm
 # super tired. refactoring gratefully appreciated.
 
 sub _build_joins {
-    my $self = shift;
-    my $sb   = shift;
+    my $self       = shift;
+    my $collection = shift;
 
-    $self->_optimize_joins( collection => $sb );
+    $self->_optimize_joins( collection => $collection );
 
-    my $join_clause = CORE::join " CROSS JOIN ", ($sb->table ." main"), @{ $sb->{'aliases'} };
-    my %processed = map { /^\S+\s+(\S+)$/; $1 => 1 } @{ $sb->{'aliases'} };
+    my @cross = grep { lc $_->{type} eq "cross" }
+        values %{ $collection->{'joins'} };
+    my $join_clause = ( $collection->table . " main" )
+        . CORE::join( " ", map { $_->{alias_string} } @cross );
+    my %processed = map { $_->{alias} => 1 } @cross;
     $processed{'main'} = 1;
 
-    # get a @list of joins that have not been processed yet, but depend on processed join
-    my $joins = $sb->{'leftjoins'};
-    while ( my @list = grep !$processed{ $_ }
-            && $processed{ $joins->{ $_ }{'depends_on'} }, keys %$joins )
+# get a @list of joins that have not been processed yet, but depend on processed join
+    my $joins = $collection->{'joins'};
+    while ( my @list = grep !$processed{$_}
+        && $processed{ $joins->{$_}{'depends_on'} }, keys %$joins )
     {
-        foreach my $join ( @list ) {
-            $processed{ $join }++;
+        foreach my $join (@list) {
+            $processed{$join}++;
 
-            my $meta = $joins->{ $join };
+            my $meta = $joins->{$join};
             my $aggregator = $meta->{'entry_aggregator'} || 'AND';
 
             $join_clause .= $meta->{'alias_string'} . " ON ";
             my @tmp = map {
-                    ref($_)?
-                        $_->{'column'} .' '. $_->{'operator'} .' '. $_->{'value'}:
-                        $_
+                ref($_)
+                    ? $_->{'column'} . ' '
+                    . $_->{'operator'} . ' '
+                    . $_->{'value'}
+                    : $_
                 }
-                map { ('(', @$_, ')', $aggregator) } values %{ $meta->{'criteria'} };
+                map {
+                ( '(', @$_, ')', $aggregator )
+                } values %{ $meta->{'criteria'} };
+
             # delete last aggregator
             pop @tmp;
             $join_clause .= CORE::join ' ', @tmp;
         }
     }
 
-    # here we could check if there is recursion in joins by checking that all joins
-    # are processed
-    if ( my @not_processed = grep !$processed{ $_ }, keys %$joins ) {
+# here we could check if there is recursion in joins by checking that all joins
+# are processed
+    if ( my @not_processed = grep !$processed{$_}, keys %$joins ) {
         die "Unsatisfied dependency chain in joins @not_processed";
     }
     return $join_clause;
 }
 
 sub _optimize_joins {
-    my $self = shift;
-    my %args = ( collection => undef, @_ );
-    my $joins = $args{'collection'}->{'leftjoins'};
+    my $self  = shift;
+    my %args  = ( collection => undef, @_ );
+    my $joins = $args{'collection'}->{'joins'};
 
-    my %processed = map { /^\S+\s+(\S+)$/; $1 => 1 } @{ $args{'collection'}->{'aliases'} };
-    $processed{ $_ }++ foreach grep $joins->{ $_ }{'type'} ne 'LEFT', keys %$joins;
+    my %processed;
+    $processed{$_}++
+        foreach grep $joins->{$_}{'type'} ne 'LEFT', keys %$joins;
     $processed{'main'}++;
 
     my @ordered;
-    # get a @list of joins that have not been processed yet, but depend on processed join
-    # if we are talking about forest then we'll get the second level of the forest,
-    # but we should process nodes on this level at the end, so we build FILO ordered list.
-    # finally we'll get ordered list with leafes in the beginning and top most nodes at
-    # the end.
-    while ( my @list = grep !$processed{ $_ }
-            && $processed{ $joins->{ $_ }{'depends_on'} }, keys %$joins )
+
+# get a @list of joins that have not been processed yet, but depend on processed join
+# if we are talking about forest then we'll get the second level of the forest,
+# but we should process nodes on this level at the end, so we build FILO ordered list.
+# finally we'll get ordered list with leafes in the beginning and top most nodes at
+# the end.
+    while ( my @list = grep !$processed{$_}
+        && $processed{ $joins->{$_}{'depends_on'} }, keys %$joins )
     {
         unshift @ordered, @list;
-        $processed{ $_ }++ foreach @list;
+        $processed{$_}++ foreach @list;
     }
 
-    foreach my $join ( @ordered ) {
-        next if $self->may_be_null( collection => $args{'collection'}, alias => $join );
+    foreach my $join (@ordered) {
+        next
+            if $self->may_be_null(
+            collection => $args{'collection'},
+            alias      => $join
+            );
 
-        $joins->{ $join }{'alias_string'} =~ s/^\s*LEFT\s+/ /i;
-        $joins->{ $join }{'type'} = 'NORMAL';
+        $joins->{$join}{'alias_string'} =~ s/^\s*LEFT\s+/ /i;
+        $joins->{$join}{'type'} = 'NORMAL';
     }
 
 }
 
 =head2 may_be_null
 
-Takes a C<collection> and C<alias> in a hash and returns
-true if restrictions of the query allow NULLs in a table joined with
-the alias, otherwise returns false value which means that you can
-use normal join instead of left for the aliased table.
-
-Works only for queries have been built with L<Jifty::DBI::Collection/join> and
-L<Jifty::DBI::Collection/limit> methods, for other cases return true value to
-avoid fault optimizations.
+Takes a C<collection> and C<alias> in a hash and returns true if
+restrictions of the query allow NULLs in a table joined with the
+alias, otherwise returns false value which means that you can use
+normal join instead of left for the aliased table.
+
+Works only for queries have been built with
+L<Jifty::DBI::Collection/join> and L<Jifty::DBI::Collection/limit>
+methods, for other cases return true value to avoid fault
+optimizations.
 
 =cut
 
 sub may_be_null {
     my $self = shift;
-    my %args = (collection => undef, alias => undef, @_);
-    # if we have at least one subclause that is not generic then we should get out
-    # of here as we can't parse subclauses
-    return 1 if grep $_ ne 'generic_restrictions', keys %{ $args{'collection'}->{'subclauses'} };
+    my %args = ( collection => undef, alias => undef, @_ );
+
+# if we have at least one subclause that is not generic then we should get out
+# of here as we can't parse subclauses
+    return 1
+        if grep $_ ne 'generic_restrictions',
+        keys %{ $args{'collection'}->{'subclauses'} };
 
     # build full list of generic conditions
     my @conditions;
@@ -1068,16 +1041,18 @@
     }
 
     # find tables that depends on this alias and add their join conditions
-    foreach my $join ( values %{ $args{'collection'}->{'leftjoins'} } ) {
+    foreach my $join ( values %{ $args{'collection'}->{'joins'} } ) {
+
         # left joins on the left side so later we'll get 1 AND x expression
         # which equal to x, so we just skip it
         next if $join->{'type'} eq 'LEFT';
         next unless $join->{'depends_on'} eq $args{'alias'};
 
-        my @tmp = map { ('(', @$_, ')', $join->{'entry_aggregator'}) } values %{ $join->{'criteria'} };
+        my @tmp = map { ( '(', @$_, ')', $join->{'entry_aggregator'} ) }
+            values %{ $join->{'criteria'} };
         pop @tmp;
 
-        @conditions = ('(', @conditions, ')', 'AND', '(', @tmp ,')');
+        @conditions = ( '(', @conditions, ')', 'AND', '(', @tmp, ')' );
 
     }
     return 1 unless @conditions;
@@ -1087,12 +1062,15 @@
         unless ( ref $_ ) {
             push @conditions, $_;
         } elsif ( $_->{'column'} =~ /^\Q$args{'alias'}./ ) {
+
             # only operator IS allows NULLs in the aliased table
             push @conditions, lc $_->{'operator'} eq 'is';
         } elsif ( $_->{'value'} && $_->{'value'} =~ /^\Q$args{'alias'}./ ) {
+
             # right operand is our alias, such condition don't allow NULLs
             push @conditions, 0;
         } else {
+
             # conditions on other aliases
             push @conditions, 1;
         }
@@ -1100,13 +1078,12 @@
 
     # returns index of closing paren by index of openning paren
     my $closing_paren = sub {
-        my $i = shift;
+        my $i     = shift;
         my $count = 0;
         for ( ; $i < @conditions; $i++ ) {
             if ( $conditions[$i] && $conditions[$i] eq '(' ) {
                 $count++;
-            }
-            elsif ( $conditions[$i] && $conditions[$i] eq ')' ) {
+            } elsif ( $conditions[$i] && $conditions[$i] eq ')' ) {
                 $count--;
             }
             return $i unless $count;
@@ -1116,11 +1093,12 @@
 
     # solve boolean expression we have, an answer is our result
     my @tmp = ();
-    while ( defined ( my $e = shift @conditions ) ) {
+    while ( defined( my $e = shift @conditions ) ) {
+
         #warn "@tmp >>>$e<<< @conditions";
         return $e if !@conditions && !@tmp;
 
-        unless ( $e ) {
+        unless ($e) {
             if ( $conditions[0] eq ')' ) {
                 push @tmp, $e;
                 next;
@@ -1128,9 +1106,11 @@
 
             my $aggreg = uc shift @conditions;
             if ( $aggreg eq 'OR' ) {
+
                 # 0 OR x == x
                 next;
             } elsif ( $aggreg eq 'AND' ) {
+
                 # 0 AND x == 0
                 my $close_p = $closing_paren->(0);
                 splice @conditions, 0, $close_p + 1, (0);
@@ -1145,10 +1125,12 @@
 
             my $aggreg = uc shift @conditions;
             if ( $aggreg eq 'OR' ) {
+
                 # 1 OR x == 1
                 my $close_p = $closing_paren->(0);
                 splice @conditions, 0, $close_p + 1, (1);
             } elsif ( $aggreg eq 'AND' ) {
+
                 # 1 AND x == x
                 next;
             } else {
@@ -1174,26 +1156,27 @@
 
 takes an incomplete SQL SELECT statement and massages it to return a DISTINCT result set.
 
-
 =cut
 
 sub distinct_query {
     my $self         = shift;
     my $statementref = shift;
-    my $sb           = shift;
+    my $collection   = shift;
 
     # Prepend select query for DBs which allow DISTINCT on all column types.
-    $$statementref = "SELECT DISTINCT ".$sb->_preload_columns." FROM $$statementref";
+    $$statementref
+        = "SELECT DISTINCT "
+        . $collection->query_columns
+        . " FROM $$statementref";
 
-    $$statementref .= $sb->_group_clause;
-    $$statementref .= $sb->_order_clause;
+    $$statementref .= $collection->_group_clause;
+    $$statementref .= $collection->_order_clause;
 }
 
 =head2 distinct_count STATEMENTREF 
 
 takes an incomplete SQL SELECT statement and massages it to return a DISTINCT result set.
 
-
 =cut
 
 sub distinct_count {
@@ -1245,7 +1228,7 @@
 
 =head1 AUTHOR
 
-Jesse Vincent, jesse at fsck.com
+Jesse Vincent, <jesse at bestpractical.com>
 
 =head1 SEE ALSO
 

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Informix.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Informix.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Informix.pm	Tue Nov 13 13:32:26 2007
@@ -1,5 +1,3 @@
-# $Header:  $
-
 package Jifty::DBI::Handle::Informix;
 use Jifty::DBI::Handle;
 @ISA = qw(Jifty::DBI::Handle);
@@ -103,10 +101,10 @@
 sub distinct_query {
     my $self         = shift;
     my $statementref = shift;
-    my $sb           = shift;
-    my $table        = $sb->table;
+    my $collection   = shift;
+    my $table        = $collection->table;
 
-    if ( $sb->_order_clause =~ /(?<!main)\./ ) {
+    if ( $collection->_order_clause =~ /(?<!main)\./ ) {
 
         # Don't know how to do ORDER BY when the DISTINCT is in a subquery
         warn
@@ -119,8 +117,8 @@
         $$statementref
             = "SELECT * FROM $table main WHERE id IN ( SELECT DISTINCT main.id FROM $$statementref )";
     }
-    $$statementref .= $sb->_group_clause;
-    $$statementref .= $sb->_order_clause;
+    $$statementref .= $collection->_group_clause;
+    $$statementref .= $collection->_order_clause;
 }
 
 1;

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle/ODBC.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle/ODBC.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle/ODBC.pm	Tue Nov 13 13:32:26 2007
@@ -39,10 +39,10 @@
 sub build_dsn {
     my $self = shift;
     my %args = (
-        driver     => undef,
-        database   => undef,
-        host       => undef,
-        port       => undef,
+        driver   => undef,
+        database => undef,
+        host     => undef,
+        port     => undef,
         @_
     );
 
@@ -77,12 +77,11 @@
 sub distinct_query {
     my $self         = shift;
     my $statementref = shift;
-
-    my $sb = shift;
+    my $collection   = shift;
 
     $$statementref = "SELECT main.* FROM $$statementref";
-    $$statementref .= $sb->_group_clause;
-    $$statementref .= $sb->_order_clause;
+    $$statementref .= $collection->_group_clause;
+    $$statementref .= $collection->_order_clause;
 }
 
 =head2 encoding

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Oracle.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Oracle.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Oracle.pm	Tue Nov 13 13:32:26 2007
@@ -54,10 +54,9 @@
 =cut
 
 sub database_version {
-    return ''. ORA_OCI;
+    return '' . ORA_OCI;
 }
 
-
 =head2 insert
 
 Takes a table name as the first argument and assumes that the rest of
@@ -239,33 +238,33 @@
 sub distinct_query {
     my $self         = shift;
     my $statementref = shift;
-    my $sb           = shift;
-    my $table        = $sb->Table;
+    my $collection   = shift;
+    my $table        = $collection->Table;
 
     # Wrapp select query in a subselect as Oracle doesn't allow
     # DISTINCT against CLOB/BLOB column types.
-    if ( $sb->_order_clause =~ /(?<!main)\./ ) {
+    if ( $collection->_order_clause =~ /(?<!main)\./ ) {
 
         # If we are ordering by something not in 'main', we need to GROUP
         # BY and adjust the ORDER_BY accordingly
-        local $sb->{group_by}
-            = [ @{ $sb->{group_by} || [] }, { column => 'id' } ];
-        local $sb->{order_by} = [
+        local $collection->{group_by}
+            = [ @{ $collection->{group_by} || [] }, { column => 'id' } ];
+        local $collection->{order_by} = [
             map {
                       ( $_->{alias} and $_->{alias} ne "main" )
                     ? { %{$_}, column => "min(" . $_->{column} . ")" }
                     : $_
-                } @{ $sb->{order_by} }
+                } @{ $collection->{order_by} }
         ];
-        my $group = $sb->_group_clause;
-        my $order = $sb->_order_clause;
+        my $group = $collection->_group_clause;
+        my $order = $collection->_order_clause;
         $$statementref
             = "SELECT main.* FROM ( SELECT main.id FROM $$statementref $group $order ) distinctquery, $table main WHERE (main.id = distinctquery.id)";
     } else {
         $$statementref
             = "SELECT main.* FROM ( SELECT DISTINCT main.id FROM $$statementref ) distinctquery, $table main WHERE (main.id = distinctquery.id) ";
-        $$statementref .= $sb->_group_clause;
-        $$statementref .= $sb->_order_clause;
+        $$statementref .= $collection->_group_clause;
+        $$statementref .= $collection->_order_clause;
     }
 }
 

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Pg.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Pg.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Pg.pm	Tue Nov 13 13:32:26 2007
@@ -120,12 +120,13 @@
 =cut
 
 sub blob_params {
-    my $self   = shift;
+    my $self = shift;
     my $name = shift;
     my $type = shift;
 
     # Don't assign to key 'value' as it is defined later.
-    return ( { pg_type => DBD::Pg::PG_BYTEA() } ) if $type =~ /^(?:blob|bytea)$/;
+    return ( { pg_type => DBD::Pg::PG_BYTEA() } )
+        if $type =~ /^(?:blob|bytea)$/;
     return ( {} );
 }
 
@@ -171,11 +172,11 @@
     my $operator = shift;
     my $value    = shift;
 
-    if ($self->_case_insensitivity_valid($column, $operator, $value)) {
+    if ( $self->_case_insensitivity_valid( $column, $operator, $value ) ) {
         if ( $operator =~ /(?:LIKE|=|IN)/i ) {
             $column = "LOWER($column)";
-            if ($operator eq 'IN' and ref($value) eq 'ARRAY') {
-                $value = [map { "LOWER($_)" } @$value];
+            if ( $operator eq 'IN' and ref($value) eq 'ARRAY' ) {
+                $value = [ map {"LOWER($_)"} @$value ];
             } else {
                 $value = "LOWER($value)";
             }
@@ -191,44 +192,48 @@
 =cut
 
 sub distinct_query {
-  my $self         = shift;
-  my $statementref = shift;
-  my $sb           = shift;
-  my $table        = $sb->table;
-
-  if (
-    grep {
-      ( defined $_->{'alias'} and $_->{'alias'} ne 'main' )
-        || defined $_->{'function'}
-    } @{ $sb->order_by }
-    )
-  {
-
-    # If we are ordering by something not in 'main', we need to GROUP
-    # BY and adjust the ORDER_BY accordingly
-    local $sb->{group_by}
-      = [ @{ $sb->{group_by} || [] }, { column => 'id' } ];
-    local $sb->{order_by} = [
-      map {
-          my $alias = $_->{alias} || '';
-          my $column = $_->{column};
-          $alias .= '.' if $alias;
-          #warn "alias $alias => column $column\n";
-            ((!$alias or $alias eq 'main.') and $column eq 'id')
-          ? $_
-          : { %{$_}, alias => '', column => "min($alias$column)" }
-        } @{ $sb->{order_by} }
-    ];
-    my $group = $sb->_group_clause;
-    my $order = $sb->_order_clause;
-    $$statementref
-      = "SELECT ".$sb->_preload_columns." FROM ( SELECT main.id FROM $$statementref $group $order ) distinctquery, $table main WHERE (main.id = distinctquery.id)";
-  }
-  else {
-    $$statementref = "SELECT DISTINCT ".$sb->_preload_columns." FROM $$statementref";
-    $$statementref .= $sb->_group_clause;
-    $$statementref .= $sb->_order_clause;
-  }
+    my $self         = shift;
+    my $statementref = shift;
+    my $collection   = shift;
+    my $table        = $collection->table;
+
+    if (grep {
+            ( defined $_->{'alias'} and $_->{'alias'} ne 'main' )
+                || defined $_->{'function'}
+        } @{ $collection->order_by }
+        )
+    {
+
+        # If we are ordering by something not in 'main', we need to GROUP
+        # BY and adjust the ORDER_BY accordingly
+        local $collection->{group_by}
+            = [ @{ $collection->{group_by} || [] }, { column => 'id' } ];
+        local $collection->{order_by} = [
+            map {
+                my $alias = $_->{alias} || '';
+                my $column = $_->{column};
+                $alias .= '.' if $alias;
+
+                #warn "alias $alias => column $column\n";
+                ( ( !$alias or $alias eq 'main.' ) and $column eq 'id' )
+                    ? $_
+                    : { %{$_}, alias => '', column => "min($alias$column)" }
+                } @{ $collection->{order_by} }
+        ];
+        my $group = $collection->_group_clause;
+        my $order = $collection->_order_clause;
+        $$statementref
+            = "SELECT "
+            . $collection->query_columns
+            . " FROM ( SELECT main.id FROM $$statementref $group $order ) distinctquery, $table main WHERE (main.id = distinctquery.id)";
+    } else {
+        $$statementref
+            = "SELECT DISTINCT "
+            . $collection->query_columns
+            . " FROM $$statementref";
+        $$statementref .= $collection->_group_clause;
+        $$statementref .= $collection->_order_clause;
+    }
 }
 
 1;

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Sybase.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Sybase.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Handle/Sybase.pm	Tue Nov 13 13:32:26 2007
@@ -82,7 +82,7 @@
 #     my $statementref = shift;
 #     my $per_page     = shift;
 #     my $first        = shift;
-# 
+#
 # }
 
 =head2 distinct_query STATEMENTREF
@@ -95,10 +95,10 @@
 sub distinct_query {
     my $self         = shift;
     my $statementref = shift;
-    my $sb           = shift;
-    my $table        = $sb->table;
+    my $collection   = shift;
+    my $table        = $collection->table;
 
-    if ( $sb->_order_clause =~ /(?<!main)\./ ) {
+    if ( $collection->_order_clause =~ /(?<!main)\./ ) {
 
         # Don't know how to do ORDER BY when the DISTINCT is in a subquery
         warn
@@ -111,8 +111,8 @@
         $$statementref
             = "SELECT main.* FROM ( SELECT DISTINCT main.id FROM $$statementref ) distinctquery, $table main WHERE (main.id = distinctquery.id) ";
     }
-    $$statementref .= $sb->_group_clause;
-    $$statementref .= $sb->_order_clause;
+    $$statementref .= $collection->_group_clause;
+    $$statementref .= $collection->_order_clause;
 }
 
 1;

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm	Tue Nov 13 13:32:26 2007
@@ -7,9 +7,8 @@
 use Lingua::EN::Inflect ();
 use Jifty::DBI::Column  ();
 use UNIVERSAL::require  ();
-use Scalar::Util      qw(blessed);
-use Class::Trigger; # exports by default
-
+use Scalar::Util qw(blessed);
+use Class::Trigger;    # exports by default
 
 use base qw/
     Class::Data::Inheritable
@@ -19,10 +18,10 @@
 our $VERSION = '0.01';
 
 Jifty::DBI::Record->mk_classdata(qw/COLUMNS/);
-Jifty::DBI::Record->mk_classdata(qw/TABLE_NAME/ );
+Jifty::DBI::Record->mk_classdata(qw/TABLE_NAME/);
 Jifty::DBI::Record->mk_classdata(qw/_READABLE_COLS_CACHE/);
 Jifty::DBI::Record->mk_classdata(qw/_WRITABLE_COLS_CACHE/);
-Jifty::DBI::Record->mk_classdata(qw/_COLUMNS_CACHE/ );
+Jifty::DBI::Record->mk_classdata(qw/_COLUMNS_CACHE/);
 Jifty::DBI::Record->mk_classdata(qw/RECORD_MIXINS/);
 
 =head1 NAME
@@ -64,7 +63,8 @@
     $self->input_filters('Jifty::DBI::Filter::Truncate');
 
     if ( scalar(@_) == 1 ) {
-        Carp::cluck("new(\$handle) is deprecated, use new( handle => \$handle )");
+        Carp::cluck(
+            "new(\$handle) is deprecated, use new( handle => \$handle )");
         $self->_init( handle => shift );
     } else {
         $self->_init(@_);
@@ -75,8 +75,8 @@
 
 # Not yet documented here.  Should almost certainly be overloaded.
 sub _init {
-    my $self   = shift;
-    my %args   = (@_);
+    my $self = shift;
+    my %args = (@_);
     if ( $args{'handle'} ) {
         $self->_handle( $args{'handle'} );
     }
@@ -86,10 +86,10 @@
 sub import {
     my $class = shift;
     my ($flag) = @_;
-    if ($class->isa(__PACKAGE__) and defined $flag and $flag eq '-base') {
+    if ( $class->isa(__PACKAGE__) and defined $flag and $flag eq '-base' ) {
         my $descendant = (caller)[0];
         no strict 'refs';
-        push @{$descendant . '::ISA'}, $class;
+        push @{ $descendant . '::ISA' }, $class;
         shift;
 
         # run the schema callback
@@ -99,7 +99,7 @@
     $class->SUPER::import(@_);
 
     # Turn off redefinition warnings in the caller's scope
-    @_ = (warnings => 'redefine');
+    @_ = ( warnings => 'redefine' );
     goto &warnings::unimport;
 }
 
@@ -128,7 +128,6 @@
     return (%hash);
 }
 
-
 =head2 _accessible COLUMN ATTRIBUTE
 
 Private method. 
@@ -146,7 +145,7 @@
     my $self        = shift;
     my $column_name = shift;
     my $attribute   = lc( shift || '' );
-    my $col = $self->column($column_name);
+    my $col         = $self->column($column_name);
     return undef unless ( $col and $col->can($attribute) );
     return $col->$attribute();
 
@@ -199,33 +198,49 @@
 
 =head2 _init_methods_for_columns
 
-This is an internal method responsible for calling L</_init_methods_for_column> for each column that has been configured.
+This is an internal method responsible for calling
+L</_init_methods_for_column> for each column that has been configured.
 
 =cut
 
 sub _init_methods_for_columns {
     my $self = shift;
 
-    for my $column (sort keys %{ $self->COLUMNS || {} }) {
-        $self->_init_methods_for_column($self->COLUMNS->{ $column });
+    for my $column ( sort keys %{ $self->COLUMNS || {} } ) {
+        $self->_init_methods_for_column( $self->COLUMNS->{$column} );
     }
 }
 
 =head2 schema_version
 
-If present, this method must return a string in '1.2.3' format to be used to determine which columns are currently active in the schema. That is, this value is used to determine which columns are defined, based upon comparison to values set in C<till> and C<since>.
-
-If no implementation is present, the "latest" schema version is assumed, meaning that any column defining a C<till> is not active and all others are.
+If present, this method must return a string in '1.2.3' format to be
+used to determine which columns are currently active in the
+schema. That is, this value is used to determine which columns are
+defined, based upon comparison to values set in C<till> and C<since>.
+
+If no implementation is present, the "latest" schema version is
+assumed, meaning that any column defining a C<till> is not active and
+all others are.
 
 =head2 _init_methods_for_column COLUMN
 
-This method is used internally to update the symbol table for the record class to include an accessor and mutator for each column based upon the column's name.
-
-In addition, if your record class defines the method L</schema_version>, it will automatically generate methods according to whether the column currently exists for the current application schema version returned by that method. The C<schema_version> method must return a value in the same form used by C<since> and C<till>.
-
-If the column doesn't currently exist, it will create the methods, but they will die with an error message stating that the column does not exist for the current version of the application. If it does exist, a normal accessor and mutator will be created.
+This method is used internally to update the symbol table for the
+record class to include an accessor and mutator for each column based
+upon the column's name.
+
+In addition, if your record class defines the method
+L</schema_version>, it will automatically generate methods according
+to whether the column currently exists for the current application
+schema version returned by that method. The C<schema_version> method
+must return a value in the same form used by C<since> and C<till>.
+
+If the column doesn't currently exist, it will create the methods, but
+they will die with an error message stating that the column does not
+exist for the current version of the application. If it does exist, a
+normal accessor and mutator will be created.
 
-See also L<Jifty::DBI::Column/active>, L<Jifty::DBI::Schema/since>, L<Jifty::DBI::Schema/till> for more information.
+See also L<Jifty::DBI::Column/active>, L<Jifty::DBI::Schema/since>,
+L<Jifty::DBI::Schema/till> for more information.
 
 =cut
 
@@ -238,30 +253,36 @@
 
     # Make sure column has a record_class set as not all columns are added
     # through add_column
-    $column->record_class( $package ) if not $column->record_class;
+    $column->record_class($package) if not $column->record_class;
 
     # Check for the correct column type when the Storable filter is in use
     if ( grep { $_ eq 'Jifty::DBI::Filter::Storable' }
-              ($column->input_filters, $column->output_filters)
-         and $column->type !~ /^(blob|bytea)$/i)
+        ( $column->input_filters, $column->output_filters )
+            and $column->type !~ /^(blob|bytea)$/i )
     {
-        die "Column '$column_name' in @{[$column->record_class]} ".
-            "uses the Storable filter but is not of type 'blob'.\n";
+        die "Column '$column_name' in @{[$column->record_class]} "
+            . "uses the Storable filter but is not of type 'blob'.\n";
     }
 
     no strict 'refs';    # We're going to be defining subs
 
     if ( not $self->can($column_name) ) {
+
         # Accessor
         my $subref;
         if ( $column->active ) {
-            
 
             if ( $column->readable ) {
-                if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
+                if (UNIVERSAL::isa(
+                        $column->refers_to, "Jifty::DBI::Record"
+                    )
+                    )
                 {
                     $subref = sub {
-                        if ( @_ > 1 ) { Carp::carp "Value passed to column accessor.  You probably want to use the mutator." }
+                        if ( @_ > 1 ) {
+                            Carp::carp
+                                "Value passed to column accessor.  You probably want to use the mutator.";
+                        }
                         $_[0]->_to_record( $column_name,
                             $_[0]->__value($column_name) );
                     };
@@ -274,18 +295,23 @@
                     $subref = sub { $_[0]->_collection_value($column_name) };
                 } else {
                     $subref = sub {
-                        if ( @_ > 1 ) { Carp::carp "Value passed to column accessor.  You probably want to use the mutator." }
+                        if ( @_ > 1 ) {
+                            Carp::carp
+                                "Value passed to column accessor.  You probably want to use the mutator.";
+                        }
                         return ( $_[0]->_value($column_name) );
                     };
                 }
             } else {
                 $subref = sub { return '' }
             }
-        }
-        else {
-            # XXX sterling: should this be done with Class::ReturnValue instead
+        } else {
+
+           # XXX sterling: should this be done with Class::ReturnValue instead
             $subref = sub {
-                Carp::croak("column $column_name is not available for $package for schema version ".$self->schema_version);
+                Carp::croak(
+                    "column $column_name is not available for $package for schema version "
+                        . $self->schema_version );
             };
         }
         *{ $package . "::" . $column_name } = $subref;
@@ -293,11 +319,15 @@
     }
 
     if ( not $self->can( "set_" . $column_name ) ) {
+
         # Mutator
         my $subref;
         if ( $column->active ) {
             if ( $column->writable ) {
-                if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
+                if (UNIVERSAL::isa(
+                        $column->refers_to, "Jifty::DBI::Record"
+                    )
+                    )
                 {
                     $subref = sub {
                         my $self = shift;
@@ -306,7 +336,10 @@
                         $val = $val->id
                             if UNIVERSAL::isa( $val, 'Jifty::DBI::Record' );
                         return (
-                            $self->_set( column => $column_name, value => $val )
+                            $self->_set(
+                                column => $column_name,
+                                value  => $val
+                            )
                         );
                     };
                 } elsif (
@@ -315,8 +348,9 @@
                     )
                     )
                 {    # XXX elw: collections land here, now what?
-                    my $ret     = Class::ReturnValue->new();
-                    my $message = "Collection column '$column_name' not writable";
+                    my $ret = Class::ReturnValue->new();
+                    my $message
+                        = "Collection column '$column_name' not writable";
                     $ret->as_array( 0, $message );
                     $ret->as_error(
                         errno        => 3,
@@ -327,7 +361,10 @@
                 } else {
                     $subref = sub {
                         return (
-                            $_[0]->_set( column => $column_name, value => $_[1] )
+                            $_[0]->_set(
+                                column => $column_name,
+                                value  => $_[1]
+                            )
                         );
                     };
                 }
@@ -342,18 +379,19 @@
                 );
                 $subref = sub { return ( $ret->return_value ); };
             }
-        }
-        else {
-            # XXX sterling: should this be done with Class::ReturnValue instead
+        } else {
+
+           # XXX sterling: should this be done with Class::ReturnValue instead
             $subref = sub {
-                Carp::croak("column $column_name is not available for $package for schema version ".$self->schema_version);
+                Carp::croak(
+                    "column $column_name is not available for $package for schema version "
+                        . $self->schema_version );
             };
         }
         *{ $package . "::" . "set_" . $column_name } = $subref;
     }
 }
 
-
 =head2 _to_record COLUMN VALUE
 
 This B<PRIVATE> method takes a column name and a value for that column. 
@@ -375,50 +413,75 @@
     my $classname     = $column->refers_to();
     my $remote_column = $column->by() || 'id';
 
-    return       unless defined $value;
     return undef unless $classname;
-    return       unless UNIVERSAL::isa( $classname, 'Jifty::DBI::Record' );
+    return unless UNIVERSAL::isa( $classname, 'Jifty::DBI::Record' );
+
+    if ( my $prefetched = $self->prefetched($column_name) ) {
+        return $prefetched;
+    }
 
-    # XXX TODO FIXME we need to figure out the right way to call new here
-    # perhaps the handle should have an initiializer for records/collections
-    my $object = $classname->new( handle => $self->_handle );
-    $object->load_by_cols( $remote_column => $value );
+    my $object = $classname->new( $self->_new_record_args );
+    $object->load_by_cols( $remote_column => $value ) if defined $value;
     return $object;
 }
 
-sub _collection_value {
+sub _new_record_args {
     my $self = shift;
+    return ( handle => $self->_handle );
+}
 
-    my $method_name = shift;
-    return unless defined $method_name;
+sub _collection_value {
+    my $self        = shift;
+    my $column_name = shift;
 
-    my $column    = $self->column($method_name);
+    my $column    = $self->column($column_name);
     my $classname = $column->refers_to();
 
     return undef unless $classname;
     return unless UNIVERSAL::isa( $classname, 'Jifty::DBI::Collection' );
 
-    if ( my $prefetched_col = $self->_prefetched_collection($method_name)) {
-        return $prefetched_col;
+    if ( my $prefetched = $self->prefetched($column_name) ) {
+        return $prefetched;
     }
 
-    my $coll = $classname->new( handle => $self->_handle );
-    $coll->limit( column => $column->by(), value => $self->id );
+    my $coll = $classname->new( $self->_new_collection_args );
+    $coll->limit( column => $column->by, value => $self->id )
+        if $column->by and $self->id;
     return $coll;
 }
 
-sub _prefetched_collection {
-    my $self =shift;
+sub _new_collection_args {
+    my $self = shift;
+    return ( handle => $self->_handle );
+}
+
+=head2 prefetched NAME
+
+Returns the prefetched value for column of property C<NAME>, if it
+exists.
+
+=cut
+
+sub prefetched {
+    my $self        = shift;
     my $column_name = shift;
     if (@_) {
-        $self->{'_prefetched_collections'}->{$column_name} = shift;
+        my $column = $self->column($column_name);
+        if ( $column and not $column->refers_to ) {
+            warn "$column_name isn't supposed to be an object reference!";
+            return;
+        } elsif ( $column
+            and not UNIVERSAL::isa( $_[0], $column->refers_to ) )
+        {
+            warn "$column_name is supposed to be a @{[$column->refers_to]}!";
+        } else {
+            $self->{'_prefetched'}->{$column_name} = shift;
+        }
     } else {
-        return $self->{'_prefetched_collections'}->{$column_name};
+        return $self->{'_prefetched'}->{$column_name};
     }
-
 }
 
-
 =head2 add_column
 
 =cut
@@ -426,17 +489,18 @@
 sub add_column {
     my $self = shift;
     my $name = shift;
+
     #$name = lc $name;
-    
+
     $self->COLUMNS->{$name} = Jifty::DBI::Column->new()
-    unless exists $self->COLUMNS->{$name};
+        unless exists $self->COLUMNS->{$name};
     $self->_READABLE_COLS_CACHE(undef);
     $self->_WRITABLE_COLS_CACHE(undef);
-    $self->_COLUMNS_CACHE(undef );
+    $self->_COLUMNS_CACHE(undef);
     $self->COLUMNS->{$name}->name($name);
 
-    my $class = ref( $self ) || $self;
-    $self->COLUMNS->{$name}->record_class( $class );
+    my $class = ref($self) || $self;
+    $self->COLUMNS->{$name}->record_class($class);
 
     return $self->COLUMNS->{$name};
 }
@@ -452,7 +516,7 @@
 sub column {
     my $self = shift;
     my $name = ( shift || '' );
-    my $col = $self->_columns_hashref;
+    my $col  = $self->_columns_hashref;
     return undef unless $col && exists $col->{$name};
     return $col->{$name};
 
@@ -468,14 +532,20 @@
 
 sub columns {
     my $self = shift;
-    return @{$self->_COLUMNS_CACHE() || $self->_COLUMNS_CACHE([
-        sort {
-            ( ( ( $b->type || '' ) eq 'serial' )
-                <=> ( ( $a->type || '' ) eq 'serial' ) )
-                or ( ($a->sort_order || 0) <=> ($b->sort_order || 0))
-                or ( $a->name cmp $b->name )
-            } grep { $_->active } values %{ $self->_columns_hashref }
-	])}
+    return @{
+        $self->_COLUMNS_CACHE() || $self->_COLUMNS_CACHE(
+            [   sort {
+                    ( ( ( $b->type || '' ) eq 'serial' )
+                        <=> ( ( $a->type || '' ) eq 'serial' ) )
+                        or (
+                        ( $a->sort_order || 0 ) <=> ( $b->sort_order || 0 ) )
+                        or ( $a->name cmp $b->name )
+                    } grep {
+                    $_->active
+                    } values %{ $self->_columns_hashref }
+            ]
+        )
+        };
 }
 
 =head2 all_columns
@@ -490,22 +560,20 @@
     my $self = shift;
 
     # Not cached because it's not expected to be used often
-    return
-        sort {
-            ( ( ( $b->type || '' ) eq 'serial' )
-                <=> ( ( $a->type || '' ) eq 'serial' ) )
-                or ( ($a->sort_order || 0) <=> ($b->sort_order || 0))
-                or ( $a->name cmp $b->name )
-            } values %{ $self->_columns_hashref || {} }
+    return sort {
+        ( ( ( $b->type || '' ) eq 'serial' )
+            <=> ( ( $a->type || '' ) eq 'serial' ) )
+            or ( ( $a->sort_order || 0 ) <=> ( $b->sort_order || 0 ) )
+            or ( $a->name cmp $b->name )
+    } values %{ $self->_columns_hashref || {} };
 }
 
 sub _columns_hashref {
     my $self = shift;
 
-      return ($self->COLUMNS||{});
+    return ( $self->COLUMNS || {} );
 }
 
-
 # sub {{{ readable_attributes
 
 =head2 readable_attributes
@@ -516,16 +584,21 @@
 
 sub readable_attributes {
     my $self = shift;
-    return @{$self->_READABLE_COLS_CACHE() || $self->_READABLE_COLS_CACHE([sort map { $_->name } grep { $_->readable } $self->columns])};
+    return @{
+        $self->_READABLE_COLS_CACHE() || $self->_READABLE_COLS_CACHE(
+            [ sort map { $_->name } grep { $_->readable } $self->columns ]
+        )
+        };
 }
 
 =head2 serialize_metadata
 
-Returns a hash which describes how this class is stored in the database. 
-Right now, the keys are C<class>, C<table>, and C<columns>. C<class> and C<table>
-return simple scalars, but C<columns> returns a hash of C<name =&gt; value> pairs
-for all the columns in this model. See C<Jifty::DBI::Column/serialize_metadata> for 
-the format of that hash.
+Returns a hash which describes how this class is stored in the
+database.  Right now, the keys are C<class>, C<table>, and
+C<columns>. C<class> and C<table> return simple scalars, but
+C<columns> returns a hash of C<name =&gt; value> pairs for all the
+columns in this model. See C<Jifty::DBI::Column/serialize_metadata>
+for the format of that hash.
 
 
 =cut
@@ -533,25 +606,22 @@
 sub serialize_metadata {
     my $self = shift;
     return {
-            class => (ref($self) || $self),
-            table => $self->table,
-            columns => { $self->_serialize_columns },
-    }
+        class => ( ref($self) || $self ),
+        table => $self->table,
+        columns => { $self->_serialize_columns },
+    };
 }
 
 sub _serialize_columns {
     my $self = shift;
     my %serialized_columns;
-    foreach my $column ( $self->columns  ) {
+    foreach my $column ( $self->columns ) {
         $serialized_columns{ $column->name } = $column->serialize_metadata();
     }
 
     return %serialized_columns;
 }
 
-
-
-
 =head2 writable_attributes
 
 Returns a list of this table's writable columns
@@ -561,17 +631,22 @@
 
 sub writable_attributes {
     my $self = shift;
-    return @{$self->_WRITABLE_COLS_CACHE() || $self->_WRITABLE_COLS_CACHE([sort map { $_->name } grep { $_->writable } $self->columns])};
+    return @{
+        $self->_WRITABLE_COLS_CACHE() || $self->_WRITABLE_COLS_CACHE(
+            [ sort map { $_->name } grep { $_->writable } $self->columns ]
+        )
+        };
 }
 
 =head2 record values
 
-As you've probably already noticed, C<Jifty::DBI::Record> autocreates methods for your
-standard get/set accessors. It also provides you with some hooks to massage the values
-being loaded or stored.
-
-When you fetch a record value by calling C<$my_record-E<gt>some_field>, C<Jifty::DBI::Record>
-provides the following hook
+As you've probably already noticed, C<Jifty::DBI::Record> autocreates
+methods for your standard get/set accessors. It also provides you with
+some hooks to massage the values being loaded or stored.
+
+When you fetch a record value by calling
+C<$my_record-E<gt>some_field>, C<Jifty::DBI::Record> provides the
+following hook
 
 =over
 
@@ -614,7 +689,8 @@
 
 =item before_set PARAMHASH
 
-This is identical to the C<before_set_I<column_name>>, but is called for every column set.
+This is identical to the C<before_set_I<column_name>>, but is called
+for every column set.
 
 =item after_set_I<column_name> PARAMHASH
 
@@ -627,17 +703,18 @@
 
 =item after_set PARAMHASH
 
-This is identical to the C<after_set_I<column_name>>, but is called for every column set.
+This is identical to the C<after_set_I<column_name>>, but is called
+for every column set.
 
 =item validate_I<column_name> VALUE
 
 This hook is called just before updating the database. It expects the
 actual new value you're trying to set I<column_name> to. It returns
 two values.  The first is a boolean with truth indicating success. The
-second is an optional message. Note that validate_I<column_name> may be
-called outside the context of a I<set> operation to validate a potential
-value. (The Jifty application framework uses this as part of its AJAX
-validation system.)
+second is an optional message. Note that validate_I<column_name> may
+be called outside the context of a I<set> operation to validate a
+potential value. (The Jifty application framework uses this as part of
+its AJAX validation system.)
 
 =back
 
@@ -657,8 +734,10 @@
     my $column = shift;
 
     my $value = $self->__value( $column => @_ );
-    $self->_run_callback( name => "after_".$column,
-                          args => \$value);
+    $self->_run_callback(
+        name => "after_" . $column,
+        args => \$value
+    );
     return $value;
 }
 
@@ -670,51 +749,53 @@
 =cut
 
 sub __value {
-    my $self        = shift;
+    my $self = shift;
 
     my $column_name = shift;
+
     # If the requested column is actually an alias for another, resolve it.
     my $column = $self->column($column_name);
-    if  ($column   and defined $column->alias_for_column ) {
-        $column = $self->column($column->alias_for_column());
+    if ( $column and defined $column->alias_for_column ) {
+        $column      = $self->column( $column->alias_for_column() );
         $column_name = $column->name;
     }
 
     return unless ($column);
 
-
     # In the default case of "yeah, we have a value", return it as
     # fast as we can.
-    return   $self->{'values'}{$column_name}
+    return $self->{'values'}{$column_name}
         if ( $self->{'fetched'}{$column_name}
-          && $self->{'decoded'}{$column_name} );
+        && $self->{'decoded'}{$column_name} );
 
-    if ( !$self->{'fetched'}{ $column_name } and my $id = $self->id() ) {
-        my $pkey         = $self->_primary_key();
-        my $query_string = "SELECT "
+    if ( !$self->{'fetched'}{$column_name} and my $id = $self->id() ) {
+        my $pkey = $self->_primary_key();
+        my $query_string
+            = "SELECT "
             . $column_name
             . " FROM "
             . $self->table
             . " WHERE $pkey = ?";
         my $sth = $self->_handle->simple_query( $query_string, $id );
         my ($value) = eval { $sth->fetchrow_array() };
-        $self->{'values'}{ $column_name }  = $value;
-        $self->{'fetched'}{ $column_name } = 1;
+        $self->{'values'}{$column_name}  = $value;
+        $self->{'fetched'}{$column_name} = 1;
     }
-    unless ( $self->{'decoded'}{ $column_name } ) {
+    unless ( $self->{'decoded'}{$column_name} ) {
         $self->_apply_output_filters(
             column    => $column,
-            value_ref => \$self->{'values'}{ $column_name },
-        ) if exists $self->{'values'}{ $column_name };
-        $self->{'decoded'}{ $column_name } = 1;
+            value_ref => \$self->{'values'}{$column_name},
+        ) if exists $self->{'values'}{$column_name};
+        $self->{'decoded'}{$column_name} = 1;
     }
 
-    return $self->{'values'}{ $column_name };
+    return $self->{'values'}{$column_name};
 }
 
 =head2 as_hash 
 
-Returns a version of this record's readable columns rendered as a hash of key => value pairs
+Returns a version of this record's readable columns rendered as a hash
+of key => value pairs
 
 =cut
 
@@ -725,13 +806,11 @@
     return %values;
 }
 
-
-
 =head2 _set
 
-_set takes a single column name and a single unquoted value.
-It updates both the in-memory value of this column and the in-database copy.
-Subclasses can override _set to insert custom access control.
+_set takes a single column name and a single unquoted value.  It
+updates both the in-memory value of this column and the in-database
+copy.  Subclasses can override _set to insert custom access control.
 
 =cut
 
@@ -749,30 +828,30 @@
         name => "before_set",
         args => \%args,
     );
-    return $ok if( not defined $ok);
+    return $ok if ( not defined $ok );
 
     # Call the specific before_set_column triggers
     $ok = $self->_run_callback(
         name => "before_set_" . $args{column},
         args => \%args,
     );
-    return $ok if( not defined $ok);
+    return $ok if ( not defined $ok );
 
     $ok = $self->__set(%args);
     return $ok if not $ok;
 
     # Fetch the value back to make sure we have the actual value
-    my $value = $self->_value($args{column});
+    my $value = $self->_value( $args{column} );
 
     # Call the general after_set triggers
-    $self->_run_callback( 
+    $self->_run_callback(
         name => "after_set",
         args => { column => $args{column}, value => $value },
     );
 
     # Call the specific after_set_column triggers
-    $self->_run_callback( 
-        name => "after_set_" . $args{column}, 
+    $self->_run_callback(
+        name => "after_set_" . $args{column},
         args => { column => $args{column}, value => $value },
     );
 
@@ -789,7 +868,6 @@
         @_
     );
 
-
     my $ret = Class::ReturnValue->new();
 
     my $column = $self->column( $args{'column'} );
@@ -815,11 +893,17 @@
     if ( $self->{'fetched'}{ $column->name }
         || !$self->{'decoded'}{ $column->name } )
     {
-        if ((      !defined $args{'value'} && !defined $self->{'values'}{ $column->name })
-            || (   defined $args{'value'} && defined $self->{'values'}{ $column->name } 
+        if ((      !defined $args{'value'}
+                && !defined $self->{'values'}{ $column->name }
+            )
+            || (   defined $args{'value'}
+                && defined $self->{'values'}{ $column->name }
+
                 # XXX: This is a bloody hack to stringify DateTime
                 # and other objects for compares
-                && $args{value} . "" eq "" . $self->{'values'}{ $column->name }
+                && $args{value}
+                . "" eq ""
+                . $self->{'values'}{ $column->name }
             )
             )
         {
@@ -828,8 +912,6 @@
         }
     }
 
-
-
     if ( my $sub = $column->validator ) {
         my ( $ok, $msg ) = $sub->( $self, $args{'value'} );
         unless ($ok) {
@@ -842,12 +924,11 @@
             return ( $ret->return_value );
         }
     }
-    
 
     # Implement 'is distinct' checking
     if ( $column->distinct ) {
         my $ret = $self->is_distinct( $column->name, $args{'value'} );
-        return ( $ret ) if not ( $ret );
+        return ($ret) if not($ret);
     }
 
     # The blob handling will destroy $args{'value'}. But we assign
@@ -855,7 +936,8 @@
     my $unmunged_value = $args{'value'};
 
     if ( $column->type =~ /^(text|longtext|clob|blob|lob|bytea)$/i ) {
-        my $bhash = $self->_handle->blob_params( $column->name, $column->type );
+        my $bhash
+            = $self->_handle->blob_params( $column->name, $column->type );
         $bhash->{'value'} = $args{'value'};
         $args{'value'} = $bhash;
     }
@@ -886,7 +968,7 @@
         # XXX TODO primary_keys
         $self->load_by_cols( id => $self->id );
     } else {
-        $self->{'values'}{ $column->name } = $unmunged_value;
+        $self->{'values'}{ $column->name }  = $unmunged_value;
         $self->{'decoded'}{ $column->name } = 0;
     }
     $ret->as_array( 1, "The new value has been set." );
@@ -897,7 +979,7 @@
 
 C<load> can be called as a class or object method.
 
-Takes a single argument, $id. Calls load_by_cols to retrieve the row 
+Takes a single argument, $id. Calls load_by_cols to retrieve the row
 whose primary key is $id.
 
 =cut
@@ -913,24 +995,24 @@
 
 C<load_by_cols> can be called as a class or object method.
 
-Takes a hash of columns and values. Loads the first record that matches all
-keys.
+Takes a hash of columns and values. Loads the first record that
+matches all keys.
 
 The hash's keys are the columns to look at.
 
-The hash's values are either: scalar values to look for
-OR hash references which contain 'operator' and 'value'
+The hash's values are either: scalar values to look for OR hash
+references which contain 'operator' and 'value'
 
 =cut
 
 sub load_by_cols {
-    my $class    = shift;
-    my %hash = (@_);
+    my $class = shift;
+    my %hash  = (@_);
     my ($self);
-    if (ref($class)) {
-            ($self,$class) = ($class,undef);
+    if ( ref($class) ) {
+        ( $self, $class ) = ( $class, undef );
     } else {
-            $self = $class->new( handle => (delete $hash{'_handle'} || undef));
+        $self = $class->new( handle => ( delete $hash{'_handle'} || undef ) );
     }
 
     my ( @bind, @phrases );
@@ -938,14 +1020,16 @@
         if ( defined $hash{$key} && $hash{$key} ne '' ) {
             my $op;
             my $value;
-            my $function = "?";
-            my $column_obj = $self->column( $key );
-            Carp::confess("Unknown column '$key' in class '".ref($self)."'") if !defined $column_obj;
+            my $function   = "?";
+            my $column_obj = $self->column($key);
+            Carp::confess(
+                "Unknown column '$key' in class '" . ref($self) . "'" )
+                if !defined $column_obj;
             my $case_sensitive = $column_obj->case_sensitive;
             if ( ref $hash{$key} eq 'HASH' ) {
-                $op       = $hash{$key}->{operator};
-                $value    = $hash{$key}->{value};
-                $function = $hash{$key}->{function} || "?";
+                $op             = $hash{$key}->{operator};
+                $value          = $hash{$key}->{value};
+                $function       = $hash{$key}->{function} || "?";
                 $case_sensitive = $hash{$key}->{case_sensitive}
                     if exists $hash{$key}->{case_sensitive};
             } else {
@@ -953,7 +1037,8 @@
                 $value = $hash{$key};
             }
 
-            if (blessed $value && $value->isa('Jifty::DBI::Record') ) {
+            if ( blessed $value && $value->isa('Jifty::DBI::Record') ) {
+
                 # XXX TODO: check for proper foriegn keyness here
                 $value = $value->id;
             }
@@ -962,13 +1047,15 @@
             # a case-insensitive query
             if ( $self->_handle->case_sensitive && $value ) {
                 if ( $column_obj->is_string && !$case_sensitive ) {
-                    ( $key, $op, $function ) = $self->_handle->_make_clause_case_insensitive( $key, $op, $function );
-        	}
+                    ( $key, $op, $function )
+                        = $self->_handle->_make_clause_case_insensitive( $key,
+                        $op, $function );
+                }
             }
 
             push @phrases, "$key $op $function";
             push @bind,    $value;
-	} elsif (!defined $hash{$key}) {
+        } elsif ( !defined $hash{$key} ) {
             push @phrases, "$key IS NULL";
         } else {
             push @phrases, "($key IS NULL OR $key = ?)";
@@ -983,12 +1070,17 @@
         }
     }
 
-    my $query_string = "SELECT  * FROM "
+    my $query_string
+        = "SELECT  * FROM "
         . $self->table
         . " WHERE "
         . join( ' AND ', @phrases );
-    if ($class) { $self->_load_from_sql( $query_string, @bind ); return $self}
-    else {return $self->_load_from_sql( $query_string, @bind );}
+    if ($class) {
+        $self->_load_from_sql( $query_string, @bind );
+        return $self;
+    } else {
+        return $self->_load_from_sql( $query_string, @bind );
+    }
 
 }
 
@@ -1012,31 +1104,34 @@
 
 =head2 load_from_hash
 
-Takes a hashref, such as created by Jifty::DBI and populates this record's
-loaded values hash.
+Takes a hashref, such as created by Jifty::DBI and populates this
+record's loaded values hash.
 
 =cut
 
 sub load_from_hash {
-    my $class    = shift;
+    my $class   = shift;
     my $hashref = shift;
     my ($self);
 
-    if (ref($class)) {
-            ($self,$class) = ($class,undef);
+    if ( ref($class) ) {
+        ( $self, $class ) = ( $class, undef );
     } else {
-            $self = $class->new( handle => (delete $hashref->{'_handle'} || undef));
+        $self = $class->new(
+            handle => ( delete $hashref->{'_handle'} || undef ) );
     }
-    
+
     $self->{values} = {};
+
     #foreach my $f ( keys %$hashref ) { $self->{'fetched'}{  $f } = 1; }
-    foreach my $col (map {$_->name} $self->columns) {
-        next unless exists $hashref->{lc($col)};
+    foreach my $col ( map { $_->name } $self->columns ) {
+        next unless exists $hashref->{ lc($col) };
         $self->{'fetched'}{$col} = 1;
-        $self->{'values'}->{$col} = $hashref->{lc($col)};
+        $self->{'values'}->{$col} = $hashref->{ lc($col) };
 
-        }
-        #$self->{'values'}  = $hashref;
+    }
+
+    #$self->{'values'}  = $hashref;
     $self->{'decoded'} = {};
     return $self->id();
 }
@@ -1058,15 +1153,16 @@
 
     return ( 0, "Couldn't execute query" ) unless $sth;
 
-   my $hashref  = $sth->fetchrow_hashref;
-    delete $self->{values} ;
+    my $hashref = $sth->fetchrow_hashref;
+    delete $self->{values};
     $self->{'fetched'} = {};
     $self->{'decoded'} = {};
+
     #foreach my $f ( keys %$hashref ) { $self->{'fetched'}{  $f } = 1; }
-    foreach my $col (map {$_->name} $self->columns) {
-        next unless exists $hashref->{lc($col)};
+    foreach my $col ( map { $_->name } $self->columns ) {
+        next unless exists $hashref->{ lc($col) };
         $self->{'fetched'}{$col} = 1;
-        $self->{'values'}->{$col} = $hashref->{lc($col)};
+        $self->{'values'}->{$col} = $hashref->{ lc($col) };
     }
     if ( !$self->{'values'} && $sth->err ) {
         return ( 0, "Couldn't fetch row: " . $sth->err );
@@ -1084,7 +1180,7 @@
     }
 
     foreach my $f ( keys %{ $self->{'values'} } ) {
-        $self->{'fetched'}{ $f } = 1;
+        $self->{'fetched'}{$f} = 1;
     }
     return ( 1, "Found object" );
 
@@ -1102,7 +1198,10 @@
 
 =item before_create
 
-When adding the C<before_create> trigger, you can determine whether the trigger may cause an abort or not by passing the C<abortable> parameter to the C<add_trigger> method. If this is not set, then the return value is ignored regardless.
+When adding the C<before_create> trigger, you can determine whether
+the trigger may cause an abort or not by passing the C<abortable>
+parameter to the C<add_trigger> method. If this is not set, then the
+return value is ignored regardless.
 
   sub before_create {
       my $self = shift;
@@ -1118,11 +1217,15 @@
 This method is called before trying to create our row in the
 database. It's handed a reference to your paramhash. (That means it
 can modify your parameters on the fly).  C<before_create> returns a
-true or false value. If it returns C<undef> and the trigger has been added as C<abortable>, the create is aborted.
+true or false value. If it returns C<undef> and the trigger has been
+added as C<abortable>, the create is aborted.
 
 =item after_create
 
-When adding the C<after_create> trigger, you can determine whether the trigger may cause an abort or not by passing the C<abortable> parameter to the C<add_trigger> method. If this is not set, then the return value is ignored regardless.
+When adding the C<after_create> trigger, you can determine whether the
+trigger may cause an abort or not by passing the C<abortable>
+parameter to the C<add_trigger> method. If this is not set, then the
+return value is ignored regardless.
 
   sub after_create {
       my $self                    = shift;
@@ -1141,7 +1244,9 @@
 database. It gets handed a reference to the return value of the
 insert. That'll either be a true value or a L<Class::ReturnValue>.
 
-Aborting the trigger merely causes C<create> to return a false (undefined) value even thought he create may have succeeded. This prevents the loading of the record that would normally be returned.
+Aborting the trigger merely causes C<create> to return a false
+(undefined) value even thought he create may have succeeded. This
+prevents the loading of the record that would normally be returned.
 
 =back
 
@@ -1149,43 +1254,46 @@
 =cut 
 
 sub create {
-    my $class    = shift;
+    my $class   = shift;
     my %attribs = @_;
 
     my ($self);
-    if (ref($class)) {
-            ($self,$class) = ($class,undef);
+    if ( ref($class) ) {
+        ( $self, $class ) = ( $class, undef );
     } else {
-            $self = $class->new( handle => (delete $attribs{'_handle'} || undef));
+        $self = $class->new(
+            handle => ( delete $attribs{'_handle'} || undef ) );
     }
 
-
-
-    my $ok = $self->_run_callback( name => "before_create", args => \%attribs);
-    return $ok if ( not defined $ok);
+    my $ok
+        = $self->_run_callback( name => "before_create", args => \%attribs );
+    return $ok if ( not defined $ok );
 
     my $ret = $self->__create(%attribs);
 
-    $ok = $self->_run_callback( name => "after_create",
-                           args => \$ret);
-    return $ok if (not defined $ok);
-    
+    $ok = $self->_run_callback(
+        name => "after_create",
+        args => \$ret
+    );
+    return $ok if ( not defined $ok );
+
     if ($class) {
-        $self->load_by_cols(id => $ret);
+        $self->load_by_cols( id => $ret );
         return ($self);
-    }
-    else {
-     return ($ret);
+    } else {
+        return ($ret);
     }
 }
 
 sub __create {
-    my ($self, %attribs) = @_;
+    my ( $self, %attribs ) = @_;
 
     foreach my $column_name ( keys %attribs ) {
         my $column = $self->column($column_name);
         unless ($column) {
-            # "Virtual" columns beginning with __ is passed through to handle without munging.
+
+            # "Virtual" columns beginning with __ are passed through
+            # to handle without munging.
             next if $column_name =~ /^__/;
 
             Carp::confess "$column_name isn't a column we know about";
@@ -1206,33 +1314,47 @@
 
         # Implement 'is distinct' checking
         if ( $column->distinct ) {
-            my $ret = $self->is_distinct( $column_name, $attribs{$column_name} );
-            if (not $ret ) {
-                Carp::cluck("$self failed a 'is_distinct' check for $column_name on ".$attribs{$column_name});
-            return ( $ret ) 
+            my $ret
+                = $self->is_distinct( $column_name, $attribs{$column_name} );
+            if ( not $ret ) {
+                Carp::cluck(
+                    "$self failed a 'is_distinct' check for $column_name on "
+                        . $attribs{$column_name} );
+                return ($ret);
             }
         }
 
         if ( $column->type =~ /^(text|longtext|clob|blob|lob|bytea)$/i ) {
-            my $bhash = $self->_handle->blob_params( $column_name, $column->type );
+            my $bhash
+                = $self->_handle->blob_params( $column_name, $column->type );
             $bhash->{'value'} = $attribs{$column_name};
             $attribs{$column_name} = $bhash;
         }
     }
 
-    for my $column ($self->columns) {
-        if (not defined $attribs{$column->name} and defined $column->default and not ref $column->default) {
-            $attribs{$column->name} = $column->default;
+    for my $column ( $self->columns ) {
+        if (    not defined $attribs{ $column->name }
+            and defined $column->default
+            and not ref $column->default )
+        {
+            $attribs{ $column->name } = $column->default;
         }
 
-        if (not defined $attribs{$column->name} and $column->mandatory and $column->type ne "serial" ) {
+        if (    not defined $attribs{ $column->name }
+            and $column->mandatory
+            and $column->type ne "serial" )
+        {
+
             # Enforce "mandatory"
-            Carp::carp "Did not supply value for mandatory column ".$column->name;
-            unless ($column->active) {
-                Carp::carp "The mandatory column ".$column->name." is no longer active. This is likely to cause problems!";
+            Carp::carp "Did not supply value for mandatory column "
+                . $column->name;
+            unless ( $column->active ) {
+                Carp::carp "The mandatory column "
+                    . $column->name
+                    . " is no longer active. This is likely to cause problems!";
             }
 
-            return ( 0 );
+            return (0);
         }
     }
 
@@ -1244,7 +1366,7 @@
 Delete this record from the database. On failure return a
 Class::ReturnValue with the error. On success, return 1;
 
-This method has two hooks
+This method has two hooks:
 
 =over 
 
@@ -1254,8 +1376,8 @@
 failure it returns a L<Class::ReturnValue> with the error.  On success
 it returns 1.
 
-If this method returns an error, it causes the delete to abort and return
-the return value from this hook.
+If this method returns an error, it causes the delete to abort and
+return the return value from this hook. 
 
 =item after_delete
 
@@ -1269,12 +1391,12 @@
 sub delete {
     my $self = shift;
     my $before_ret = $self->_run_callback( name => 'before_delete' );
-    return $before_ret unless (defined $before_ret);
+    return $before_ret unless ( defined $before_ret );
     my $ret = $self->__delete;
 
     my $after_ret
         = $self->_run_callback( name => 'after_delete', args => \$ret );
-    return $after_ret unless (defined $after_ret);
+    return $after_ret unless ( defined $after_ret );
     return ($ret);
 
 }
@@ -1287,7 +1409,7 @@
 
     ## Constructs the where clause.
     my %pkeys = $self->primary_keys();
-    my $return       = $self->_handle->delete( $self->table, $self->primary_keys );
+    my $return = $self->_handle->delete( $self->table, $self->primary_keys );
 
     if ( UNIVERSAL::isa( 'Class::ReturnValue', $return ) ) {
         return ($return);
@@ -1312,20 +1434,22 @@
 
 sub table {
     my $self = shift;
-    $self->TABLE_NAME($self->_guess_table_name) unless ($self->TABLE_NAME());
+    $self->TABLE_NAME( $self->_guess_table_name )
+        unless ( $self->TABLE_NAME() );
     return $self->TABLE_NAME();
 }
 
 =head2 collection_class
 
-Returns the collection class which this record belongs to; override this to
-subclass.  If you haven't specified a collection class, this returns a best
-guess at the name of the collection class for this collection.
-
-It uses a simple heuristic to determine the collection class name -- It
-appends "Collection" to its own name. If you want to name your records
-and collections differently, go right ahead, but don't say we didn't
-warn you.
+Returns the collection class which this record belongs to; override
+this to subclass.  If you haven't specified a collection class, this
+returns a best guess at the name of the collection class for this
+collection.
+
+It uses a simple heuristic to determine the collection class name --
+It appends "Collection" to its own name. If you want to name your
+records and collections differently, go right ahead, but don't say we
+didn't warn you.
 
 =cut
 
@@ -1373,7 +1497,6 @@
 
 used for the declarative syntax
 
-
 =cut
 
 sub _filters {
@@ -1411,8 +1534,8 @@
     my $action = $args{'direction'} eq 'output' ? 'decode' : 'encode';
     foreach my $filter_class (@filters) {
         local $UNIVERSAL::require::ERROR;
-        $filter_class->require() unless 
-         $INC{ join('/', split(/::/,$filter_class)).".pm" };
+        $filter_class->require()
+            unless $INC{ join( '/', split( /::/, $filter_class ) ) . ".pm" };
 
         if ($UNIVERSAL::require::ERROR) {
             warn $UNIVERSAL::require::ERROR;
@@ -1440,17 +1563,17 @@
 =cut 
 
 sub is_distinct {
-    my $self = shift;
+    my $self   = shift;
     my $column = shift;
-    my $value = shift;
+    my $value  = shift;
 
-    my $record = $self->new( handle => $self->_handle );
-    $record->load_by_cols ( $column => $value );
+    my $record = $self->new( $self->_new_record_args );
+    $record->load_by_cols( $column => $value );
 
     my $ret = Class::ReturnValue->new();
 
-    if( $record->id ) {
-        $ret->as_array( 0, "Value already exists for unique column $column");
+    if ( $record->id ) {
+        $ret->as_array( 0, "Value already exists for unique column $column" );
         $ret->as_error(
             errno        => 3,
             do_backtrace => 0,
@@ -1462,7 +1585,6 @@
     }
 }
 
-
 =head2 run_canonicalization_for_column column => 'COLUMN', value => 'VALUE'
 
 Runs all canonicalizers for the specified column.
@@ -1471,13 +1593,22 @@
 
 sub run_canonicalization_for_column {
     my $self = shift;
-    my %args = ( column => undef,
-                 value => undef,
-                 @_);
+    my %args = (
+        column => undef,
+        value  => undef,
+        @_
+    );
 
-    my ($ret,$value_ref) = $self->_run_callback ( name => "canonicalize_".$args{'column'}, args => $args{'value'});
+    my ( $ret, $value_ref ) = $self->_run_callback(
+        name => "canonicalize_" . $args{'column'},
+        args => $args{'value'}
+    );
     return unless defined $ret;
-    return ( exists $value_ref->[-1]->[0] ? $value_ref->[-1]->[0] : $args{'value'});
+    return (
+        exists $value_ref->[-1]->[0]
+        ? $value_ref->[-1]->[0]
+        : $args{'value'}
+    );
 }
 
 =head2 has_canonicalizer_for_column COLUMN
@@ -1487,17 +1618,16 @@
 =cut
 
 sub has_canonicalizer_for_column {
-    my $self = shift;
-    my $key = shift;
-        my $method = "canonicalize_$key";
-     if( $self->can($method) ) {
-         return 1;
-     } else {
-         return undef;
-     }
+    my $self   = shift;
+    my $key    = shift;
+    my $method = "canonicalize_$key";
+    if ( $self->can($method) ) {
+        return 1;
+    } else {
+        return undef;
+    }
 }
 
-
 =head2 run_validation_for_column column => 'COLUMN', value => 'VALUE'
 
 Runs all validators for the specified column.
@@ -1511,19 +1641,18 @@
         value  => undef,
         @_
     );
-    my $key    = $args{'column'};
-    my $attr   = $args{'value'};
-
+    my $key  = $args{'column'};
+    my $attr = $args{'value'};
 
-    my ($ret, $results)  = $self->_run_callback( name => "validate_".$key, args => $attr );
+    my ( $ret, $results )
+        = $self->_run_callback( name => "validate_" . $key, args => $attr );
 
-    if (defined $ret) {
+    if ( defined $ret ) {
         return ( 1, 'Validation ok' );
+    } else {
+        return ( @{ $results->[-1] } );
     }
-    else {
-        return (@{ $results->[-1]});
-    }
-    
+
 }
 
 =head2 has_validator_for_column COLUMN
@@ -1542,7 +1671,6 @@
     }
 }
 
-
 sub _run_callback {
     my $self = shift;
     my %args = (
@@ -1556,14 +1684,15 @@
     my @results;
     if ( my $func = $self->can($method) ) {
         @results = $func->( $self, $args{args} );
-        return ( wantarray ? ( undef, [[@results]] ) : undef )
+        return ( wantarray ? ( undef, [ [@results] ] ) : undef )
             unless $results[0];
     }
     $ret = $self->call_trigger( $args{'name'} => $args{args} );
     return (
         wantarray
         ? ( $ret, [ [@results], @{ $self->last_trigger_results } ] )
-        : $ret );
+        : $ret
+    );
 }
 
 1;
@@ -1574,7 +1703,9 @@
 
 =head1 AUTHOR
 
-Jesse Vincent <jesse at bestpractical.com>, Alex Vandiver <alexmv at bestpractical.com>, David Glasser <glasser at bestpractical.com>, Ruslan Zakirov <ruslan.zakirov at gmail.com>
+Jesse Vincent <jesse at bestpractical.com>, Alex Vandiver
+<alexmv at bestpractical.com>, David Glasser <glasser at bestpractical.com>,
+Ruslan Zakirov <ruslan.zakirov at gmail.com>
 
 Based on DBIx::SearchBuilder::Record, whose credits read:
 
@@ -1584,8 +1715,6 @@
 
 =head1 SEE ALSO
 
-L<Jifty::DBI>
+L<Jifty::DBI>, L<Jifty::DBI::Handle>, L<Jifty::DBI::Collection>.
 
 =cut
-
-

Modified: Jifty-DBI/trunk/t/12prefetch.t
==============================================================================
--- Jifty-DBI/trunk/t/12prefetch.t	(original)
+++ Jifty-DBI/trunk/t/12prefetch.t	Tue Nov 13 13:32:26 2007
@@ -1,6 +1,5 @@
 #!/usr/bin/env perl -w
 
-
 use strict;
 use warnings;
 use File::Spec;
@@ -8,114 +7,175 @@
 
 BEGIN { require "t/utils.pl" }
 our (@available_drivers);
-use constant TESTS_PER_DRIVER => 41;
+use constant TESTS_PER_DRIVER => 58;
 
 my $total = scalar(@available_drivers) * TESTS_PER_DRIVER;
-#plan tests => $total;
-plan tests => TESTS_PER_DRIVER;
 
-foreach my $d ('SQLite'){ # @available_drivers ) {
+plan tests => $total;
+
+foreach my $d (@available_drivers) {
 SKIP: {
-        unless( has_schema( 'TestApp', $d ) ) {
-                skip "No schema for '$d' driver", TESTS_PER_DRIVER;
+        unless ( has_schema( 'TestApp', $d ) ) {
+            skip "No schema for '$d' driver", TESTS_PER_DRIVER;
         }
-        unless( should_test( $d ) ) {
-                skip "ENV is not defined for driver '$d'", TESTS_PER_DRIVER;
+        unless ( should_test($d) ) {
+            skip "ENV is not defined for driver '$d'", TESTS_PER_DRIVER;
         }
 
-        my $handle = get_handle( $d );
-        connect_handle( $handle );
-        isa_ok($handle->dbh, 'DBI::db', "Got handle for $d");
-
-        {my $ret = init_schema( 'TestApp', $handle );
-        isa_ok($ret,'DBI::st', "Inserted the schema. got a statement handle back" );}
+        my $handle = get_handle($d);
+        connect_handle($handle);
+        isa_ok( $handle->dbh, 'DBI::db', "Got handle for $d" );
+
+        {
+            my $ret = init_schema( 'TestApp', $handle );
+            isa_ok( $ret, 'DBI::st',
+                "Inserted the schema. got a statement handle back" );
+        }
 
-        
-        my $emp = TestApp::Employee->new( handle => $handle );
-        my $e_id = $emp->create( name => 'RUZ' );
-        ok($e_id, "Got an id for the new employee: $e_id");
+        my $emp  = TestApp::Employee->new( handle => $handle );
+        my $e_id = $emp->create( name             => 'RUZ' );
+        ok( $e_id, "Got an id for the new employee: $e_id" );
         $emp->load($e_id);
-        is($emp->id, $e_id);
-        
+        is( $emp->id, $e_id );
+
         my $phone_collection = $emp->phones;
-        isa_ok($phone_collection, 'TestApp::PhoneCollection');
-        { 
-        my $phone = TestApp::Phone->new( handle => $handle );
-        isa_ok( $phone, 'TestApp::Phone');
-        my $p_id = $phone->create( employee => $e_id, phone => '+7(903)264-03-51');
-        is($p_id, 1, "Loaded phone $p_id");
-        $phone->load( $p_id );
-
-        my $obj = $phone->employee;
-
-        ok($obj, "Employee #$e_id has phone #$p_id");
-        isa_ok( $obj, 'TestApp::Employee');
-        is($obj->id, $e_id);
-        is($obj->name, 'RUZ');
-        }
-
-        my $emp2 = TestApp::Employee->new( handle => $handle );
-        my $e2_id = $emp2->create( name => 'JESSE' );
-        my $phone2 = TestApp::Phone->new( handle => $handle );
-        my $p2_id = $phone2->create( employee => $e2_id, phone => '+16173185823');
-
-        for (3..6){
-        my $i = $_;
-        my $phone = TestApp::Phone->new( handle => $handle );
-        isa_ok( $phone, 'TestApp::Phone');
-        my $p_id = $phone->create( employee => $e_id, phone => "+1 $i");
-        is($p_id, $i, "Loaded phone $p_id");
-        $phone->load( $p_id );
-
-        my $obj = $phone->employee;
-
-        ok($obj, "Employee #$e_id has phone #$p_id");
-        isa_ok( $obj, 'TestApp::Employee');
-        is($obj->id, $e_id);
-        is($obj->name, 'RUZ');
-         
-        
-       } 
-        
+        isa_ok( $phone_collection, 'TestApp::PhoneCollection' );
+        {
+            my $phone = TestApp::Phone->new( handle => $handle );
+            isa_ok( $phone, 'TestApp::Phone' );
+            my $p_id = $phone->create(
+                employee => $e_id,
+                phone    => '+7(903)264-03-51'
+            );
+            is( $p_id, 1, "Loaded phone $p_id" );
+            $phone->load($p_id);
+
+            my $obj = $phone->employee;
+
+            ok( $obj, "Employee #$e_id has phone #$p_id" );
+            isa_ok( $obj, 'TestApp::Employee' );
+            is( $obj->id,   $e_id );
+            is( $obj->name, 'RUZ' );
+        }
+
+        my $emp2   = TestApp::Employee->new( handle => $handle );
+        my $e2_id  = $emp2->create( name            => 'JESSE' );
+        my $phone2 = TestApp::Phone->new( handle    => $handle );
+        my $p2_id
+            = $phone2->create( employee => $e2_id, phone => '+16173185823' );
+
+        for ( 3 .. 6 ) {
+            my $i = $_;
+            my $phone = TestApp::Phone->new( handle => $handle );
+            isa_ok( $phone, 'TestApp::Phone' );
+            my $p_id = $phone->create( employee => $e_id, phone => "+1 $i" );
+            is( $p_id, $i, "Loaded phone $p_id" );
+            $phone->load($p_id);
+
+            my $obj = $phone->employee;
+
+            ok( $obj, "Employee #$e_id has phone #$p_id" );
+            isa_ok( $obj, 'TestApp::Employee' );
+            is( $obj->id,   $e_id );
+            is( $obj->name, 'RUZ' );
+
+        }
+
         $handle->log_sql_statements(1);
 
-        my $collection
-            = TestApp::EmployeeCollection->new( handle => $handle );
-        my $phones_alias = $collection->join(
-            alias1  => 'main',
-            column1 => 'id',
-            table2  => 'phones',
-            column2 => 'employee'
-        );
-        $collection->prefetch( $phones_alias => 'phones'); #
-        $collection->limit( column => 'id', value => '1', operator => '>=' );
-        my $user = $collection->next;
-        is( $user->id, 1, "got our user" );
-        my $phones = $user->phones;
-        is( $phones->first->id, 1 );
-        is( $phones->count, 5 );
-
-
-        my $jesse = $collection->next;
-        is ($jesse->name, 'JESSE');
-        my $jphone = $jesse->phones;
-        is ($jphone->count,1);
+        {    # Old prefetch syntax
+            $handle->clear_sql_statement_log;
+            my $collection
+                = TestApp::EmployeeCollection->new( handle => $handle );
+            $collection->unlimit;
+            my $phones_alias = $collection->join(
+                alias1  => 'main',
+                column1 => 'id',
+                table2  => 'phones',
+                column2 => 'employee'
+            );
+            $collection->prefetch( $phones_alias => 'phones' );
+            is( $collection->count, 2 );
+            is( scalar( $handle->sql_statement_log ),
+                1, "count is one statement" );
+
+            $handle->clear_sql_statement_log;
+            my $user = $collection->next;
+            is( $user->id, 1, "got our user" );
+            my $phones = $user->phones;
+            is( $phones->first->id, 1 );
+            is( $phones->count, 5 );
+
+            my $jesse = $collection->next;
+            is( $jesse->name, 'JESSE' );
+            my $jphone = $jesse->phones;
+            is( $jphone->count, 1 );
+
+            is( scalar( $handle->sql_statement_log ),
+                1, "all that. just one sql statement" );
+        }
+
+        {    # New syntax, one-to-many
+            $handle->clear_sql_statement_log;
+            my $collection
+                = TestApp::EmployeeCollection->new( handle => $handle );
+            $collection->unlimit;
+            $collection->prefetch( name => 'phones' );
+            is( $collection->count, 2 );
+            is( scalar( $handle->sql_statement_log ),
+                1, "count is one statement" );
+
+            $handle->clear_sql_statement_log;
+            my $user = $collection->next;
+            is( $user->id, 1, "got our user" );
+            my $phones = $user->phones;
+            is( $phones->first->id, 1 );
+            is( $phones->count, 5 );
+
+            my $jesse = $collection->next;
+            is( $jesse->name, 'JESSE' );
+            my $jphone = $jesse->phones;
+            is( $jphone->count, 1 );
+
+            is( scalar( $handle->sql_statement_log ),
+                1, "all that. just one sql statement" );
+        }
 
-        my @statements = $handle->sql_statement_log;
+        {    # New syntax, one-to-one
+            $handle->clear_sql_statement_log;
+            my $collection
+                = TestApp::PhoneCollection->new( handle => $handle );
+            $collection->unlimit;
+            $collection->prefetch( name => 'employee' );
+            is( $collection->count, 6 );
+            is( scalar( $handle->sql_statement_log ),
+                1, "count is one statement" );
+
+            $handle->clear_sql_statement_log;
+            my $phone = $collection->next;
+            is( $phone->id, 1, "Got a first phone" );
+            is( $phone->phone, '+7(903)264-03-51', "Got ruz's phone number" );
+            my $employee = $phone->employee;
+            is( $employee->id, 1 );
+            is( $employee->name, "RUZ", "Employee matches" );
 
-        is (scalar @statements, 1, "all that. just one sql statement");
+            is( scalar( $handle->sql_statement_log ),
+                1, "all that. just one sql statement" );
+        }
 
         cleanup_schema( 'TestApp', $handle );
-        disconnect_handle( $handle );
-}} # SKIP, foreach blocks
+        disconnect_handle($handle);
+    }
 
-1;
 
+}    # SKIP, foreach blocks
+
+1;
 
 package TestApp;
+
 sub schema_sqlite {
-[
-q{
+    [   q{
 CREATE table employees (
         id integer primary key,
         name varchar(36)
@@ -126,11 +186,11 @@
         employee integer NOT NULL,
         phone varchar(18)
 ) }
-]
+    ];
 }
 
 sub schema_mysql {
-[ q{
+    [   q{
 CREATE TEMPORARY table employees (
         id integer AUTO_INCREMENT primary key,
         name varchar(36)
@@ -141,11 +201,12 @@
         employee integer NOT NULL,
         phone varchar(18)
 )
-} ]
+}
+    ];
 }
 
 sub schema_pg {
-[ q{
+    [   q{
 CREATE TEMPORARY table employees (
         id serial PRIMARY KEY,
         name varchar
@@ -156,34 +217,33 @@
         employee integer references employees(id),
         phone varchar
 )
-} ]
 }
-
+    ];
+}
 
 package TestApp::PhoneCollection;
 use base qw/Jifty::DBI::Collection/;
 
 sub table {
     my $self = shift;
-    my $tab = $self->new_item->table();
+    my $tab  = $self->new_item->table();
     return $tab;
 }
 
-
 package TestApp::Employee;
 use base qw/Jifty::DBI::Record/;
 
-sub _value  {
-  my $self = shift;
-  my $x =  ($self->__value(@_));
-  return $x;
+sub _value {
+    my $self = shift;
+    my $x    = ( $self->__value(@_) );
+    return $x;
 }
 
 BEGIN {
     use Jifty::DBI::Schema;
     use Jifty::DBI::Record schema {
-    column name => type is 'varchar';
-    column phones => references TestApp::PhoneCollection by 'employee';
+        column name   => type is 'varchar';
+        column phones => references TestApp::PhoneCollection by 'employee';
     }
 }
 
@@ -193,8 +253,8 @@
 BEGIN {
     use Jifty::DBI::Schema;
     use Jifty::DBI::Record schema {
-    column employee => references TestApp::Employee;
-    column phone    => type is 'varchar';
+        column employee => references TestApp::Employee;
+        column phone    => type is 'varchar';
     }
 }
 
@@ -202,6 +262,4 @@
 
 use base qw/Jifty::DBI::Collection/;
 
-
-
 1;

Modified: Jifty-DBI/trunk/t/14handle-pg.t
==============================================================================
--- Jifty-DBI/trunk/t/14handle-pg.t	(original)
+++ Jifty-DBI/trunk/t/14handle-pg.t	Tue Nov 13 13:32:26 2007
@@ -15,7 +15,7 @@
 package Foo::Bar::Collection;
 our @ISA = 'Jifty::DBI::Collection';
 
-sub _preload_columns { "blah" }
+sub query_columns { "blah" }
 sub table { "bars" }
 
 package main;


More information about the Jifty-commit mailing list