jian he <jian.universal...@gmail.com> 于2024年9月9日周一 16:31写道：
> On Mon, Sep 2, 2024 at 6:33 PM Tender Wang <tndrw...@gmail.com> wrote: > > > > > > > > The attached patch adds List *nnconstraints, which store the not-null > definition, in struct CreateStmt. > > This makes me a little confused about List *constraints in struct > CreateStmt. Actually, the List constraints > > store ckeck constraint, and it will be better if the comments can > reflect that. Re-naming it to List *ckconstraints > > seems more reasonable. But a lot of codes that use stmt->constraints > will be changed. > > > hi. > seems you forgot to attach the patch? > I also noticed this minor issue. > I have no preference for Renaming it to List *ckconstraints. > +1 for better comments. maybe reword to > >>> > List *constraints; /* CHECK constraints (list of Constraint > nodes) */ > >>> > I just gave advice; whether it is accepted or not, it's up to Alvaro. If Alvaro agrees with the advice, he will patch a new one. We can continue to review the new patch. If Alvaro disagrees, he doesn't need to change the current patch. I think this way will be more straightforward for others who will review this feature. > > On Tue, Sep 3, 2024 at 3:17 PM Tender Wang <tndrw...@gmail.com> wrote: > > > > The test case in constraints.sql, as below: > > CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL NOT NULL); > > > ^^^^^^^^^^ > > There are two not-null definitions, and is the second one redundant? > > > > hi. > i think this is ok. please see > function transformColumnDefinition and variable saw_nullable. > Yeah, it is ok. > we need to make sure this: > CREATE TABLE notnull_tbl3 (a INTEGER NULL NOT NULL); > fails. > > > of course, it's also OK do this: > CREATE TABLE notnull_tbl3 (a INTEGER NULL NULL); > -- Thanks, Tender Wang
