From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B54F32864A for ; Fri, 9 Aug 2019 15:23:14 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id S8TxYE8B1Bu9 for ; Fri, 9 Aug 2019 15:23:14 -0400 (EDT) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 03D6D2862C for ; Fri, 9 Aug 2019 15:23:13 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/3] box: rfc for stacked diagnostic area in Tarantool References: <101e2b28c29ebfc4cf58b45e534207fd4f6d9b3a.1564657285.git.kshcherbatov@tarantool.org> <20190807232755.s7gruhxeja75hob7@tkn_work_nb> <20190808232900.bf6wj2g6of4cf6ti@tkn_work_nb> From: Vladislav Shpilevoy Message-ID: <53b83ffb-138c-00c1-b701-e122a143c185@tarantool.org> Date: Fri, 9 Aug 2019 21:25:44 +0200 MIME-Version: 1.0 In-Reply-To: <20190808232900.bf6wj2g6of4cf6ti@tkn_work_nb> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Alexander Turenko Cc: tarantool-patches@freelists.org, Kirill Shcherbatov , kostja@tarantool.org >>> 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.