On Sun, Mar 27, 2022 at 1:46 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> 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
wherethey are not performed instead of trusting in the absence of a statement meaning that no such scan happens. In
thiscase 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
positionbut 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
alsoproviding useful guidance to the reader to make sure the hazard of ALTER COLUMN SET NOT NULL is something they are
thinkingabout 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
