Add setPostFilter method to Elastica\Query

[krzaczek]

Hi,

Just added a missing method setPostFilter() that allows to set "post_filter" param to Elastica\Query object.
@see http://www.elasticsearch.org/guide/en/elasticsearch/guide/current/_filtering_queries_and_aggregations.html#_post_filter

Pawel

[coveralls]

Coverage increased (+0.02%) when pulling 9e6d598 on krzaczek:fix-query-post-filter into 8024d1f on ruflin:master.

[coveralls]

Coverage increased (+0.1%) when pulling 572e44b on krzaczek:fix-query-post-filter into 8024d1f on ruflin:master.

[ruflin]

@krzaczek Can you add a second "integration" test to make sure it also works directly with ES?

[krzaczek]

Can I add it to testSetPostQuery() test ... or do You prefer to put it in a seperate test ?

[coveralls]

Coverage increased (+0.02%) when pulling 27667c5 on krzaczek:fix-query-post-filter into 8024d1f on ruflin:master.

[krzaczek]

@ruflin Added new test file Query/PostFilterTest.php and placed both tests there. Hope it's ok now.

Pawel

[coveralls]

Coverage increased (+0.02%) when pulling bfc8ace on krzaczek:fix-query-post-filter into 8024d1f on ruflin:master.

[ruflin]

One detail I forgot: Can you add the pull request Id at the end of the line. I just realized the one before is also missing. If you can add also this one, I don't mind ;-)

[krzaczek]

Done

[ruflin]

Merged. Thx.

[coveralls]

Coverage increased (+0.02%) when pulling d9ca120 on krzaczek:fix-query-post-filter into 8024d1f on ruflin:master.

[MrHash]

Don't you think this should support the setting of a post filter that extends AbstractFilter as opposed to requiring an array?

[MrHash]

Please see #669

[krzaczek]

According to doc:

Also, the top-level filter parameter in search has been renamed to post_filter, to indicate that it should not be used as the primary way to filter search results (use a filtered query instead), but only to filter results AFTER facets/aggregations have been calculated.

We should probably make setFilter deprecated and change it to:

public function setFilter(AbstractFilter $filter)
    {
        return $this->setPostFilter($filer)
    }

and rewrite setPostFilter to

public function setPostFilter(AbstractFilter $filter)
    {
        return $this->setParam('post_filter', $filter->toArray());
    }

or better to make it backwards compatible ..

public function setPostFilter($filter)
    {
        if (is_array($filter)) {
            return $this->setParam("post_filter", $filter);
        } elseif ($filter instarnceof AbstractFilter) {
            return $this->setParam('post_filter', $filter->toArray());
        }
    }

[MrHash]

Personally i prefer the non-backwards compatible solution you described since the changes are fresh.

[krzaczek]

@ruflin What do You think ?

[ruflin]

As the function setFilter is used very often, I prefer the BC compatible option but we should have a way to tell the people that they are using a deprecated version (phpdoc @deprecated). Can you open a pull request with the suggestion? So we also se what kind of changes are needed on the Elastica side.

[krzaczek]

Well the BC doesn't affect setFilter method. It always required AbstractFIlter as parameter. Only the setPostFilter required array ... and it's rather new (merged 1 month ago) so the question is shoud we change the param to AbstractFIlter for setPostFilter .. or keep it BC and allow array and AbstractFilter

[ruflin]

Unfortunately there was already a release in between so I would suggest to make it BC compatible for the moment but make it clear that this will not be the case in the long run and should be cleaned up later.

[krzaczek]

OK ... i've just sent a pull request ... check it out.