-
Notifications
You must be signed in to change notification settings - Fork 57
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
OR filter support for all engines #392
Comments
OrFilter concept query:
The concept behind this FilterOperation is that it will become a new type of FilterOperation, called CompoundFilterOperation or CombinedFilterOperation. Rather than having a concept of 'from' and 'to' or 'compareTo', it will contain an operator (or) and a set of input Filters to render. In this way, assuming we independently decide to implement a new type of nested combined filter (such as AND), that new type can be passed within the list. This new filter will be phased in as such:
This phased development should serve to simplify the work being done & the review process such that errors can't slip in, as with large requests. |
Good use case, just few additions to design. CompoundFilterOperation or CombinedFilterOperation filter list should support all the existing filters. Input Reporting Request looks good to me. |
At the moment, the design looks like it should be able to support all existing, as well as nesting on itself for future usage that is to say: As well as any existing FilterOperations. It should be entirely agnostic of what sort of FilterOperation passes into it. |
The current implementation assumes all items in the filter list are AND'd. We would want to keep the existing functionality intact. However, the user has the option to use logical operators as necessary (AND, OR, NOT). Here are some scenarios we should cover:
|
I understand the premise of making this a larger change that allows for top-level AND, OR, NOT however that sort of change would be quite large. It would mean breaking default behavior to allow for top-level OR, AND, NOT. The proposal here is backwards compatible by giving this new OR filter the same presedence as existing operators. In this case,
Using strictly this operator on all fields would generate a top-level OR operation without any further top-level filters. Likewise, AND and NOT can be created with the same functionality in mind. As nesting is a possibility, a stretch functionality would look something like:
|
@ryankwagner What you are proposing is the same, top level filter which is a logical operator. That's what I also said, just that the existing functionality of not having top level logical operator be preserved. E.g. if top level filter is not a logical operator, then you can just assume all filters are AND'd together, e.g. by default produce AndFilter(filters)... |
PR #415 outlines the steps that will be taken in OrFilter support more concretely. This issue can't be cleanly solved in one Pull Request, and will likely take place over 5. Please add comments/suggested edits on this first step, as it will be the basis for future filter work. |
TODO List:
Future TODOs (Features out of scope):
|
No description provided.
The text was updated successfully, but these errors were encountered: