Hi Alvaro,
On Mon, Aug 5, 2019 at 11:39 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-Aug-05, yuzuko wrote:
>
> > So I proposed moving the if() block to the current place.
> > The latest patch can solve both queries but I found the latter
> > problem can be solved by setting constraint_exclusion = on.
>
> So we have three locations for that test; one is where it currently is,
> which handles a small subset of the cases. The other is where Amit
> first proposed putting it, which handles some additional cases; and the
> third one is where your latest patch puts it, which seems to handle all
> cases. Isn't that what Amit is saying? If that's correct (and that's
> what I want to imply with the comment changes I proposed), then we
> should just accept that version of the patch.
That's a correct summary, thanks.
> I don't think that we care about what happens with constraint_exclusion
> is on. That's not the recommended value for that setting anyway.
One issue I expressed with unconditionally applying constraint
exclusion in partprune.c the way the third patch proposes to do it is
that it means we end up performing the same processing twice for a
given relation in come cases. Specifically, when constraint_exclusion
is set to on, relation_excluded_by_constraints() that occurs when
set_rel_sizes() is applied to that relation would perform the same
proof. But maybe we shouldn't worry about the repetition too much,
because it's not likely to be very problematic in practice,
considering that setting constraint_exclusion to on is not
recommended.
Thanks,
Amit