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
