Skip to content
This repository has been archived by the owner on Dec 29, 2023. It is now read-only.

Issue on searching under several query-like fields #38

Closed
jquinter opened this issue Mar 23, 2012 · 1 comment
Closed

Issue on searching under several query-like fields #38

jquinter opened this issue Mar 23, 2012 · 1 comment

Comments

@jquinter
Copy link

I'll try to describe what I've discover, and how I did to manage it.


Assumptions


You are familiar with cakephp, php language, and capable of understand pseudo-code explanations written very very late :)


Issue


Suppose I want to search in some model, by two fields simultaneously. Let's call them A and B. In the corresponding model, both are specified as of type "query" in the $filterArgs array: this is, both need methods which implement the conditions to be performed in the underlying real query. Let's call those methods filterA and filterB.

The problem I found rises (in its simple case/flavour) when both methods (filterA and filterB) return an array indexed by the exact same key. For example, let's say that methods filterA and filterB return something like

  function filterA($data, $field){

            ... code ...

    return array(
        'OR' => array(
                         .... conditions regarding A ...
             );
  }

  function filterB($data, $field){

            ... code ...

    return array(
        'OR' => array(
                         .... conditions regarding B ...
             );
  }

If you examine the current implementation of the searchable behaviour (under /plugins/search/models/behaviours/searchable.php), you will notice that every array obtained by calling this methods are merged all-together in the following fashion:

protected function _addCondQuery(Model $model, &$conditions, $data, $field) {
    if ((method_exists($model, $field['method']) || $this->__checkBehaviorMethods($model, $field['method'])) && !empty($data[$field['name']])) {
        $conditionsAdd = $model->{$field['method']}($data);
        $conditions = array_merge($conditions, (array)$conditionsAdd);
    }
    return $conditions;
}

the problem (which has nothing to do with the plugin) is that array_merge function will not work as expected. If you check its oficial specification at http://www.php.net/manual/en/function.array-merge.php, you will find this paragraph:

[quoting]
"Merges the elements of one or more arrays together so that the values of one are appended to the end of the previous one. It returns the resulting array.

If the input arrays have the same string keys, then the later value for that key will overwrite the previous one. If, however, the arrays contain numeric keys, the later value will not overwrite the original value, but will be appended."
[/quoting]

So what's happening right now with the plugin: if something like my example happens, the search will simply be performed just under the last field queried/submitted. The first one is just overwrittern by the second. And all because of the array_merge function.


My solution


By checking cakephp cookbook on how to build complex conditions for querying, I did notice that when you want to perform this kind of queries, you need to build it in a "onion" or "wrapped" fashion. The idea is to begin with conditions, and when you face a collision, then wrap the current set of conditions under an "AND" array, and just after that perform the merging of conditions. Let me depict it to make it just a little clearer:

[example]
I want to query under " (p1 OR p2) AND (p3 OR p4) AND p5"

//1st stage: add p5
$conditions = array(
p5
);
result (looking inside $conditions):
[0] => p5

//2nd stage: add (p3 OR p4)
$conditions['OR'] = array(p3, p4);
result (looking inside $conditions):
'OR' => (p3,p4)
[0] => p5

//3rd stage: add (p1 OR p2)
CANNOT DO
$conditions['OR'] = array(p1, p2);
result (looking inside $conditions):
'OR' => (p1,p2)
[0] => p5
(p3 OR p4 is LOST... this is what array_merge is doing)

WHAT YOU SHOULD DO
$conditions['AND'] = $conditions;
$conditions['OR'] = array(p1,p2);
result (looking inside $conditions):
'OR' => (p1,p2)
'AND' => [
'OR' => (p3, p4)
[0] => p5
]

[/example]

I implemented the above solution by updating the _addCondQuery function. Before sharing it, I would like to discuss it just a little further, so when you read my patch you understand it. I hope by this point you are not lost nor bored nor aslept. Well, by this time you should have the idea that upon a collision of keys (logical operators, such as 'OR'), the idea is to wrap the existing conditions, and then add the new one. Ok, this might work... but there's more. If you remember the default representation of conditions in cakephp,

$conditions = array( p1, p2, p3, .... )

there's an implicit operator always present: yes, the logical 'AND'. The problem with this is... suppose we face a collision on 'AND' keys! . By following the above procedure, we should wrap existing conditions inside an array under an 'AND' key, and then add the new set of conditions under ... an 'AND' key! . Got the problem? not yet? ... yesssss! the problem is that the new set of conditions WILL OVERWRITE the former set of conditions that were wrapped. Ok, this is a problem, but an easy one to solve: instead of wrapping and merging, we just need to merge both sets of conditions. The argument for this is we are operating under AND operators all the time, at the biggest ultimate level.

Well, the updated function looks like this. And it worked for me under several escenarios and apps.

protected function _addCondQuery(Model $model, &$conditions, $data, $field) {
    if ((method_exists($model, $field['method']) || $this->__checkBehaviorMethods($model, $field['method'])) && !empty($data[$field['name']])) {
        $conditionsAdd = $model->{$field['method']}($data);
        $keyAdd = array_keys($conditionsAdd);
        foreach($keyAdd as $key2add){
            if( array_key_exists($key2add, $conditions) ){
                //cannot array_merge(), because existing conditions 
                //under $key2add in $conditions array will be overwritten
                if( $key2add == 'AND' ){
                    //add this set of conditions to existing $conditions['AND']
                    $conditions['AND'] = array_merge( $conditions['AND'], $conditionsAdd['AND'] );
                    //delete value from $conditionsAdd (they're now under $conditions['AND']
                    unset( $conditionsAdd['AND'] );
                }else{
                    //wrapp them
                    $conditions['AND'] = array( $conditions ); // no keys in this wrapper, so no future collission                      
                }
            }
        }
        $conditions = array_merge($conditions, (array)$conditionsAdd);
    }
    return $conditions;
}
@skie
Copy link
Member

skie commented Apr 6, 2012

Why not just wrap you array into additional array. So filterA will looks like
function filterA($data, $field){

        ... code ...

return array(
    array(
      'OR' => array(
                     .... conditions regarding A ...
         ));

}

@skie skie closed this as completed Apr 6, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants