Hi,
On 2019-04-22 09:43:56 -0700, Andres Freund wrote:
> On 2019-04-22 12:33:24 -0400, Tom Lane wrote:
> > ISTM that Michael's proposed wording change shows that the existing
> > comment is easily misinterpreted. I don't think these minor grammatical
> > fixes will avoid the misinterpretation problem, and so some more-extensive
> > rewording is called for.
>
> Fair enough.
The computation of that variable above has:
* If the column is possibly missing, we can't rely on its (or
* subsequent) NOT NULL constraints to indicate minimum attributes in
* the tuple, so stop here.
*/
if (att->atthasmissing)
break;
/*
* Column is NOT NULL and there've been no preceding missing columns,
* it's guaranteed that all columns up to here exist at least in the
* NULL bitmap.
*/
if (att->attnotnull)
guaranteed_column_number = attnum;
and only then the comment referenced in the discussion here follows:
/*
* Check if's guaranteed the all the desired attributes are available in
* tuple. If so, we can start deforming. If not, need to make sure to
* fetch the missing columns.
*/
I think just reformulating that to something like
/*
* Check if it's guaranteed that all the desired attributes are available
* in the tuple (but still possibly NULL), by dint of either the last
* to-be-deformed column being NOT NULL, or subsequent ones not accessed
* here being NOT NULL. If that's not guaranteed the tuple headers natt's
* has to be checked, and missing attributes potentially have to be
* fetched (using slot_getmissingattrs().
*/
should make that clearer?
Greetings,
Andres Freund
