On Thu, Feb 27, 2025 at 3:25 PM Rushabh Lathia <rushabh.lat...@gmail.com> wrote:
> Thank Alvaro for the fixup patch. > > > > > On Fri, Feb 21, 2025 at 11:43 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> > wrote: > >> Hello, >> >> Thanks! >> >> I noticed a typo 'constrint' in several places; fixed in the attached. >> >> I discovered that this sequence, taken from added regression tests, >> >> CREATE TABLE notnull_tbl1 (a int); >> ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn_parent not null a not valid; >> CREATE TABLE notnull_chld (a int); >> ALTER TABLE notnull_chld ADD CONSTRAINT nn_child not null a not valid; >> ALTER TABLE notnull_chld INHERIT notnull_tbl1; >> ALTER TABLE notnull_tbl1 VALIDATE CONSTRAINT nn_parent; >> >> does mark the constraint as validated in the child, but only in >> pg_constraint -- pg_attribute continues to be marked as 'i', so if you >> try to use it for a PK, it fails: >> >> alter table notnull_chld add constraint foo primary key (a); >> ERROR: column "a" of table "notnull_chld" is marked as NOT VALID NOT >> NULL constrint >> >> I thought that was going to be a quick fix, so I tried to do so; since >> we already have a function 'set_attnotnull', I thought it was the >> perfect tool to changing attnotnull. However, it's not great, because >> since that part of the code is already doing the validation, I don't >> want it to queue the validation again, so the API needs a tweak; I >> changed it to receiving separately which new value to update attnotnull >> to, and whether to queue validation. With that change it works >> correctly, but it is a bit ugly at the callers' side. Maybe it works to >> pass two booleans instead? Please have a look at whether that can be >> improved. >> > > I haven't given much thought to improving the API, but I'll look into it > now. Also, > I'm considering renaming AdjustNotNullInheritance() since it now also > checks for invalid NOT NULL constraints. What do you think? > I adjusted the set_attnotnull() API and removed the added queue_validation parameter. Rather, the function start using wqueue input parameter as a check. If wqueue is NULL, skip the queue_validation. Attaching patch here, but not sure how clear it is, in term of understanding the API. Your thoughts/inputs? Looking further for AdjustNotNullInheritance() I don't see need of rename this API as it's just adding new error check now for an existing NOT NULL constraint. JFYI, I can reproduce the failure Ashutosh Bapat reported, while running the pg_upgrade test through meson commands. I am investigating that further. Thanks, Rushabh Lathia
0004-Adjust-set_attnotnull-API-input-parameters.patch
Description: Binary data
