On Thu, Jan 20, 2022 at 3:31 PM Andrew Dunstan <and...@dunslane.net> wrote:
>
>
> On 1/20/22 12:25, Bossart, Nathan wrote:
> > On 1/19/22, 5:15 PM, "James Coleman" <jtc...@gmail.com> wrote:
> >> I'm open to the idea of wordsmithing here, of course, but I strongly
> >> disagree that this is irrelevant data. There are plenty of
> >> optimizations Postgres could theoretically implement but doesn't, so
> >> measuring what should happen by what you think is obvious ("told it to
> >> populate with default values - why do you need to check") is clearly
> >> not valid.
> >>
> >> This patch actually came out of our specifically needing to verify
> >> that this is true before an op precisely because docs aren't actually
> >> clear and because we can't risk a large table scan under an exclusive
> >> lock. We're clearly not the only ones with that question; it came up
> >> in a comment on this blog post announcing the newly committed feature
> >> [1].
> > My initial reaction was similar to David's. It seems silly to
> > document that we don't do something that seems obviously unnecessary.
> > However, I think you make a convincing argument for including it. I
> > agree with David's feedback on where this information should go.
> >
>
> I still don't understand the confusion. When you add a new column with a
> non-null non-volatile default, none of the existing rows has any storage
> for the new column, so there is nothing to scan and nothing to verify on
> such rows. Only the catalog is changed. This is true whether or not the
> new column is constrained by NOT NULL. I don't understand what people
> think might have had to be verified by scanning the table.
>
> If what's happening is not clear from the docs then by all means let's
> make it clear. But in general I don't think we should talk about what we
> used to do.
This path isn't about talking about what we used to do (though that's
already in the docs); it is about trying to make it clear.
But actually "When you add a new column with a non-null non-volatile
default...there is nothing to scan" doesn't always hold as I showed
with the check constraint above. Other than that I think that phrasing
is actually almost close to the kind of clarity I'd like to see in the
docs.
As noted earlier I expect to be posting an updated patch soon.
Thanks,
James Coleman
