Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Phalcon\Mvc\Model\Query\Builder error if using mixed integer and string placeholder #701

Closed
jeffreycahyono opened this issue Jun 13, 2013 · 7 comments

Comments

@jeffreycahyono
Copy link

Phalcon\Mvc\Model\Query\Builder error if using mixed integer and string placeholder. It only happen when the integer placeholder is not starting from zero.
In my case I added the string placeholder using andWhere. The sql output: SQLSTATE[HY093]: Invalid parameter number: parameter was not defined

After I debuged, It turns out that the array keys of the integer placeholder in __bindParams has changed.

Example in my project:
Before andWhere

$builder->getQuery() attributes:
_phql = SELECT ai.*, ad.* FROM [BI\Models\Adindex] AS [ai] JOIN [BI\Models\Ad] AS [ad] ON ad.id=ai.ad_id WHERE (ai.adcat_parent = ?1 AND ai.status = ?2) AND (ai.ad_kind=1) ORDER BY id DESC
_bindParams = [ 1=>2, 2=>1 ]

after $builder->andWhere('ad.price >= :minprice:', ['minprice'=>$x['minprice']]);

$builder->getQuery() attributes become:
_phql = SELECT ai.*, ad.* FROM [BI\Models\Adindex] AS [ai] JOIN [BI\Models\Ad] AS [ad] ON ad.id=ai.ad_id WHERE ((ai.adcat_parent = ?1 AND ai.status = ?2) AND (ai.ad_kind=1)) AND (ad.price >= :minprice:) ORDER BY id DESC
_bindParams = [ 0=>2, 1=>1, 'minprice' => '70000000' ]

Notice that the _bindParams keys has changed

@phalcon
Copy link
Collaborator

phalcon commented Jun 13, 2013

This behavior is produced by array_merge:

$x = array_merge(array(1=>2, 2=>1), array('minprice' => '70000000'));
print_r($x);

Produces:

Array ( [0] => 2 [1] => 1 [minprice] => 70000000 )

http://www.php.net/manual/en/function.array-merge.php

@jeffreycahyono
Copy link
Author

I see.
This behaviour is not desirable. How about change it using array '+' operator for merging bindParams instead, since + operator doesn't rearrange numercial index,

@phalcon
Copy link
Collaborator

phalcon commented Jun 15, 2013

The '+' does not merge elements:

print_r(array(1 => "hello", 2 => "hola") + array(2 => "hi", "minprice" => 700));

Produces:

Array
(
    [1] => hello
    [2] => hola
    [minprice] => 700
)

It seems we need to create our own 'merge' function to fix this.

@jeffreycahyono
Copy link
Author

@phalcon:

print_r( array(2 => "hi", "minprice" => 700) + array(1 => "hello", 2 => "hola") )

Produces:

Array(
    [2] => hi
    [minprice] => 700
    [1] => hello
)

That's what we need, right?

@phalcon
Copy link
Collaborator

phalcon commented Jun 17, 2013

No, note that the value in '2' is still 'hi' if you pass a new value for '2' like in your example 'hola', the value must be replaced/merged because you don't want the old one 'hi' anymore

@jeffreycahyono
Copy link
Author

@phalcon, what I mean is we can accomplish merge and replace with '+' operator by flipping the new array to the left operand.
So We can accomplish $result=array_merge($original,$new); and still maintain numerical index by using $result=$new+$original;

@phalcon
Copy link
Collaborator

phalcon commented Jul 25, 2013

Fixed by @sjinks, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant