From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org, Imeev Mergen <imeevma@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt
Date: Tue, 4 Jun 2019 21:34:45 +0200 [thread overview]
Message-ID: <0ac8fb6a-b06f-5c36-6bab-11dbcb128484@tarantool.org> (raw)
In-Reply-To: <2f731c49-b8c4-f25b-a0c8-b5dcf382b60c@tarantool.org>
On 03/06/2019 11:41, Imeev Mergen wrote:
> Hi! Thank you for the review! My answers below. Also, I think I'm
> going to drop this patch, since I plan to move the error code from
> p5 to p2 in the next patch.
>
> On 6/2/19 7:35 PM, Vladislav Shpilevoy wrote:
>>
>> On 28/05/2019 14:39, imeevma@tarantool.org wrote:
>>> Prior to this patch, the way to set Tarantool error in OP_Halt was
>>> too universal. It was possible to set a description of the error
>>> that does not match its errcode.
>> There was a concrete reason, why it was possible - because different
>> error codes have different arguments of various types, and the only way
>> to set an error at parsing stage is to allow to set arbitrary error
>> message to any error code. Without '...', va_arg etc. Besides, we could
>> use it to set correct line number and function name in future. Now you
>> use diag_set, which restricts us.
>>
>> So why do you need that patch? We will need to revert it when an error
>> appears requiring more than one argument, or an argument of not
>> const char * type. That will definitely happen.
> For some reason, manually creating an error message looks a bit
> wrong to me. How about creating a new macro to create such
> messages, for example, diag_prepare(), which takes the same
> arguments as diag_set() and returns the complete error message?
Perhaps. Sounds good to me. You can do it, if you want. But I
would suggest name "diag_prepare_msg", because it won't create a
struct error object. Only create a message.
next prev parent reply other threads:[~2019-06-04 19:34 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-28 11:39 [tarantool-patches] [PATCH v1 0/9] sql: set errors in VDBE using diag_set() imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 1/9] sql: remove mayAbort field from struct Parse imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 2/9] sql: remove error codes SQL_TARANTOOL_*_FAIL imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 3/9] sql: remove error ER_SQL imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 4/9] sql: rework diag_set() in OP_Halt imeevma
2019-06-02 16:35 ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-03 8:41 ` Imeev Mergen
2019-06-04 19:34 ` Vladislav Shpilevoy [this message]
2019-06-08 12:11 ` Mergen Imeev
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 5/9] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt imeevma
2019-06-02 16:35 ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-03 11:53 ` Mergen Imeev
2019-06-04 19:34 ` Vladislav Shpilevoy
2019-06-08 12:15 ` Mergen Imeev
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 6/9] sql: remove error SQL_INTERRUPT imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 7/9] sql: remove error SQL_MISMATCH imeevma
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 8/9] sql: use diag_set() to set an error in SQL functions imeevma
2019-06-02 16:35 ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-03 11:54 ` Mergen Imeev
2019-05-28 11:39 ` [tarantool-patches] [PATCH v1 9/9] sql: set errors in VDBE using diag_set() imeevma
2019-06-02 16:34 ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-03 12:10 ` Mergen Imeev
2019-06-03 12:20 ` Mergen Imeev
2019-06-09 17:14 ` [tarantool-patches] Re: [PATCH v1 0/9] " Vladislav Shpilevoy
2019-06-13 9:44 ` Kirill Yukhin
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=0ac8fb6a-b06f-5c36-6bab-11dbcb128484@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=imeevma@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt' \
/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