Hi jian,
> This seems unnecessary.
> In EEOP_JSONEXPR_PATH, we can
> if document or jsonpath is NULL, we can just go to jump_end (return
> NULL) or jump_eval_coercion (NULL need coerce to constrainted domain),
> no need to worry about ON ERROR, ON EMPTY.
That also takes care of the back-branch concern I raised on v2: with
no new opcode, v3 does not change any header, so the ExprEvalOp values
stay as they are.
v3 applies cleanly to master, REL_18_STABLE and REL_17_STABLE, builds
without warnings, and the main regression suite passes on all three.
Since the bug is state leaking from one row to the next, I also
checked it with a differential test (attached): every row of a
two-row query must give the same result as that row evaluated alone.
It covers json_value (RETURNING int, a NOT NULL domain and a CHECK
domain), json_query and json_exists, with every ON EMPTY / ON ERROR
combination, over every ordered pair of rows built from five
documents ('{}', '{"a":1}', '{"a":"x"}', '{"a":[1,2]}', NULL) and
three paths ('$.a', 'strict $.a', NULL). That is 9675 cases:
- unpatched master (09a579abaca): 567 mismatches (497 json_value,
56 json_query, 14 json_exists)
- v3 on master, REL_18_STABLE and REL_17_STABLE: 0
The single-row answers do not change: all 645 of them are the same
with and without v3, including NULL input into the NOT NULL domain.
With Srinath's fix for #19695 on top, its case is right too
(1, NULL, 2). As before, I could not exercise JIT here.
Regards,
Manu
