[tarantool-patches] Re: [PATCH v1 4/9] sql: rework diag_set() in OP_Halt

[Mergen Imeev <imeevma@tarantool.org>]

Thank you for review! My answer below.

On Tue, Jun 04, 2019 at 09:34:45PM +0200, Vladislav Shpilevoy wrote:
> 
> 
> 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.
>
Thanks. I'm going to do this as an follow-up patch to the
current patch-set.