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

fix filter composition #43

Closed
wants to merge 1 commit into from
Closed

fix filter composition #43

wants to merge 1 commit into from

Conversation

autre
Copy link

@autre autre commented Feb 20, 2016

This should close #42.

@autre autre changed the title fix filter composition: fix filter composition Feb 20, 2016
a filter to be applied should flatten its operands so that they
can be appended in order (instead of nest inside each other). The bug
becomes evident when applying at least three filters.
@mcorbe
Copy link

mcorbe commented Feb 26, 2016

Quick observation, this fix wont work if you nest "or" and "and" (basically would only return last bitwise operator).

You need to check for the actual type as such :

``
def and(self, x):

   if 'type' in self.filter['filter'] and self.filter['filter']['type'] == "and":
        flattened = self.flatten()
        flattened.extend(x.flatten())

        return Filter(type="and", fields=flattened)

    return Filter(type="and",
                  fields=[self.filter['filter'], x.filter['filter']])

``

@xvrl
Copy link
Member

xvrl commented Mar 8, 2016

this seems to be similar to #46. @autre can you work with @dakra to merge your two code bases and or decide which one to use?

@autre
Copy link
Author

autre commented Mar 10, 2016

@xvrl: from a quick look, seems like @dakra's pr is pretty similar (and maybe simpler/cleaner). Moreover it handles the case for more than one and/or's at filter construction time. If you think #46 is good to land, I'm ok with closing this one.

@xvrl
Copy link
Member

xvrl commented Mar 10, 2016

@autre #46 was merged, so feel free to close this one.

@autre autre closed this Mar 11, 2016
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

Successfully merging this pull request may close these issues.

Query construction results in nested filters instead of an array
3 participants