[tarantool-patches] Re: [PATCH v1 1/3] box: rfc for stacked diagnostic area in Tarantool

[Vladislav Shpilevoy]

>>> I like Java terminology here: cause is exactly what we want to save for
>>> an error. Wrap / unwrap (Go terminology) look too abstract and maybe
>>> even unclear for me. Maybe this terms appears from merry / xerror
>>> packages, which were inspired to wrap built-in errors with additional
>>> information (say, stack traces). It is up to maintainers, but my vote is
>>> for 'cause' field, 'get_cause()' and 'set_cause()' functions (maybe with
>>> some prefix, because it is C).
>>
>> Strangely, but I understood wrap/unwrap easily. So I vote to keep them.
>> But if there are more people who doesn't understand, then here is my
>> humble proposal:
>>
>>     C API:
>>     box_error_reason(const struct error *) // To get a reason error.
>>     box_error_set_reason(struct error *, struct error *) // To set a reason.
>>
>>     Lua API:
>>     error:reason() -- Get a reason error.
>>     error:set_reason(reason) -- Set a reason error.
> 
> Just to compare, now we have:
> 
>  | C API:
>  | struct error *
>  | box_error_unwrap(box_error_t *error); // Get a reason and detach from a
>  |                                       // passed error.
>  | struct error *
>  | box_error_wrap(box_error_t *error, box_error_t *reason); // Set a reason.
> 
>  | Lua API:
>  | error:unwrap() -> reason    -- Get a reason and detach it from a passed
>  |                             -- error.
>  | error:wrap(reason) -> error -- Set a reason.
> 
> I doubt whether a function to get a reason should modify the original
> error. The terms need to be discussed with others.

Agree, an attempt to get reason should not modify the original error. And
should not touch the global diagnostics area. Currently error:unwrap()
changes box.error.last() too.

>>> Maybe we should use 'warnings' term here if this feature is intended to
>>> be used for SQL warnings. If we want to use the proposed mechanics for
>>> warnings, then my proposal re using 'cause' term looks doubtful. Don't
>>> sure whether we should introduce some kind of warnings list for the
>>> diagnostic area or reuse 'cause' / 'parent' / ... field of struct error.
> 
> This is most confusing part for me. Say, we want to set a warning re
> precision loss during execution a SQL query. The response will be
> successful. There will not be an error to wrap this warning. How to
> store the warning (it looks as a query-local object) and how to show it
> in the response (in the binary protocol)?
> 
> Another case: we emit a warning re precission loss and an error occurs
> afterwards during the query execution (say, a constraint violation). The
> warning is not a reason / cause / parent for the error and it is not
> logical to using our current terms for this case.
> 
> We want to support SQL stacked diagnostics and it seems that the current
> proposal does not move us forward to them. I had read mysql docs on
> that, but I hope the standard described quite same thing:
> https://dev.mysql.com/doc/refman/5.6/en/diagnostics-area.html
> 
> I think we need at least keep SQL stacked diagnostics in a mind and
> explicitly decide whether we'll going (a bit?) forward to support them
> within this issue / proposal / discussion.
I didn't think it was related to warnings. I didn't even think, that we
are going to support warnings. After all, if something is not an error,
people will just ignore it. I bet, one of the first SQL related issues
after we release the warnings would be 'How to turn warnings off?'.
IMO, we should just treat everything as errors.

If warnings are already discussed and planned, then unfortunately I don't
have a strong opinion on the subject. Nonetheless, below are my first
thoughts about the warnings.

I think, that warnings should be stored in the same stack as errors,
and we need a flag saying if the stack contains at least one error. In
that case the whole stack is treated as an error.

Why in the same stack? Because it will look like a request execution
history. All errors and warnings would be sorted by occurrence time,
and it will be easy to locate in scope of what an error or a warning
has happened.

Talking of how would it look in code - perhaps we will add something
like 'struct warning', and both 'struct error' and 'struct warning'
would inherit from a base structure. Fiber would contain list of that
structures. This implementation lays good on the current version of
stacked diagnostics. Just raw thoughts.