[Dbix-class] Incorrect SQL being generated after DBIC library upgrade

demerphq demerphq at gmail.com
Thu Mar 19 21:17:42 GMT 2009


2009/3/19 Peter Rabbitson <rabbit+dbic at rabbit.us>:
> demerphq wrote:
>> 2009/3/19 Peter Rabbitson <rabbit+dbic at rabbit.us>:
>> [...]
>>> Reading your example again I noticed you are actually getting what you asked for.
>>> Consider (if we assume -and in a hashref is OK)
>>
>> FWIW I agree with the OP.  And the docs state that -and in a hashref
>> is legal as far as i recall.
>
> I couldn't find it, we should clarify this if it is the case.

Ok, "state" is a bit strong. They state little, but imply much.

<quote>
In addition to -and and -or, there is also a special -nest operator
which adds an additional set of parens, to create a subquery. For
example, to get something like this:

    $stmt = WHERE user = ? AND ( workhrs > ? OR geo = ? )
    @bind = ('nwiger', '20', 'ASIA');

You would do:

    my %where = (
         user => 'nwiger',
        -nest => [ workhrs => {'>', 20}, geo => 'ASIA' ],
    );
</quote>

Says to me that -and and -or and -nest are related, and that they can
be used inside a where hash.

And in fact, you can use all three at the same time in a hash in 1.24.

>>> 1) You say -and as in AND all the contents of the following arrayref
>>> 2) Then as first element of the array you say - OR the elements following the -or
>>>   modifier
>>> 3) The first element after OR is another arrayref - you get the
>>>   ( module_access.expires > ? OR scheme_access.expires > ? ) chunk
>>> 4) Then due to the -or keyword (2) you get ... OR me.person = ?
>>
>> It doesnt work like that, at least not in my experience.
>>
>> The '-and => [ ... ]' is self contained, the "-and" says join together
>> the next thing using "AND" (and not whatever the default join operand
>> is for that type), not "join together everything following".
>
> Precisely. You are saying -and => [ STUFF ], where:
> STUFF == [ '-or', { SUBSTUFF }, SUBSTUFF2 ]
>
> The -and [ STUFF ] asks for an unconditional AND-ing of STUFF
> The -or asks for an unconditional OR-ing of the significant bits of STUFF
> (SUBSTUFF and SUBSTUFF2). It is logical imo for the last request to win...

Again no.  the '-or' does not apply to SUBSTUFF2, it it applies to the
INSIDE of the next item in the array, or to the value when it is used
as key in a hash. Not anything following that. it is merely a way of
overriding the normal join operand semantics of the reference type.
The default clause join operand for HASHes is documented as "AND", and
the default in an ARRAY is documented as "OR". In order to change that
you need to prefix it with a modifier.

Consider the following:

Version: 1.24
$query = {'-or' => {'bar' => 2,'foo' => 1}};
$sql = ' WHERE ( ( bar = ? OR foo = ? ) )';
@args = (2,1);
####
$query = {'bar' => 2,'foo' => 1};
$sql = ' WHERE ( bar = ? AND foo = ? )';
@args = (2,1);
####
$query = [{'bar' => 2,'foo' => 1}];
$sql = ' WHERE ( ( bar = ? AND foo = ? ) )';
@args = (2,1);
####
$query = [{'bar' => 2,'foo' => 1},{'baz' => 1,'bop' => 2}];
$sql = ' WHERE ( ( bar = ? AND foo = ? ) OR ( baz = ? AND bop = ? ) )';
@args = (2,1,1,2);
####
$query = ['-and',{'bar' => 2,'foo' => 1},'-and',{'baz' => 1,'bop' => 2}];
$sql = ' WHERE ( ( ( bar = ? AND foo = ? ) ) OR ( ( baz = ? AND bop = ? ) ) )';
@args = (2,1,1,2);
####
$query = ['-and',{'bar' => 2,'foo' => 1},'-or',{'baz' => 1,'bop' => 2}];
$sql = ' WHERE ( ( ( bar = ? AND foo = ? ) ) OR ( ( baz = ? OR bop = ? ) ) )';
@args = (2,1,1,2);
####
$query = ['-or',{'bar' => 2,'foo' => 1},'-or',{'baz' => 1,'bop' => 2}];
$sql = ' WHERE ( ( ( bar = ? OR foo = ? ) ) OR ( ( baz = ? OR bop = ? ) ) )';
@args = (2,1,1,2);
####
$query = ['-or',{'bar' => 2,'foo' => 1},{'baz' => 1,'bop' => 2}];
$sql = ' WHERE ( ( ( bar = ? OR foo = ? ) ) OR ( baz = ? AND bop = ? ) )';
@args = (2,1,1,2);
####
$query = {'-or' => {'bar' => 2,'foo' => 1},'-and' => {'baz' => 1,'bop'
=> 2},'-nest' => ['foo',1]};
$sql = ' WHERE ( ( baz = ? AND bop = ? ) AND ( ( foo = ? ) ) AND ( bar
= ? OR foo = ? ) )';
@args = (1,2,1,2,1);
####

>
> I committed a test extension[1] similar to what you wrote, except that it follows
> my logic. While it is debatable which behavior is correct, it fails in either
> case, either as the OP demonstrated or as currently shown in the commit.

Personally I don't think there is much room for debate here.
Considering that 1.5 isnt even a major release bump from 1.24 it is
unarguably a regression if it does not produce the same output
(logically speaking) as it used to in 1.24 as long as what it used to
do wasn't just outright wrong (such as being broken sql, or whatever).
And I dont think that caveat applies here.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"



More information about the DBIx-Class mailing list