Skip to content

Expose noop filter to users#5597

Merged
b-slim merged 1 commit intoapache:masterfrom
scrawfor:expose_noop_filter
Apr 18, 2018
Merged

Expose noop filter to users#5597
b-slim merged 1 commit intoapache:masterfrom
scrawfor:expose_noop_filter

Conversation

@scrawfor
Copy link
Copy Markdown
Contributor

@scrawfor scrawfor commented Apr 9, 2018

It would be useful to expose the noop filter to users. This allows a filter to be "switched off" without removing the filter property from the json. This is useful when developing a query, or in tools which programmatically generate druid queries.

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Apr 11, 2018

Thanks for the contribution, can you please sign the CLA?

http://druid.io/community/cla.html

@scrawfor
Copy link
Copy Markdown
Contributor Author

Done.

@b-slim b-slim merged commit 15f4ab2 into apache:master Apr 18, 2018
@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented Apr 18, 2018

@scrawfor not sure how this is different from null filter but it is okay if you like reading noop instead.

@scrawfor
Copy link
Copy Markdown
Contributor Author

@b-slim If by null filter you mean just removing the filter property, I think this has two small use cases.

  1. We use a (limited) reporting tool which builds our JSON queries for us, which based on user input enables or disables filters. Without the noop filter, we have to check if any filter should be applied before adding a filter property. With the noop filter, we can just modify the type for each individual filter to make it inactive. It reduces the amount of logic we need to build when generating queries.

  2. When developing a query, you can just disable a filter, instead of removing the filter property entirely. At least for me, this is preferable, because I know I've removed a filter and forgotten what I had in place.

"filter": {
    "type": "and",
    "fields": [{
      "type": "selector",
      "dimension": "profile",
      "value": "my_application"
    }, {
      "type": "selector",
      "dimension": "browser_name",
      "value": "Chrome"
    }]
  }

becomes

"filter": {
    "type": "noop",
    "fields": [{
      "type": "selector",
      "dimension": "profile",
      "value": "premierconnect"
    }, {
      "type": "selector",
      "dimension": "browser_name",
      "value": "Chrome"
    }]
  },

Now I can see what my results are without the filter, and I haven't made any large modifications to my query. If I want to re-enable my filter, I just change "noop" back to "and".

@niketh
Copy link
Copy Markdown
Contributor

niketh commented Apr 19, 2018

@scrawfor You can also change "filter" to "afilter" , it does the same thing :). But this is useful too, but you have to remember which filter you have replaced with noop

@scrawfor
Copy link
Copy Markdown
Contributor Author

I've been running this in my cluster for a week or so, and just found a bug. If you change the type of a filter to noop inside of an and filter a NPE is thrown.

"filter": { "type": "and", "fields": [{ "type": "selector", "dimension": "my_dimension", "value": "some_value" }, {"type": "noop"}] },

throws

java.lang.NullPointerException at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:213) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableCollection$ArrayBasedBuilder.add(ImmutableCollection.java:339) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableList$Builder.add(ImmutableList.java:652) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableList$Builder.add(ImmutableList.java:630) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableCollection$Builder.addAll(ImmutableCollection.java:301) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableList$Builder.addAll(ImmutableList.java:691) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableList.copyOf(ImmutableList.java:275) ~[guava-16.0.1.jar:?] at com.google.common.collect.ImmutableList.copyOf(ImmutableList.java:226) ~[guava-16.0.1.jar:?] at io.druid.segment.filter.Filters.toFilters(Filters.java:84) ~[druid-processing-0.11.0.jar:0.11.0] at io.druid.query.filter.AndDimFilter.toFilter(AndDimFilter.java:75) ~[druid-processing-0.11.0.jar:0.11.0] at io.druid.segment.filter.Filters.toFilter(Filters.java:109) ~[druid-processing-0.11.0.jar:0.11.0]

Very unfamiliar with the project still, but to me the appropriate solution would be to remove any noop filters instead of trying to actually convert it to a filter. But I wanted to open it up and see if you guys had any input.

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented Apr 27, 2018

@scrawfor IMO this noop filter is no sense i think what you are asking for is true filter that matchs everything and maybe false opposite of true. How about adding a filter of type true instead of exposing noop ?

@scrawfor
Copy link
Copy Markdown
Contributor Author

I'm fine with that. It does better represent what we are actually doing. I only went with the noop because it already existed

Take a look and let me know if this is more like what you're thinking.

@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented Apr 27, 2018

Take a look and let me know if this is more like what you're thinking.

@scrawfor where ?

@scrawfor
Copy link
Copy Markdown
Contributor Author

My bad. Wan't thinking that this wouldn't be updated. I created a new PR #5711

sathishsri88 pushed a commit to sathishs/druid that referenced this pull request May 8, 2018
@dclim dclim added this to the 0.13.0 milestone Oct 8, 2018
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.

6 participants