Sql\Predicate\Operator.php #991

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

rsandrea commented Apr 1, 2012

Regarding commit 164ccc3: $left and $right should be compared to null while validating, because if you try to create a sentence like : where field = 0, $right will be equal to 0 which will make line 80 evaluate to false, never calling $this->setRight($right)

Regarding commit f6d40d0: I don't know if this is the best way to do it, but this kind of validation is necessary because on some RDBMS (like PostgreSQL) you can't compare an integer field to a string value, it will generate an error, as it can't compare different data types.

rsandrea added some commits Apr 1, 2012

$left and $right should be compared to null while validating, because…
… if you try to create a sentence like : where field = 0, $right will be equal to 0 which will make line 80 evaluate to false, never calling $this->setRight($right)
I don't know if this is the best way to do it, but this kind of valid…
…ation is necessary because on some RDBMS (like PostgreSQL) you can't compare an integer field to a string value, it will generate an error, as it can't compare different data types.
Member

Freeaqingme commented Apr 1, 2012

Hi rsandrea,

Could you perhaps add a test or two for these changes?

Thanks.

Member

ralphschindler commented Apr 2, 2012

I'll take 164ccc3, but not f6d40d0. Basically, on the latter, unquoted values are considered constants, which is generally not something you'd ever want inside Operator literal. To be in a situation where you're not quoting something, then you should be using the new (not yet in master) functionality of using a "expression", see: zendframework#934

I can create tests for your former commit though, I'll likely merge it into my bigger pull request noted above first.

Contributor

rsandrea commented Apr 2, 2012

Hello ralphschindler, I wasn't aware of this new "expression" functionality and I do agree on rejecting f6d40d0, it made the creation of statements fail anyway.

@ghost ghost assigned ralphschindler May 4, 2012

Member

ralphschindler commented May 21, 2012

This was fixed in a6ac879, thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment