cloudevents / spec

CloudEvents Specification
https://cloudevents.io
Apache License 2.0
5.11k stars 586 forks source link

Clarify CESQL AND and OR evaluation #1260

Closed Cali0707 closed 8 months ago

Cali0707 commented 9 months ago

In the subscriptions spec when talking about the CESQL filter it states that the result MUST be false if an error occurred while evaluating the expression.

This can cause issues because while CESQL expressions are total (they always return a result, and optionally an error), if an error occurs during the evaluation of an expression using an AND or an OR, the result of the filter would depend not only on whether or not the expression evaluated to true but how quickly it evaluated to true and whether it evaluated far enough into the expression to encounter an error. For example:

(type = "my.event.type") OR (numericAttribute / 0)

If the implementation decides to return early when evaluating an OR it would never encounter an error and so the filter would pass. However, if the implementation evaluates the entire expression it would then get the arithmetic error when the division by 0 occurs.

To ensure consistency of CESQL behaviour when used as a filter, we should specify whether the implementations of CESQL must return early when possible, or must not return early.

duglin commented 9 months ago

If the first part evaluates to "true" does the cesql spec mandate that the 2nd part of the OR be skipped or is the impl free to evaluate it any way? And if so, do you know why it doesn't mandate to stop after the first "true"?

Cali0707 commented 9 months ago

@duglin I couldn't see anywhere where it mandated to stop after the first true, personally I think mandating it would make more sense especially given the behaviour I described above

duglin commented 9 months ago

On the 2/22 call we agreed with the general direction of changing the spec so that we only evaluate enough of the OR (left to right) to get our first true