From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches@freelists.org, Kirill Shcherbatov <kshcherbatov@tarantool.org>, kostja@tarantool.org Subject: [tarantool-patches] Re: [PATCH v1 1/3] box: rfc for stacked diagnostic area in Tarantool Date: Fri, 9 Aug 2019 21:25:44 +0200 [thread overview] Message-ID: <53b83ffb-138c-00c1-b701-e122a143c185@tarantool.org> (raw) In-Reply-To: <20190808232900.bf6wj2g6of4cf6ti@tkn_work_nb> >>> 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.
next prev parent reply other threads:[~2019-08-09 19:23 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-01 11:13 [tarantool-patches] [PATCH v1 0/3] box: stacked diagnostics area in fiber Kirill Shcherbatov 2019-08-01 11:13 ` [tarantool-patches] [PATCH v1 1/3] box: rfc for stacked diagnostic area in Tarantool Kirill Shcherbatov 2019-08-05 21:16 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <06bd2140-3d2b-4bc3-7bc4-5f3d293bf891@tarantool.org> 2019-08-06 20:50 ` Vladislav Shpilevoy 2019-08-07 23:27 ` Alexander Turenko 2019-08-08 20:46 ` Vladislav Shpilevoy 2019-08-08 23:29 ` Alexander Turenko 2019-08-09 19:25 ` Vladislav Shpilevoy [this message] 2019-08-12 20:35 ` Konstantin Osipov 2019-08-01 11:13 ` [tarantool-patches] [PATCH v1 2/3] box: stacked diagnostics area in fiber Kirill Shcherbatov 2019-08-05 21:16 ` [tarantool-patches] " Vladislav Shpilevoy 2019-08-01 11:13 ` [tarantool-patches] [PATCH v1 3/3] box: extend ffi error object API Kirill Shcherbatov 2019-08-05 21:18 ` [tarantool-patches] " Vladislav Shpilevoy 2019-08-06 7:56 ` Kirill Shcherbatov 2019-08-06 20:50 ` Vladislav Shpilevoy 2019-08-08 23:33 ` Alexander Turenko 2019-08-09 19:27 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=53b83ffb-138c-00c1-b701-e122a143c185@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=kostja@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 1/3] box: rfc for stacked diagnostic area in Tarantool' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox