On Wed, 6 Mar 2019 at 08:41, Sergei Kornilov <sk@zsrv.org> wrote:
> In this case we need extra argument for ConstraintImpliedByRelConstraint for some caller-provided existConstraint,
right?Along with Relation itself? Then I need make copy of existConstraint, append relation constraints and call
predicate_implied_by.If I understood correctly pseudocode would be:
>
> PartConstraintImpliedByRelConstraint(rel, testConstraint)
> generate notNullConstraint from NOT NULL table attributes
> call ConstraintImpliedByRelConstraint(rel, testConstraint, notNullConstraint)
>
> ConstraintImpliedByRelConstraint(rel, testConstraint, existsConstraint)
> copy existsConstraint to localExistsConstraint variable
> append relation constraints to localExistsConstraint list
> call predicate_implied_by
>
> I thing redundant IS NOT NULL check is not issue here and we not need extra arguments for
ConstraintImpliedByRelConstraint.Was not changed in attached version, but I will change if you think this would be
betterdesign.
I was really just throwing the idea out there for review. I don't
think that I'm insisting on it.
FWIW I think you could get away without the copy of the constraint
providing it was documented that the list is modified during the
ConstraintImpliedByRelConstraint call and callers must make a copy
themselves if they need an unmodified version. Certainly,
PartConstraintImpliedByRelConstraint won't need to make a copy, so
there's probably not much reason to assume that possible future
callers will.
Providing I'm imagining it correctly, I do think the patch is slightly
cleaner with that change.
It's:
a) slightly more efficient due to not needlessly checking a bunch of
IS NOT NULLs (imagine a 1000 column table with almost all NOT NULL and
a single CHECK constraint); and
b) patch has a smaller footprint (does not modify existing callers of
PartConstraintImpliedByRelConstraint()); and
c) does not modify an existing API function.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
