Re: Declarative partitioning - another take

On Mon, Aug 15, 2016 at 7:21 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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