From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
        Michael Paquier <michael@paquier.xyz>,
        Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Subject: Re: clean up docs for v12
In-reply-to: <20190422175547.cpjwgt2ghwyqy2cc@alap3.anarazel.de>
References: <20190422161955.GA17411@alvherre.pgsql>
 <16490.1555950804@sss.pgh.pa.us>
 <20190422164356.7d63s4bui3735r3q@alap3.anarazel.de>
 <20190422165324.iyduznrdspcia7ja@alap3.anarazel.de>
 <18604.1555954037@sss.pgh.pa.us>
 <20190422175547.cpjwgt2ghwyqy2cc@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Mon, 22 Apr 2019 10:55:47 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20636.1555957068.1@sss.pgh.pa.us>
Date: Mon, 22 Apr 2019 14:17:48 -0400
Message-ID: <20637.1555957068@sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> On 2019-04-22 13:27:17 -0400, Tom Lane wrote:
>> I wonder
>> also if it wouldn't be smart to explicitly check that the "guaranteeing"
>> column is not attisdropped.

> Yea, that probably would be smart.  I don't think there's an active
> problem, because we remove NOT NULL when deleting an attribute, but it
> seems good to be doubly sure / explain why that's safe:
> 		/* Remove any NOT NULL constraint the column may have */
> 		attStruct->attnotnull = false;
> I'm a bit unsure whether to make it an assert, elog(ERROR) or just not
> assume column presence?

I'd just make the code look like

        /*
         * If it's NOT NULL then it must be present in every tuple,
         * unless there's a "missing" entry that could provide a non-null
         * value for it.  Out of paranoia, also check !attisdropped.
         */
        if (att->attnotnull &&
            !att->atthasmissing &&
            !att->attisdropped)
            guaranteed_column_number = attnum;

I don't think the extra check is so expensive as to be worth obsessing
over.

			regards, tom lane