Re: Patch to fix FK-related selectivity estimates with constants

On Tue, Oct 27, 2020 at 01:58:56PM -0400, Tom Lane wrote:
>I wrote:
>> Over in the thread at [1] it's discussed how our code for making
>> selectivity estimates using knowledge about FOREIGN KEY constraints
>> is busted in the face of EquivalenceClasses including constants.
>> ...
>> Attached is a patch series that attacks it that way.
>

The patch sems fine to me, thanks for investigating and fixing this.

Two minor comments:

I find it a bit strange that generate_base_implied_equalities_const adds
the rinfo to ec_derives, while generate_base_implied_equalities_no_const
does not. I understand it's correct as we don't lookup the non-const
clauses, and we want to keep the list as short as possible, but it seems
like a bit surprising/unexpected difference in behavior.

I think casting the 'clause' to (Node*) in process_implied_equality is
unnecessary - it was probably needed when it was declared as Expr* but
the patch changes that.

As for the backpatching, I don't feel very strongly about it. It's
clearly a bug/thinko in the code, and I don't see any obvious risks in
backpatching it (no ABI breaks etc.). OTOH multi-column foreign keys are
not very common, and the query pattern seems rather unusual too, so the
risk is pretty low I guess. We certainly did not get many reports, so.

>I'd failed to generate a test case I liked yesterday, but perhaps
>the attached will do. (While the new code is exercised in the
>core regression tests already, it doesn't produce any visible
>plan changes.) I'm a little nervous about whether the plan
>shape will be stable in the buildfarm, but it works for me on
>both 64-bit and 32-bit machines, so probably it's OK.
>

Works fine on raspberry pi 4 (i.e. armv7l, 32-bit arm) too.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services