GROQ icon indicating copy to clipboard operation
GROQ copied to clipboard

Type errors for operators?

Open yozlet opened this issue 5 years ago • 3 comments

At present, operators just return null on type failure (e.g.: 3 + "str"). I'd much rather have this blow up with a useful error than have to check all query results for buried nulls.

yozlet avatar Jun 13 '20 21:06 yozlet

I understand that it's annoying to not get type errors and we do have a plan for improving the situation, but let me first explain the reasons for why we can't just naively add type errors:

  1. By having type errors return null it means that we can safely re-order boolean expression. Example: *[_type == "event" && externalID == "abc"] and *[externalID == "abc" && _type == "event"]. If we say that type mismatch should return an error, then the second query would return an error (assuming that there are other types where externalID is a number). Having the ability to reorder && and || is very convenient when optimizing the GROQ query at the backend.

  2. It's more expensive to fetch the data. For a query like *[_type == "event" && externalID == "abc"] with the current semantics we only need to fetch all documents which has the given _type and externalID. If we want to this to return an error on type failures then we also need to find all documents which has _type == "event" and an externalID which is not a string. The more fields there are, the more there is to fetch.

We do agree that it's annoying during development so the plan is to introduce a warning concept (see #16). This would be a flag you can pass to the backend (warnings=true) which will return additional warnings "on the side". The expression itself will return null (and the query will succeed as before), but next to the result there will be a warnings array which contains information.

Does this sound good?

judofyr avatar Jun 15 '20 08:06 judofyr

Thanks, that's a helpful explanation. I confess that I find the two given reasons to be a little odd, but my questions would probably prove your overall point: that adding type errors to an existing system turns up all kinds of ugly edge case questions.

The warning idea sounds like the best in the circumstances, as long as GROQ queries are read only (and it looks like there's no plan to change that). The nice side effect of providing multiple warnings from a query is that it shows the user all the distinct problems with the query, not just the first one. (I would consider the example you give in reason 2 to be a single problem, not a separate problem for each document with numeric IDs.)

yozlet avatar Jul 02 '20 00:07 yozlet

Thanks, that's a helpful explanation. I confess that I find the two given reasons to be a little odd, but my questions would probably prove your overall point: that adding type errors to an existing system turns up all kinds of ugly edge case questions.

I understand that some of these issues are bit vague when you're not aware of the processing we do internally to optimize it. Here's maybe a better example if you're interested: Look at the query *[_type == "person" && (upVotes - downVotes) > 0 && age >= 18]. By looking at the query alone we would assume that a document with "upVotes": 5, "downVotes": 0, "age": "18" should give a type error (since age is a string), but "upVotes": 5, "downVotes": 10, "age": "18" should be fine (because we filter it away on (upVotes - downVotes) > 0 before we try to access the age attribute).

Internally we have an index on the _type and age attributes which means that we can very quickly find the people with age >= 18. (upVotes - downVotes) > 0 on the other hand is way too complicated. What we want to do in this case is to optimize the query to use the index for finding all people over 18, and then later filter away those with (upVotes - downVotes) <= 0. You will notice that with this optimization we would not return any type error for either of those cases mentioned above.

judofyr avatar Jul 04 '20 08:07 judofyr