lox / pheasant

A lightweight data mapper designed to take advantage of PHP 5.3+
http://getpheasant.com
MIT License
101 stars 21 forks source link

Quick-fix to bindInto, so filter() and find() handle IN() queries equally #94

Closed bjornpost closed 10 years ago

bjornpost commented 10 years ago

Hi Lox,

We've noticed find() and filter() do not behave equally when given this syntax:

\Model\Project::find('status = ?', array('foo', 'bar');
\Model\Project::all()->filter('status = ?', array('foo', 'bar'));

The last call (right now) results in a "Parameters left over in bind" exception.

This pull request demonstrates the issue with two test cases and provides a quick/dirty(?) fix.

/cc: @jorisleker

dhotson commented 10 years ago

This issue might be related to this problem: https://github.com/lox/pheasant/pull/90/files

Can you try my branch to test your code?

jorisleker commented 10 years ago

Hi @dhotson Tried it, but it does not fix the issue we're having. Effectively we're requesting a change to the Collection->filter() parameter syntax.

Right now these are all supported syntaxes:

#A: 1 condition + 1 parameter (string)
$a = Project::all()->filter('status = ?','foo'); 

#B: 2 conditions + array containing 2 parameters (both strings)
$b = Project::all()->filter('status =? and owner = ?, array('foo','me'));

#C: 1 condition + array containing 1 parameter (an array)
$c = Project::all()->filter('status = ?',array(array('foo','bar')); 

Our fix adds support for this syntax:

#D 1 condition + 1 parameter (an array)
$d = Project::all()->filter('status = ?',array('foo','bar'); 

the filter call for $d should have the same effect as $c and is syntactically similar to $a. And to the syntax supported by find().

dhotson commented 10 years ago

Ah right, sounds like it could be a different problem then. It looked very similar to what I was seeing so I thought I'd mention it. :)

jorisleker commented 10 years ago

:-) Thanks!

@lox: why do DomainObject::find() and Collection->filter() both have a different codebase to basically transform the same set of parameters to the same type of SQL-where statements (bindInto/magicBind vs andWhere/Criteria). Wouldn't life be easier if both use the same approach?

bjornpost commented 10 years ago

@lox, can you take a look at this?

lox commented 10 years ago

I started to look into this, I'll spend some more time on it shortly.

lox commented 10 years ago

I'm thinking of removing most of the magicBind stuff and moving to native binding via mysqli or pdo.