Bug #66165 for DBIx-Class: DBIC does not guess literals in where/having/select/etc

Fri Feb 25 06:47:31 2011 JHTHORSEN [...] cpan.org - Ticket created

I got an issue, when doing count() in both SQLite and Oracle database on a rather complex query. The part where it fails is when my select looks something like this: SELECT COUNT( * ) FROM ( SELECT me.colA, MAX(CASE kv.attr_name WHEN 'FOO' THEN upper(kv.attr_value) ELSE '' END) AS MAX(CASE kv__attr_name WHEN 'FOO' THEN upper(kv.attr_value) ELSE '' END) FROM table me LEFT JOIN kv kv ON kv.foo_id = me.foo_id WHERE ... ) So the issue is the "string" after "AS". The diff below fixes the problem. diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 02bceb7..c684a64 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -1525,6 +1525,7 @@ sub _count_subq_rs { if ($colpiece =~ /\./ && $colpiece !~ /^$attrs->{alias}\./) { my $as = $colpiece; $as =~ s/\./__/; + $as =~ s/\W/_/g; $colpiece = \ sprintf ('%s AS %s', map { $sql_maker->_quote ($_) } ($colpiece, $as) ); } push @{$sub_attrs->{select}}, $colpiece;

Fri Feb 25 06:54:25 2011 JHTHORSEN [...] cpan.org - Correspondence added

... AS MAX(CASE kv__attr_name WHEN 'FOO' THEN upper(kv.attr_value) ... ^^ The column alias is not valid in Oracle, nor SQLite. Reason for this is that (qw/ ( ' . /, " ") and friends are not valid when specifying the alias. I think it should be safe to just have alphanumeric characters in the alias.

Fri Feb 25 06:54:25 2011 JHTHORSEN [...] cpan.org - Status changed from 'new' to 'open'

Fri Feb 25 07:44:49 2011 JHTHORSEN [...] cpan.org - Correspondence added

Attached failing test.

Subject:

count_complex_with_having_and_invalid_as.t

use strict; use warnings; use lib qw(t/lib); use Test::More; use DBICTest; use DBIC::SqlMakerTest; use DBIC::DebugObj; my $schema = DBICTest->init_schema(); { my $aggregates = { foo => "MAX(CASE tracks.position WHEN 'foo' THEN tracks.position ELSE '' END)", bar => "MAX(CASE tracks.title WHEN 'foo' THEN tracks.title ELSE '' END)", }; my $rs = $schema->resultset("CD")->search(undef, { 'join' => 'tracks', '+select' => [ map { \ "$_"} values %$aggregates, ], '+as' => [keys %$aggregates], 'group_by' => 'me.artist', 'having' => { "MAX(CASE tracks.title WHEN 'foo' THEN upper(tracks.title) ELSE '' END)" => 'foo' }, 'page' => 1, 'rows' => 10, } ); ok(defined eval { $rs->count }, 'Complex query with " AS ..." did not die') or diag $@; }

Sun Mar 20 19:15:44 2011 ribasushi [...] leporine.io - Correspondence added

Not sure if we ever spoke about this on irc - but this bugreport is invalid. You are in essence supplying a HAVING clause that reads: HAVING $table.$column = ? where ? == 'foo' $table == "MAX(CASE tracks" $columns == "title WHEN 'foo' THEN upper(tracks.title) ELSE '' END)" With the following modification your test works fine (and fires the proper sql): --- a/t/count/count_complex_with_having_and_invalid_as.t +++ b/t/count/count_complex_with_having_and_invalid_as.t @@ -22,7 +22,9 @@ my $schema = DBICTest->init_schema(); '+select' => [ map { \ "$_"} values %$aggregates, ], '+as' => [keys %$aggregates], 'group_by' => 'me.artist', - 'having' => \"MAX(CASE tracks.title WHEN 'foo' THEN upper(tracks.title) ELSE '' END) = 'foo'", + 'having' => { + "MAX(CASE tracks.title WHEN 'foo' THEN upper(tracks.title) ELSE '' END)" => 'foo' + }, 'page' => 1, 'rows' => 10, } Please feel free to reopen this ticket if you feel there is more to it than this. Cheers!

Sun Mar 20 19:15:52 2011 ribasushi [...] leporine.io - Status changed from 'open' to 'rejected'

Tue Apr 26 08:59:54 2011 JHTHORSEN [...] cpan.org - Status changed from 'rejected' to 'open'

Tue Apr 26 09:04:08 2011 JHTHORSEN [...] cpan.org - Correspondence added

I don't get your diff. Is it reversed? Looks like "+" is what I got in the example above... Anyway, I tried both and it works nicely for all() and next(), but not for count(). Which version of DBIC do you have? Also, what about placeholders? The data comes from a web-form so I'm not too eager to put it directly inside a \"scalar ref". (Afraid of Bobby Drop-Tables, you know;)) What I see on the count() query is that it adds some strange columns to the "SELECT ...". Why does count() do anything else than just count(*)? Why does it need to have all those columns in the query?

Tue Apr 26 09:05:36 2011 JHTHORSEN [...] cpan.org - Correspondence added

And yes: I'm the guy from IRC. Sadly I've been stuck in Windows and didn't bother to find a proper IRC client. I'll hopefully be back in Ubuntu tomorrow and also on irc.perl.org#dbix-class.

Tue Apr 26 09:41:46 2011 ribasushi [...] leporine.io - Correspondence added

Subject:	Re: [rt.cpan.org #66165]
Date:	Tue, 26 Apr 2011 15:41:30 +0200
To:	bug-DBIx-Class [...] rt.cpan.org
From:	Peter Rabbitson <ribasushi [...] cpan.org>

Jan Henning Thorsen via RT wrote: Show quoted text

> Queue: DBIx-Class > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=66165 > > > And yes: I'm the guy from IRC. Sadly I've been stuck in Windows and > didn't bother to find a proper IRC client. I'll hopefully be back in > Ubuntu tomorrow and also on irc.perl.org#dbix-class.

http://search.cpan.org/~abraxxa/DBIx-Class-0.08127/lib/DBIx/Class.pm#GETTING_HELP/SUPPORT 2nd link :)

Sat Apr 30 04:10:00 2011 ribasushi [...] leporine.io - Correspondence added

Subject:

DBIC does not guess literals in select/where/having/group/etc

On Tue Apr 26 09:04:08 2011, JHTHORSEN wrote: Show quoted text

> I don't get your diff. Is it reversed? Looks like "+" is what I got in > the example above...

It's not reversed, I fucked up the having clause :) Show quoted text

> Anyway, I tried both and it works nicely for all() and next(), but not > for count(). Which version of DBIC do you have?

Didn't work how? Show quoted text

> Also, what about placeholders? The data comes from a web-form so I'm not > too eager to put it directly inside a \"scalar ref". (Afraid of Bobby > Drop-Tables, you know;))

For placeholders one has this \[ $sql_with_placeholders, [ opt1 => val1 ], [ opt2 => val2 ] ... ] Show quoted text

> What I see on the count() query is that it adds some strange columns to > the "SELECT ...". Why does count() do anything else than just count(*)? > Why does it need to have all those columns in the query?

I am not sure I am reading the right thing - I only see count(*)...

Sat Apr 30 04:18:01 2011 ribasushi [...] leporine.io - Subject changed from (no value) to 'DBIC does not guess literals in where/having/select/etc'

Sat May 07 08:30:29 2011 ribasushi [...] leporine.io - Correspondence added

I am closing this ticket as a "wontfix" reject, as all the information so far indicates abuse of the API, and not an actual bug. Feel free to reopen this ticket with more info/questions.

Sat May 07 08:30:30 2011 ribasushi [...] leporine.io - Status changed from 'open' to 'rejected'