TupleDescAttr bounds checks

Hi,

Scrutiny of a recent test_plan_advice failure in the buildfarm
revealed a bug that had nothing to do with test_plan_advice or
pg_plan_advice; rather, it was a bug introduced by the virtual
generated columns feature, and specifically of that feature indexing
off of the beginning of a TupleDesc when whole-row attributes are
present. The first patch attached to this email fixes this issue, and
should be committed and back-patched to v18. I plan to do that soon
unless there are objections.

But that got me wondering why we don't have an assertion in
TupleDescAttr to catch this sort of thing, and it seems like that is
indeed something we can do, so patch #2 adds that and then cleans up
the resulting damage. By "damage" I mean correcting places where the
new Assert() either actually fails or could theoretically fail,
because we use TupleDescAttr() on a value that we don't know to be
within range. None of these seem to be actual bugs, because as the
commit message says, all TupleDescAttr() does is compute a pointer,
and we don't actually dereference that pointer in any of these code
paths until after we know that it's OK to do so. Nonetheless, these
all seem like good cleanups, so I do not see any of these changes as
arguments against adding the assertion. I propose to put this in
master.

Patch #3 adds a test case that would have caught the bug fixed by
patch #1 if we had already had the asserts added by patch #2. To my
surprise, we seem to have zero existing test coverage of creating an
index on a whole-row expression, so I think this is worth adding
mostly for that reason. One could also argue that it's worth adding as
a follow-up to #1 and #2, but we're unlikely to reintroduce that
specific bug. We might, however, add other bugs that this would also
catch.

Comments?

--
Robert Haas
EDB: http://www.enterprisedb.com