Thread: About get_relation_constraints and include_notnull

Why does the argument include_notnull argument exist if
get_relation_constraints() is being called from only one place? Perhaps we
could remove it and add the IS NOT NULL test expression unconditionally if
there are any NOT NULL columns.

Thanks,
Amit

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> Why does the argument include_notnull argument exist if
> get_relation_constraints() is being called from only one place? Perhaps we
> could remove it and add the IS NOT NULL test expression unconditionally if
> there are any NOT NULL columns.

Well, you could argue why have a routine at all instead of inlining it
into the one caller.  IIRC the thought was that other likely uses of
constraint-fetching might want to see only the actual check constraints.
Is there some positive benefit from removing the argument?
        regards, tom lane

On 2016/01/14 23:36, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> Why does the argument include_notnull argument exist if
>> get_relation_constraints() is being called from only one place? Perhaps we
>> could remove it and add the IS NOT NULL test expression unconditionally if
>> there are any NOT NULL columns.
> 
> Well, you could argue why have a routine at all instead of inlining it
> into the one caller.  IIRC the thought was that other likely uses of
> constraint-fetching might want to see only the actual check constraints.
> Is there some positive benefit from removing the argument?

The function is local to plancat.c and hasn't seen any callers beside
relation_excluded_by_constraints() for a long time (as can be told from
the git history). It's just that initially I thought it's being used
elsewhere which upon inspecting I didn't find any. Not really sure I can
make the analogy but ExecConstraints() performs both NOT NULL and CHECK
constraint checks without any argument.

Though, we could just leave it alone. Sorry for the noise.

Thanks,
Amit