From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Does slot_deform_tuple need to care about dropped columns? |
Date: | 2018-11-07 17:44:03 |
Message-ID: | 20181107174403.zai7fedgcjoqx44p@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
Currently functions like slot_getattr() first check if the attribute is
already deformed:
Datum
slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull)
{
...
/*
* fast path if desired attribute already cached
*/
if (attnum <= slot->tts_nvalid)
{
*isnull = slot->tts_isnull[attnum - 1];
return slot->tts_values[attnum - 1];
}
...
but later, in the case the attribute isn't already deformed, the
following hunk exists:
/*
* If the attribute's column has been dropped, we force a NULL result.
* This case should not happen in normal use, but it could happen if we
* are executing a plan cached before the column was dropped.
*/
if (TupleDescAttr(tupleDesc, attnum - 1)->attisdropped)
{
*isnull = true;
return (Datum) 0;
}
Which strikes me as quite odd. If somebody previously accessed a *later*
column (be it via slot_getattr, or slot_getsomeattrs), the whole
attisdropped check is neutralized.
I think we either should remove that check as unnecessary, or move it to
slot_deform_tuple(), so it also protects other accesses (like the very
very common direct access to tts_values/isnull).
Tom, you added that code way back when, in a9b05bdc8330. And as far as I
can tell that issue existed back then too.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2018-11-07 17:58:16 | Re: Does slot_deform_tuple need to care about dropped columns? |
Previous Message | Peter Eisentraut | 2018-11-07 17:42:00 | Re: file cloning in pg_upgrade and CREATE DATABASE |