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

Bugfix when building not filter multiple times #62

Merged
merged 1 commit into from
Dec 14, 2016

Conversation

dakra
Copy link
Contributor

@dakra dakra commented Aug 29, 2016

Make a copy of filter otherwise the field value (a :class:Filter)
of the not filter will be replaced with a dict and on next
call of build_filter with the same filter_obj variable will fail.

I didn't write an extra test but just modified the 'not filter test'
to call 2 times Filter.build_filter.

@nishantmonu51
Copy link
Member

👍

@dakra dakra force-pushed the multi-not-filter branch from 75cb42d to ceb7024 Compare August 29, 2016 10:43
actual = filters.Filter.build_filter(~f)
f = ~filters.Filter(dimension='dim', value='val')
actual = filters.Filter.build_filter(f)
actual = filters.Filter.build_filter(f)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be useful to add a comment to explain why we assign it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Fixed.

Make a copy of `filter` otherwise the `field` value (a :class:`Filter`)
of the `not` filter will be replaced with a `dict` and on next
call of `build_filter` with the same filter_obj variable will fail.
@fjy
Copy link
Member

fjy commented Dec 14, 2016

👍

@fjy fjy merged commit 5d3f7f8 into druid-io:master Dec 14, 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.

4 participants