Re: clean up docs for v12 - Mailing list pgsql-hackers

From Tom Lane
Subject Re: clean up docs for v12
Date
Msg-id 16490.1555950804@sss.pgh.pa.us
Whole thread Raw
In response to Re: clean up docs for v12  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: clean up docs for v12
List pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Apr-22, Andres Freund wrote:
>> On 2019-04-22 14:48:26 +0900, Michael Paquier wrote:
>>> /*
>>> -     * 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.
>>> +     * Check if all the desired attributes are available in the tuple.  If so,
>>> +     * we can start deforming.  If not, we need to make sure to fetch the
>>> +     * missing columns.
>>> */

>> That's imo not an improvement. The guaranteed bit is actually
>> relevant. What this block is doing is eliding the check against the
>> tuple header for the number of attributes, if NOT NULL attributes for
>> later columns guarantee that the desired columns are present in the NULL
>> bitmap. But the rephrasing makes it sound like we're actually checking
>> against the tuple.
>>
>> I think it'd be better just to fix s/the all/that all/.

> (and s/if's/if it's/)

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.

But TBH, now that I look at the code, I think the entire optimization
is a bad idea and should be removed.  Am I right in thinking that the
presence of a wrong attnotnull marker could cause the generated code to
actually crash, thanks to not checking the tuple's natts field?  I don't
have enough faith in our enforcement of those constraints to want to see
JIT taking that risk to save a nanosecond or two.

(Possibly I'd not think this if I weren't fresh off a couple of days
with my nose in the ALTER TABLE SET NOT NULL code.  But right now,
I think that believing that that code does not and never will have
any bugs is just damfool.)

            regards, tom lane



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: clean up docs for v12
Next
From: Andres Freund
Date:
Subject: Re: Thoughts on nbtree with logical/varwidth table identifiers, v12on-disk representation