On Wed, Aug 7, 2019 at 7:47 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> Hello, here's a pretty trivial cleanup.
>
> Currently, you have to pass the errmsg text to convert_tuples_by_name
> and convert_tuples_by_position that's going to be raised if the tuple
> descriptors don't match. In the latter's case that makes sense, as each
> case is pretty specific and tailored messages can be offered, so this is
> useful.
>
> However, in the case of convert_tuples_by_name, it seems we don't have
> enough control over what is being called, so there's no way to
> produce tailored messages -- all the callers are using the same generic
> wording: "could not convert row type".
>
> This code was introduced by dcb2bda9b704; I think back then we were
> thinking that it would be possible to give different error messages for
> different cases (as convert_tuples_by_position was already doing then),
> however it seems clear now that that'll never happen.
>
> I propose we get rid of it by having convert_tuples_by_name supply the
> error message by itself, as in the attached patch.
+1. I always wondered when writing partitioning patches why I have to
pass the same string.
If we're reducing the message string to occur only once in the source
code, can we maybe write it to be more informative? I wonder if users
aren't normally supposed to see this message?
Thanks,
Amit