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(). In transformAlterTableStmt: we can remove transformCheckConstraints > entirely because calling transformCheckConstraints with skipValidation > = false does nothing and has no value. This way we could save a > function call. > > I prefer removing the skipValidation parameter from > transformCheckConstraints. Others might have different opinions. > I think this is intentional, to keep the code consistent with the CREATE TABLE path i.e. transformCreateStmt(). Here is what the comment atop transformCheckConstraints() reads: /* * transformCheckConstraints * handle CHECK constraints * * Right now, there's nothing to do here when called from ALTER TABLE, * but the other constraint-transformation functions are called in both * the CREATE TABLE and ALTER TABLE paths, so do the same here, and just * don't do anything if we're not authorized to skip validation. */ This was originally discussed in thread[1] and commit: f27a6b15e6566fba7748d0d9a3fc5bcfd52c4a1b [1] https://www.postgresql.org/message-id/flat/1238779931.11913728.1449143089410.JavaMail.yahoo%40mail.yahoo.com#f2d8318b6beef37dfff06baa9a1538b7 Regards, Jeevan Ladhe
