as_query() and DBIx::Class::Cursor::Cached aren't friends

[Aran Deltac]

Just recently I upgraded DBIx::Class from 0.08010 to 0.08103 and am
getting errors about as_query() and DBIx::Class::Cursor::Cached.
Here's what the error is:

Couldn't render component "/earnings_history/earnings_history.mhtml" -
error was "Can't locate object method "as_query" via package
"DBIx::Class::Cursor::Cached" at
/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm line 2006, <DATA> line
228.
Stack:
  [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:2006]
  [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:1199]
  [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:1170]
  [/vcm/home/component/ui/mason/pub/earnings_history/earnings_history.mhtml:12]
  [/vcm/home/component/ui/mason/pub/autohandler:83]
  [/vcm/home/component/ui/mason/shared/setup.cmp:10]"

Line 12 of earnings_history.mhtml is this:

%   my $perc = sprintf("%.1f", 100 / ($media_types->count()+3) );

So, somehow a simple ->count() on a cached resultset causes the latest
DBIC to use as_query, which DBIx::Class::Cursor::Cached.

Any thoughts?  Easy fix?  Need any more details?

Thanks,

Aran

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Peter Rabbitson-2]

Aran Deltac wrote:

> Just recently I upgraded DBIx::Class from 0.08010 to 0.08103 and am
> getting errors about as_query() and DBIx::Class::Cursor::Cached.
> Here's what the error is:
>
> Couldn't render component "/earnings_history/earnings_history.mhtml" -
> error was "Can't locate object method "as_query" via package
> "DBIx::Class::Cursor::Cached" at
> /vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm line 2006, <DATA> line
> 228.
> Stack:
>   [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:2006]
>   [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:1199]
>   [/vcm/home/lib/site_perl/DBIx/Class/ResultSet.pm:1170]
>   [/vcm/home/component/ui/mason/pub/earnings_history/earnings_history.mhtml:12]
>   [/vcm/home/component/ui/mason/pub/autohandler:83]
>   [/vcm/home/component/ui/mason/shared/setup.cmp:10]"
>
> Line 12 of earnings_history.mhtml is this:
>
> %   my $perc = sprintf("%.1f", 100 / ($media_types->count()+3) );
>
> So, somehow a simple ->count() on a cached resultset causes the latest
> DBIC to use as_query, which DBIx::Class::Cursor::Cached.
>
> Any thoughts?  Easy fix?  Need any more details?
>

If a resultset has either group_by or collapse set by has_many prefetch
(which is a group_by in a sense) - then we use a subquery to get the
proper count. The as_query method was implemented on the Cursor at the
time it was written (not sure why, I wasn't the implementer). So now
that you have a different cursor, it doesn't return the SQL DBIC needs
in order to construct the subquery.

The fix should be quite simple, i.e. move as_query out of the Cursor
class. But first I'd like someone to comment on why it ended up there
in the first place.

Cheers

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Rob Kinyon]

On Sun, Jun 7, 2009 at 20:36, Peter Rabbitson <[hidden email]> wrote:
> The fix should be quite simple, i.e. move as_query out of the Cursor
> class. But first I'd like someone to comment on why it ended up there
> in the first place.

The only thing that has a query is the cursor. Everything else has the
building blocks of a query, but hasn't been materialized as such. A
ResultSet (which is where I originally thought to put it) can
materialize as all sorts of queries, so it can't live there.

The correct fix is to have Cursor::Cached implement as_query as a
delegation to Cursor. I haven't the foggiest what that would entail.

--
Thanks,
Rob Kinyon

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Peter Rabbitson-2]

Rob Kinyon wrote:

> On Sun, Jun 7, 2009 at 20:36, Peter Rabbitson <[hidden email]> wrote:
>> The fix should be quite simple, i.e. move as_query out of the Cursor
>> class. But first I'd like someone to comment on why it ended up there
>> in the first place.
>
> The only thing that has a query is the cursor. Everything else has the
> building blocks of a query, but hasn't been materialized as such. A
> ResultSet (which is where I originally thought to put it) can
> materialize as all sorts of queries, so it can't live there.
>

But this makes no sense whatsoever. A cursor is just a cursor, it knows
nothing more than what the resultset object already does. Consider:

=== lib/DBIx/Class/ResultSet.pm
==================================================================
--- lib/DBIx/Class/ResultSet.pm (revision 6543)
+++ lib/DBIx/Class/ResultSet.pm (local)
@@ -1925,7 +1925,10 @@

 =cut

-sub as_query { return shift->cursor->as_query(@_) }
+sub as_query {
+  my $self = shift;
+  return $self->result_source->storage->as_query($self);
+}

 =head2 find_or_new

=== lib/DBIx/Class/Storage/DBI/Cursor.pm
==================================================================
--- lib/DBIx/Class/Storage/DBI/Cursor.pm (revision 6543)
+++ lib/DBIx/Class/Storage/DBI/Cursor.pm (local)
@@ -49,32 +49,6 @@
   return bless ($new, $class);
 }

-=head2 as_query
-
-=over 4
-
-=item Arguments: none
-
-=item Return Value: \[ $sql, @bind ]
-
-=back
-
-Returns the SQL statement and bind vars associated with the invocant.
-
-=cut
-
-sub as_query {
-  my $self = shift;
-
-  my $storage = $self->{storage};
-  my $sql_maker = $storage->sql_maker;
-  local $sql_maker->{for};
-
-  my @args = $storage->_select_args(@{$self->{args}});
-  my ($sql, $bind)  = $storage->_prep_for_execute(@args[0 .. 2], [@args[4 .. $#args]]);
-  return \[ "($sql)", @{ $bind || [] }];
-}
-
 =head2 next

 =over 4
=== lib/DBIx/Class/Storage/DBI.pm
==================================================================
--- lib/DBIx/Class/Storage/DBI.pm (revision 6543)
+++ lib/DBIx/Class/Storage/DBI.pm (local)
@@ -910,6 +910,40 @@
   return ($sql, \@bind);
 }

+=head2 as_query
+
+=over 4
+
+=item Arguments: none
+
+=item Return Value: \[ $sql, @bind ]
+
+=back
+
+Returns the SQL statement and bind vars that would result from the given
+ResultSet (does not actually run a query)
+
+=cut
+
+sub as_query {
+  my ($self, $rs) = @_;
+
+  my $sql_maker = $self->sql_maker;
+  local $sql_maker->{for};
+
+  my $attr = $rs->_resolved_attrs;
+
+  # my ($op, $bind, $ident, $bind_attrs, $select, $cond, $order, $rows, $offset) = $self->_select_args(...);
+  my @args = $self->_select_args($attr->{from}, $attr->{select}, $attr->{where}, $attr);
+
+  # my ($sql, $bind) = $self->_prep_for_execute($op, $bind, $ident, [ $select, $cond, $order, $rows, $offset ]);
+  my ($sql, $bind) = $self->_prep_for_execute(
+    @args[0 .. 2],
+    [ @args[4 .. $#args] ],
+  );
+  return \[ "($sql)", @{ $bind || [] }];
+}
+
 sub _fix_bind_params {
     my ($self, @bind) = @_;


I am not committing this until I get more input, but it seems rather
straightforward to me.


_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Rob Kinyon]

If it passes all the tests, that's fine. But, the cursor knows more
than the resultset because the resultset isn't a query - it's a set of
potential conditions that could become an UPDATE, DELETE, or SELECT.

Rob

On Mon, Jun 8, 2009 at 05:39, Peter Rabbitson <[hidden email]> wrote:

> Rob Kinyon wrote:
>> On Sun, Jun 7, 2009 at 20:36, Peter Rabbitson <[hidden email]> wrote:
>>> The fix should be quite simple, i.e. move as_query out of the Cursor
>>> class. But first I'd like someone to comment on why it ended up there
>>> in the first place.
>>
>> The only thing that has a query is the cursor. Everything else has the
>> building blocks of a query, but hasn't been materialized as such. A
>> ResultSet (which is where I originally thought to put it) can
>> materialize as all sorts of queries, so it can't live there.
>>
>
> But this makes no sense whatsoever. A cursor is just a cursor, it knows
> nothing more than what the resultset object already does. Consider:
>
> === lib/DBIx/Class/ResultSet.pm
> ==================================================================
> --- lib/DBIx/Class/ResultSet.pm (revision 6543)
> +++ lib/DBIx/Class/ResultSet.pm (local)
> @@ -1925,7 +1925,10 @@
>
>  =cut
>
> -sub as_query { return shift->cursor->as_query(@_) }
> +sub as_query {
> +  my $self = shift;
> +  return $self->result_source->storage->as_query($self);
> +}
>
>  =head2 find_or_new
>
> === lib/DBIx/Class/Storage/DBI/Cursor.pm
> ==================================================================
> --- lib/DBIx/Class/Storage/DBI/Cursor.pm        (revision 6543)
> +++ lib/DBIx/Class/Storage/DBI/Cursor.pm        (local)
> @@ -49,32 +49,6 @@
>   return bless ($new, $class);
>  }
>
> -=head2 as_query
> -
> -=over 4
> -
> -=item Arguments: none
> -
> -=item Return Value: \[ $sql, @bind ]
> -
> -=back
> -
> -Returns the SQL statement and bind vars associated with the invocant.
> -
> -=cut
> -
> -sub as_query {
> -  my $self = shift;
> -
> -  my $storage = $self->{storage};
> -  my $sql_maker = $storage->sql_maker;
> -  local $sql_maker->{for};
> -
> -  my @args = $storage->_select_args(@{$self->{args}});
> -  my ($sql, $bind)  = $storage->_prep_for_execute(@args[0 .. 2], [@args[4 .. $#args]]);
> -  return \[ "($sql)", @{ $bind || [] }];
> -}
> -
>  =head2 next
>
>  =over 4
> === lib/DBIx/Class/Storage/DBI.pm
> ==================================================================
> --- lib/DBIx/Class/Storage/DBI.pm       (revision 6543)
> +++ lib/DBIx/Class/Storage/DBI.pm       (local)
> @@ -910,6 +910,40 @@
>   return ($sql, \@bind);
>  }
>
> +=head2 as_query
> +
> +=over 4
> +
> +=item Arguments: none
> +
> +=item Return Value: \[ $sql, @bind ]
> +
> +=back
> +
> +Returns the SQL statement and bind vars that would result from the given
> +ResultSet (does not actually run a query)
> +
> +=cut
> +
> +sub as_query {
> +  my ($self, $rs) = @_;
> +
> +  my $sql_maker = $self->sql_maker;
> +  local $sql_maker->{for};
> +
> +  my $attr = $rs->_resolved_attrs;
> +
> +  # my ($op, $bind, $ident, $bind_attrs, $select, $cond, $order, $rows, $offset) = $self->_select_args(...);
> +  my @args = $self->_select_args($attr->{from}, $attr->{select}, $attr->{where}, $attr);
> +
> +  # my ($sql, $bind) = $self->_prep_for_execute($op, $bind, $ident, [ $select, $cond, $order, $rows, $offset ]);
> +  my ($sql, $bind) = $self->_prep_for_execute(
> +    @args[0 .. 2],
> +    [ @args[4 .. $#args] ],
> +  );
> +  return \[ "($sql)", @{ $bind || [] }];
> +}
> +
>  sub _fix_bind_params {
>     my ($self, @bind) = @_;
>
>
> I am not committing this until I get more input, but it seems rather
> straightforward to me.
>
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive: http://www.grokbase.com/group/dbix-class@...
>

--
Thanks,
Rob Kinyon

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Aran Deltac]

> If a resultset has either group_by or collapse set by has_many prefetch
> (which is a group_by in a sense) - then we use a subquery to get the
> proper count. The as_query method was implemented on the Cursor at the
> time it was written (not sure why, I wasn't the implementer). So now
> that you have a different cursor, it doesn't return the SQL DBIC needs
> in order to construct the subquery.

Thanks for this - I modified my code to not use the group_by and all
is well for now.

Aran

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Matt S Trout]

On Sun, Jun 07, 2009 at 09:09:28PM -0400, Rob Kinyon wrote:
> On Sun, Jun 7, 2009 at 20:36, Peter Rabbitson <[hidden email]> wrote:
> > The fix should be quite simple, i.e. move as_query out of the Cursor
> > class. But first I'd like someone to comment on why it ended up there
> > in the first place.
>
> The only thing that has a query is the cursor. Everything else has the
> building blocks of a query, but hasn't been materialized as such.

Don't we basically materialize it during cursor construction?

I mean, $rs->all doesn't use a cursor.

So why should as_query need to? Can't it share the same logic?

--
        Matt S Trout         Catalyst and DBIx::Class consultancy with a clue
     Technical Director      and a commit bit: http://shadowcat.co.uk/catalyst/
 Shadowcat Systems Limited
  mst (@) shadowcat.co.uk        http://shadowcat.co.uk/blog/matt-s-trout/

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...

[Peter Rabbitson-2]

Hi Rob,

When you implemented subqueries you added explicit localization of the
{for} SA attribute - what is the reason you do this?

http://dev.catalyst.perl.org/svnweb/bast/revision?rev=5442#diff__DBIx-Class_0.08_branches_subquery_lib_DBIx_Class_Storage_DBI.pm

_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@...