On Mon, Aug 15, 2016 at 7:21 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: >> + if (partexprs) >> + recordDependencyOnSingleRelExpr(&myself, >> + (Node *) partexprs, >> + RelationGetRelid(rel), >> + DEPENDENCY_NORMAL, >> + DEPENDENCY_IGNORE); >> >> I don't think introducing a new DEPENDENCY_IGNORE type is a good idea >> here. Instead, you could just add an additional Boolean argument to >> recordDependencyOnSingleRelExpr. That seems less likely to create >> bugs in unrelated portions of the code. > > I did consider a Boolean argument instead of a new DependencyType value, > however it felt a bit strange to pass a valid value for the fifth argument > (self_behavior) and then ask using a separate parameter that it (a > self-dependency) is to be ignored. By the way, no pg_depend entry is > created on such a call, so the effect of the new type's usage seems > localized to me. Thoughts?
I think that's not a very plausible argument. If you add a fifth argument to that function, then only that function needs to know about the possibility of ignoring self-dependencies. If you add a new dependency type, then everything that knows about DependencyType needs to know about them. That's got to be a much larger surface area for bugs. Also, if you look around a bit, I believe you will find other examples of cases where one argument is used only for certain values of some other argument. That's not a novel design pattern. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
