Re: Verbosity of Function Return Type Checks

On Mon, 8 Sep 2008, Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> Modified as you suggested. BTW, will there be a similar i18n scenario
>> for "dropped column" you mentioned below?
>
> Yes, you need _() around those too.

For this purpose, I introduced a dropped_column_type variable in
validate_tupdesc_compat() function:

const char dropped_column_type[] = _("n/a (dropped column)");

And used this in below fashion:

OidIsValid(returned->attrs[i]->atttypid)
? format_type_be(returned->attrs[i]->atttypid)
: dropped_column_type

>> Done with format_type_be() usage.
>
> BTW you forgot to remove the quotes around the type names (I know I told
> you to add them but Tom gave the reasons why it's not needed) :-)

Done.

> Those are minor problems that are easily fixed. However there's a
> larger issue that Tom mentioned earlier and I concur, which is that the
> caller is forming the primary error message and passing it down. It
> gets a bit silly if you consider the ways the messages end up worded:
>
> errmsg("returned record type does not match expected record type"));
> errdetail("Returned type \"%s\" does not match expected type \"%s\" in column \"%s\".",
> ---> this is the case where it's OK
>
> errmsg("wrong record type supplied in RETURN NEXT"));
> errdetail("Returned type \"%s\" does not match expected type \"%s\" in column \"%s\".",
> --> this is strange
>
> errmsg("returned tuple structure does not match table of trigger event"));
> errdetail("Returned type \"%s\" does not match expected type \"%s\" in column \"%s\".",
> --> this is not OK

What we're trying to do is to avoid code duplication while checking the
returned tuple types against expected tuple types. For this purpose we
implemented a generic function (validate_tupdesc_compat) to handle all
possible cases. But, IMHO, it's important to remember that there is no
perfect generic function to satisfy all possible cases at best.

> I've been thinking how to pass down the context information without
> feeding the complete string, but I don't find a way without doing
> message construction. Construction is to be avoided because it's a
> pain for translators.
>
> Maybe we should just use something generic like errmsg("mismatching
> record type") and have the caller pass two strings specifying what's
> the "returned" tuple and what's the "expected", but I don't see how
> ... (BTW this is worth fixing, because every case seems to have
> appeared independently without much thought as to other callsites with
> the same pattern.)

I considered the subject with identical constraints but couldn't come up
with a more rational solution. Nevertheless, I'm open to any suggestion.

Regards.