Re: Verbosity of Function Return Type Checks

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Volkan YAZICI <yazicivo(at)ttmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Verbosity of Function Return Type Checks
Date: 2008-09-05 16:15:08
Message-ID: 25823.1220631308@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Volkan YAZICI <yazicivo(at)ttmail(dot)com> writes:
> On Thu, 04 Sep 2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> This is not ready to go: you've lost the ability to localize most of
>> the error message strings.

> How can I make this available? What's your suggestion?

I think the best way is to use

subroutine(..., gettext_noop("special error message here"))

at the call sites, and then

errmsg("%s", _(msg))

when throwing the error. gettext_noop() is needed to have the string
be put into the message catalog, but it doesn't represent any run-time
effort. The _() macro is what actually makes the translation lookup
happen. We don't want to incur that cost in the normal no-error case,
which is why you shouldn't just do _() at the call site and pass an
already-translated string to the subroutine.

>> Another problem with it is it's likely going to fail completely on
>> dropped columns (which will have atttypid = 0).

> Is it ok if I'd replace

> if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

> line in validate_tupdesc_compat with

> if (td1->attrs[i]->atttypid &&
> td2->attrs[i]->atttypid &&
> td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)

> expression to fix this?

No, that's weakening the compatibility check. (There's a separate issue
here of teaching plpgsql to actually cope nicely with rowtypes
containing dropped columns, but that's well beyond the scope of this
patch.)

What I'm on about is protecting format_type_be() from being passed
a zero and then failing. Perhaps it would be good enough to do
something like

OidIsValid(td1->attrs[i]->atttypid) ?
format_type_with_typemod(td1->attrs[i]->atttypid,
td1->attrs[i]->atttypmod) :
"dropped column"

while throwing the error.

BTW, since what's actually being looked at is just the type OID and not
the typmod, it seems inappropriate to me to show the typmod in the
error. I'd go with just format_type_be(td1->attrs[i]->atttypid) here
I think.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Kreen 2008-09-05 16:22:19 Re: Need more reviewers!
Previous Message Alex Hunsaker 2008-09-05 16:12:01 Re: hash index improving v3