On Sun, Mar 27, 2022 at 10:00 AM James Coleman <jtc331@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.
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.