graphql: AND/OR filter

[saihaj]

introduces alternative syntax for doing AND/OR filters.

Generated filter will have these new types

input Proposal_fitler {
  id: ID
  # ... other variants of `id` like we do today
  collection: String
  # ... other variants of `collection` like we do today
  and: [Proposal_filter]
  or: [Proposal_filter]
}

Example queries

1. using multiple AND filters

query {
  proposals(
    where: { and: [{ id_in: "111" }, { collection_contains_nocase: "a" }] }
  ) {
    id
  }
}

2. Using multiple OR filters

query {
  proposals(
    where: { or: [{ id_in: "111" }, { collection_contains_nocase: "a" }] }
  ) {
    id
  }
}

3. Using AND/OR combined

query {
  proposals(
    where: {
      or: [{ id_in: "111" }, { collection_contains_nocase: "a" }]
      and: [{ owner_contains: "0x2" }]
    }
  ) {
    id
  }
}

Downside with this approach is users can still have nested AND/OR which can be quite expensive example

query {
  proposals(
    where: {
      or: [
        { id_in: "111" }
        {
          collection_contains_nocase: "a"
          and: [{ owner_contains: "0x2" }, { id_in: "22" }]
        }
      ]
      and: [{ owner_contains: "0x2" }]
    }
  ) {
    id
  }
}

[lutter]

I am still trying to wrap my head around what this all means - the notation that GraphQL imposes on us makes this all pretty confusing.

If I have predicates a, b, c, etc. where each predicate is something like name: "Truss", where: { a, b, c } and where: { and: [a, b, c] } mean the same thing. It gets more murky with

where: {
  and: [a, b],
  or: [c, d]
}

because we need to define precedence rules. I would expect the above to mean (a and b) or c or d. That, I think, could also be written as where: { or: [ {and: [a, b]}, c, d] }.

I wonder if we should enforce the latter notation by making it illegal to have and and or at the same level, since the more explicit notation of where: { or: [ {and: [a, b]}, c, d] } is much clearer.

There's also the mixed case of where: { a, b, or: [c, d] } which would mean a and b and (c or d) and also useless cases like where: {a, b, and: [c, d] } which is the same as where: {a, b, c, d}

As for this being expensive: if people start using or a lot, especially with no additional constraints, that can become very expensive; I haven't tried it but something like where: { or: [{token_0: "ABC"}, {token_1: "ABC"}] } OTOH, we already have plenty of ways to write expensive queries.

[azf20]

Thanks so much @saihaj! I think this is close.. I wonder if we can address some of @lutter's concerns by making the approaches exclusive, i.e. the filter can only be one of the following:

Retained for backwards compatibility & simplicity (this is effectively a simple AND):

input Proposal_original_filter {
  id: ID
  # ... other variants of `id` like we do today
  collection: String
}

Allows for "or conditions":

input Proposal_and_filter {
  or: [Proposal_filter]
}

Allows for more nuanced AND filtering, sub-filters with OR etc:

input Proposal_or_filter {
  and: [Proposal_filter]
}

So in summary:

input Proposal_filter = Proposal_original_filter | Proposal_and_filter | Proposal_or_filter

[azf20]

Per @saihaj's note in Slack, we can't do a Union here, so we have to have the combined type.

One point of clarification:

where: {
  and: [a, b],
  or: [c, d]
}

I read this as a and b and (c or d), which I think is different to your assumption @lutter ((a and b) or c or d). The reason I read it like that is that different arguments at the top level are treated as one big and, rather than an or. @saihaj can you confirm which would be applied?

[azf20]

@saihaj assuming yes, @lutter and I just caught up and this syntax looks good to go, but I think we will just want to add a check to prevent and and or filters at the same level, if that makes sense? Otherwise I think this is ready for review

[azf20]

@saihaj as this is still in draft, just let us know when this is ready to take a look!

[lutter]

One thing: it would be good if there was an (undocumented) feature flag GRAPH_GRAPHQL_DISABLE_BOOL_FILTERS to turn off boolean filters. I am not sure how hard it will be to get that in, but if we could control at least whether or can be used would be very good as that has the highest potential to create slow queries. Turning off or should happen at the point where we parse filters and just result in a QueryExecutionError::NotSupported or some such. The flag should default to false (i.e., we accept filters by default)

[lutter]

This all looks good, you just need to add the environment variable that allows disabling boolean filters

[lutter]

Nice! Looks great now!