Re: pgsql: Remove ineffective check against dropped columns from slot_getat

Hi,

On 2018-11-11 01:11:34 +1300, David Rowley wrote:
> On 10 November 2018 at 14:41, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > These days locking and plan invalidation ought to ensure that dropped
> > columns are not accessed in query plans. Therefore this commit just
> > drops the insufficient check in slot_getattr(). It's possible that
> > we'll find some holes againt use of dropped columns, but if so, those
> > need to be addressed independent of slot_getattr(), as most accesses
> > don't go through that function anyway.
>
> Would it not be worth an Assert(!TupleDescAttr(tupleDesc, attnum -
> 1)->attisdropped); so that we're more likely to discover any issues
> where cached plans are not invalidated correctly?

I don't think it'd really do much. Only very few reads of tuples though
through slot_getattr(). The expression eval machinery - which will be
used for those cases - does verify this on the first execution of an
expression. C.f. CheckVarSlotCompatibility().

If you, or somebody else, feels strongly, we can add one, but I don't
really see the point here.

Greetings,

Andres Freund