Re: Document atthasmissing default optimization avoids verification table scan

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: James Coleman <jtc331(at)gmail(dot)com>
Cc: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Document atthasmissing default optimization avoids verification table scan
Date: 2022-03-25 20:40:37
Message-ID: CA+TgmoadgycbsP0fgm96GqtP5KeUrCWSs8+_TQLvQKnHvZdKfg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 25, 2022 at 8:49 AM James Coleman <jtc331(at)gmail(dot)com> wrote:
> Here's a version that looks like that. I'm not convinced it's an
> improvement over the previous version: again, I expect more advanced
> users to already understand this concept, and I think moving it to the
> ALTER TABLE page could very well have the effect of burying i(amidst
> the ton of detail on the ALTER TABLE page) concept that would be
> useful to learn early on in a tutorial like the DDL page. But if
> people really think this is an improvement, then I can acquiesce.

I vote for rejecting both of these patches.

0001 adds the following sentence to the documentation: "A <literal>NOT
NULL</literal> constraint may be added to the new column in the same
statement without requiring scanning the table to verify the
constraint." My first reaction when I read this sentence was that it
was warning the user about the absence of a hazard that no one would
expect in the first place. We could also document that adding a NOT
NULL constraint will not cause your gas tank to catch fire, but nobody
was worried about that until we brought it up. I also think that the
sentence makes the rest of the paragraph harder to understand, because
the rest of the paragraph is talking about adding a new column with a
default, and now suddenly we're talking about NOT NULL constraints.

0002 moves some advice about adding columns with defaults from one
part of the documentation to another. Maybe that's a good idea, and
maybe it isn't, but it also rewords the advice, and in my opinion, the
new wording is less clear and specific than the existing wording. It
also changes a sentence that mentions volatile defaults to give a
specific example of a volatile function -- clock_timestamp -- probably
because where the documentation was before that function was
mentioned. However, that sentence seems clear enough as it is and does
not really need an example.

I am not trying to pretend that all of our documentation in this area
is totally ideal or that nothing can be done to make it better.
However, I don't think that these particular patches actually make it
better. And I also think that there's only so much time that is worth
spending on a patch set like this. Not everything that is confusing
about the system is ever going to make its way into the documentation,
and that would remain true even if we massively expanded the level of
detail that we put in there. That doesn't mean that James or anyone
else shouldn't suggest things to add as they find things that they
think should be added, but it does mean that not every such suggestion
is going to get traction and that's OK too.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2022-03-25 20:55:47 Re: pgsql: Add 'basebackup_to_shell' contrib module.
Previous Message Andrew Dunstan 2022-03-25 20:30:05 Re: SQL/JSON: JSON_TABLE