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

add optional "logic" enum to filters #106

Closed
wants to merge 1 commit into from
Closed

add optional "logic" enum to filters #106

wants to merge 1 commit into from

Conversation

redmitry
Copy link
Collaborator

@redmitry redmitry commented Jul 28, 2023

Proposal for the #100

The simple solution is to include optional "logic" property to the query filter.
No "logic" property is treated as 'AND'

The GET query like filters=A,B|C|D would lead to POST
filters: [{"id": "A"}, {"id": "B"}, {"id": "C", "logic": "|"}, {"id": "D", "logic": "|"}]

@redmitry redmitry requested a review from mbaudis July 28, 2023 12:36
@mbaudis
Copy link
Member

mbaudis commented Jul 28, 2023

@redmitry I don't think this is a good idea right now especially reading the last comments in the discussion. The logic w/ the sequence is compact but prone to mis-interpretations if you don't read the spec well... and then the sequence itself is problematic if using POST since all kinds of nefarious reordering might happen.

IMO we should for now just document the "same scope" concept and start a scout team for 2.n filters.

@redmitry
Copy link
Collaborator Author

The filters are passed in array so no ordering problem should happen.
The rule is quite simple - group ORs and use AND for groups. It's compatible with current spec.

@costero-e
Copy link
Collaborator

Hi @redmitry . Although I see this as a great proposal that has to be studied so we can add OR logic to our beacon specifications, I don't think this should be yet merged into any branch as it is a sensitive discussion that still needs time to get a consensus. As we seen in the previous issue discussion, beacon v2 right now is only prepared to accept AND filters without breaking the specifications and if we want to add OR logics into our queries we have to be very accurate, so I feel this is a thing that maybe needs other kind of possible solutions before accepting the one that fits best.

@redmitry redmitry closed this by deleting the head repository Sep 5, 2023
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.

3 participants