On Wed, Aug 7, 2019 at 7:47 AM Alvaro Herrera <alvhe...@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