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:

Reply via email to