Make default-nulls take precedence over exceptions

[kagamiori]

The problem
The evaluation result of an expression may be affected by the order we evaluate arguments if the function has default NULL behavior and there are exceptions from one argument and NULLs from another arguments at the same rows.

For example, imagine we evaluate two expressions add(g(c0), h(c1)) and add(h(c1), g(c0)), and assume the evaluation of g(c0) throws at row 0, and h(c1) returns NULL at row 0. add(g(c0), h(c1)) would throw at row 0 immediately when it evaluates the subexpression g(c0). On the other hand, add(h(c1), g(c0)) returns NULL at row 0 because it evaluates h(c1) first, gets a NULL at row 0, and remove row 0 from the selected rows before evaluating g(c0) (such that g(c0) would not be evaluated on row 0). A real expression to reproduce this discrepancy is the following:

a: [1, 2]
b: [3, 4]
expression 1: a / (a - 2) + if (b = 1, b, NULL)
expression 2: if (b = 1, b, NULL) + a / (a - 2)

Context

• This discrepancy fails our expression fuzzer test that blocks the PR Constant-fold expressions that are default nulls #2007. The failed test evaluates an expression like default_null_function(c0, throw_error(c1), NULL). The common evaluation path constant-folds the expression by the default NULL, while the simplified evaluation path evaluates the expression and throws at the second subexpression.
• AND and OR expressions are already implemented in a way that is agnostic of the argument evaluation order.

Proposed solution
One solution we're proposing is to hold the exceptions in the error vector in context (i.e., EvalCtx::errors_) when evaluating each argument of an expression. After evaluating all arguments, if the function has default NULL behavior, we check whether there are NULLs from one argument at the same rows as exceptions from another argument. In such as case, we remove the exceptions at those rows. This will make add(g(c0), h(c1)) and add(h(c1), g(c0)) in the example above both return NULL at row 0.

Pros

• Expression result becomes agnostic to argument order, which brings less confusion to users.

Cons

• There is runtime cost of holding and processing exceptions when errors exist;
• Other expression evaluators we'd like to build in the future have to do the same.

Discussion
I'm wondering whether there are concerns about the proposed solution and what other options we may have.

[kagamiori]

CC: @mbasmanova @laithsakka @kevinwilfong @pedroerp

[Yuhta]

I think it's ok to return different error if the argument order is different. The bug is that even for same order, simplified eval returns different error from common eval.

[kagamiori]

Hi Jimmy, Thank you for the comment! Our thought was that if the evaluator hold the exceptions from arguments and remove exceptions at rows where NULLs exist, the simplified path would also return NULL instead of throw immediately, so that it behaves the same as the common path. So the proposed solution is one way to address the fuzzer test failures that is also nice to have because it reduces potential confusion. We're also open to other possible solutions to fix the fuzzer test failures.