Hi, On 2018-11-11 01:11:34 +1300, David Rowley wrote: > On 10 November 2018 at 14:41, Andres Freund <[email protected]> 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
