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

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


demerphq wrote:
> 2009/3/20 Dami Laurent (PJ) <laurent.dami at justice.ge.ch>:
>> Hi all,
>>
>> I am responsible for those changes in -and/-or behaviour between 1.24
>> and 1.50.
>>
>> The point is: it was impossible to keep 1.24 behaviour on every detail,
>> because 1.24 had several internal inconsistencies, and also was
>> inconsistent with its own documentation. So understanding the old arcane
>> behaviour, finding out where it
>> differed from the doc, and deciding what to do was really a tough task.
>> Please read carefully the CHANGES Pod section in 1.50, which starts by
>> saying :
>>
>>   Great care has been taken to preserve the I<published> behavior
>>   documented in previous versions in the 1.* family; however,
>>   some features that were previously undocumented, or behaved
>>   differently from the documentation, had to be changed in order
>>   to clarify the semantics. Hence, client code that was relying
>>   on some dark areas of C<SQL::Abstract> v1.*
>>   B<might behave differently> in v1.50.
>>
>>
>> If you want more details, also read the notes marked LDNOTE in the
>> source code.
>>
>> In particular :
>>
>> - {-and/-or  => [...]} were indeed supported in older versions, but
>> meanwhile the
>>  old doc always said :
>>
>>   B<Important:> Note that the C<-modifier> goes
>>   B<INSIDE> the arrayref, as an extra first element. This will
>>   B<NOT> do what you think it might:
>>
>>       priority => -and => [{'!=', 2}, {'!=', 1}]   # WRONG!
>>
>> - nesting of -and / -or was implemented through a global variable, with
>> silly
>>  side-effects
>>
>>           # LDNOTE : previous SQLA code for hashrefs was creating a
>> dirty
>>           # side-effect: the first hashref within an array would change
>>           # the global logic to 'AND'. So [ {cond1, cond2}, [cond3,
>> cond4] ]
>>           # was interpreted as "(cond1 AND cond2) OR (cond3 AND
>> cond4)",
>>           # whereas it should be "(cond1 AND cond2) OR (cond3 OR
>> cond4)".
> 
> 
> I dont mean to be a hard ass about this, but you cant change this and
> keep the name SQL::Abstract without wreaking havoc on the
> SQL::Abstract ecosystem.
> 
> There are acres and acres and acres of code depending on the behaviour
> i illustrated.
> 
> Changing it is a serious problem.
> 
> 
> Also your example doesnt suport this change. Note what is said:
> 
> priority => -and => [{'!=', 2}, {'!=', 1}]   # WRONG!
> 
> thats because the right thing would be either of the following:
> 
> $query = ['-and',[{'priority' => {'!=' => 2}},{'priority' => {'!=' => 1}}]];
> $sql = ' WHERE ( ( ( ( priority != ? ) AND ( priority != ? ) ) ) )';
> @args = (2,1);
> ####
> $query = ['-and',['priority',{'!=' => 2},'priority',{'!=' => 1}]];
> $sql = ' WHERE ( ( ( ( priority != ? ) AND ( priority != ? ) ) ) )';
> @args = (2,1);
> ####
> 
> The crucial point here is that the docs are pointing out that you cant say:
> 
> COLUMN => OPER => [ {OP => VALUE}, {OP => VALUE}
> 
> Which is no big surprise really if you grok how hashes work.
> 
> However we arent talking about this case, we are talking about the
> case where it is
> 
> OPER => [ CLAUSES ]
> 
> which i think IS documented to work as i detailed (see the bit on the
> -nest operator).
> 
> And yes the docs are crap, and sometimes contradictory, however I
> think this one isnt open to debate. This breakage is signifigant
> enough that it should be fixed, or SQL::A 1.50 should be withdrawn and
> renamed.  And to be absolutely clear i take absolutely NO pleasure in
> saying that. I really hope that its fixable.
> 

Anything is fixable, and we already have consensus and clearly failing tests.
All we need now are volunteers to provide code and POD patches.



More information about the DBIx-Class mailing list