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
v1-0001-Prevent-spurious-indexes-on-virtual-generated-col.patch
Description: Binary data
v1-0003-Add-a-test-for-creating-an-index-on-a-whole-row-e.patch
Description: Binary data
v1-0002-Bounds-check-access-to-TupleDescAttr-with-an-Asse.patch
Description: Binary data
