On Fri, Apr 16, 2021 at 6:26 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Thu, Apr 15, 2021 at 8:40 PM Jeevan Ladhe > <jeevan.la...@enterprisedb.com> wrote: > > IMHO, I think the idea here was to just get rid of an unnecessary variable > > rather than refactoring. > > > > On Thu, Apr 15, 2021 at 5:48 PM Bharath Rupireddy > > <bharath.rupireddyforpostg...@gmail.com> wrote: > >> > >> On Thu, Apr 15, 2021 at 5:04 PM Amul Sul <sula...@gmail.com> wrote: > >> > > >> > Hi, > >> > > >> > Attached patch removes "is_foreign_table" from transformCreateStmt() > >> > since it already has cxt.isforeign that can serve the same purpose. > >> > >> Yeah having that variable as "is_foreign_table" doesn't make sense > >> when we have the info in ctx. I'm wondering whether we can do the > >> following (like transformFKConstraints). It will be more readable and > >> we could also add more comments on why we don't skip validation for > >> check constraints i.e. constraint->skip_validation = false in case for > >> foreign tables. > > > > To address your concern here, I think it can be addressed by adding a > > comment > > just before we make a call to transformCheckConstraints(). > > +1.
Ok, added the comment in the attached version. Thanks Jeevan & Bharat for the review. Regards, Amul
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 9dd30370dae..17182500c61 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -176,7 +176,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) Oid namespaceid; Oid existing_relid; ParseCallbackState pcbstate; - bool is_foreign_table = IsA(stmt, CreateForeignTableStmt); /* * We must not scribble on the passed-in CreateStmt, so copy it. (This is @@ -327,14 +326,18 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) cxt.alist = list_concat(cxt.alist, cxt.likeclauses); /* - * Postprocess foreign-key constraints. + * Postprocess foreign-key and check constraints. */ transformFKConstraints(&cxt, true, false); /* * Postprocess check constraints. + * + * For a table, the constraint can be considered validated immediately, + * because the table must be empty. But for a foreign table this is not + * necessarily the case. */ - transformCheckConstraints(&cxt, !is_foreign_table ? true : false); + transformCheckConstraints(&cxt, !cxt.isforeign); /* * Postprocess extended statistics.
