Michael Paquier <michael.paqu...@gmail.com> writes:
> On Tue, Nov 1, 2016 at 4:17 AM, Jim Nasby <jim.na...@bluetreble.com> wrote:
>> While reviewing code coverage in pltcl, I uncovered a bug in trigger
>> function return handling. If you returned the munged name of a dropped
>> column, that would silently be ignored. It would be unusual to hit this,
>> since dropped columns end up with names like ".......pg.dropped.2.......",
>> but since that's still a legitimate name for a column silently ignoring it
>> seems rather bogus.
> It seems to me that this patch breaks $TG_relatts and what existing
> applications would except from it:
I'm not sure how it would do that. What's being changed here is the
code for processing the returned tuple; it doesn't change what is put
into $TG_relatts. Also, per the manual,
> (Empty list
> elements also appear in the positions of columns that have been
> dropped, so that the attribute numbering is correct for columns
> to their right.)
If the trigger function were blindly using that information without any
exception for dropped columns, then what it would provide here as the
column name is an empty string, not ".......pg.dropped.2.......", so that
it would get a failure anyway. This change wouldn't affect that.
So I'm inclined to agree that we should change this, but I think Jim's
patch doesn't go far enough: what we really ought to do is change
SPI_fnumber itself to reject matches to dropped columns. I just did
a survey of callers, and only a small minority of them are explicitly
testing for a match to a dropped column, but it doesn't look to me
like any of the rest are really prepared to do something reasonable
with one. I find it impossible to think of a situation where a caller
would want to treat a match to a dropped column as valid.
My proposal therefore is for SPI_fnumber to ignore (not match to)
dropped columns, and to remove any caller-side attisdropped tests that
thereby become redundant.
It's possible that it'd make sense for pltcl_trigger_handler to ignore
empty-string column names in the returned list, so that the behavior
with stupid trigger functions would be a bit more forgiving; but that
is more or less independent of this patch.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
