On Sun, Mar 27, 2022 at 1:46 PM David G. Johnston <david.g.johns...@gmail.com> wrote: > > On Sun, Mar 27, 2022 at 10:00 AM James Coleman <jtc...@gmail.com> wrote: >> >> As shown above, table scans (and specifically table scans used to >> validate constraints, which is what this patch is about) are clearly >> documented (more than once!) in the ALTER TABLE documentation. In fact >> it's documented specifically in reference to SET NOT NULL, which is >> even more specifically the type of constraint this patch is about. >> >> So "undocumented concept" is just not accurate, and so I don't see it >> as a valid reason to reject the patch. >> > > As you point out, where these scans are performed is documented. Your > request, though, is to document a location where they are not performed > instead of trusting in the absence of a statement meaning that no such scan > happens. In this case no such scan of the table is ever needed when adding a > column and so ADD COLUMN doesn't mention table scanning. We almost always > choose not to document those things which do not happen. I don't always > agree with this position but it is valid and largely adhered to. On that > documentation theory/policy basis alone this patch can be rejected. 0001 as > proposed is especially strong in violating this principle.
Hmm, I didn't realize that was project policy, but I'm a bit surprised given that the sentence which 0001 replaces seems like a direct violation of that also: "In neither case is a rewrite of the table required." > My two thoughts from yesterday take slightly different approaches to try and > mitigate the same misunderstanding while also providing useful guidance to > the reader to make sure the hazard of ALTER COLUMN SET NOT NULL is something > they are thinking about even when adding a new column since forgetting to > incorporate the NOT NULL during the add can be a costly mistake. The > tweaking the notes section seems to be the more productive of the two > approaches. Yes, I like those suggestions. I've attached an updated patch that I think fits a good bit more naturally into the Notes section specifically addressing scans and rewrites on NOT NULL constraints. Thanks for the feedback, James Coleman
v4-0001-Document-atthasmissing-default-avoids-verificatio.patch
Description: Binary data
