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

Peter Rabbitson rabbit+dbic at rabbit.us
Fri Mar 20 11:25:48 GMT 2009


demerphq wrote:
> 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.
> 

It was brought to my attention that I misread the POD (which is written in
a misleading way). In any case - we have had this (plain wrong, illogical)
behavior documented in the DBIC cookbook for a long time, and thus we need
to fix it.

The currently failing tests in 1.x trunk were independently verified and
appear to be correct. I looked at the source a bit - unfortunately this is
not a trivial thing to fix :(






More information about the DBIx-Class mailing list