Re: pg9.6 segfault using simple query (related to use fk for join estimates)

Hi,

On 05/05/2016 04:48 PM, David Rowley wrote:
> On 5 May 2016 at 16:04, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> I've started making some improvements to this, but need to talk to
>> Tomas. It's currently in the middle of his night, but will try to
>> catch him in his morning to discuss this with him.
>
> Ok, so I spoke to Tomas about this briefly, and he's asked me to
> send in this patch. He didn't get time to look over it due to some
> other commitments he has today.
>

Thanks!
>
> I do personally feel that if the attached is not good enough, or not
> very close to good enough then probably the best course of action is
> to revert the whole thing. I'd much rather have this out than to
> feel some responsibility for someone's performance problems with
> this feature.>

I share this opinion. If the approach proposed here is deemed not good 
enough, then +1 to revert.

I think we can further limit the impact by ignoring foreign keys on a 
single column, because the primary goal of the patch is improving 
estimates with multi-column FKs (and matching joins). I'd argue that 99% 
of the FKs in practice is single-column ones, but sure - if you have a 
database with many multi-column FKs this won't help.

I think it's also important to point out that whatever solution we 
choose, it should probably allow us to relax some of the restrictions in 
the future. For example, with a FK on 3 columns, the current patch only 
handles a "full match" joins, with conditions on all three columns. But 
it may be possible to also improve estimates on just two columns - the 
current patch does not do that, as that needs definitely further more 
thought and discussion.
>
> But I also do feel that the patch allows a solution to a big problem
> that we currently have with PostgreSQL, which there's any easy
> workarounds for... that's multi-column join estimates.
>

In a sense, yes. FKs allow us to improve estimates for a fairly narrow 
case of multi-column estimates - joins matching multi-column FKs. And 
for this (not entirely narrow) case they are actually more accurate and 
way cheaper than e.g. histograms or MCV lists (at least in the 
multi-column case).

FWIW the current multivariate stats patch does not even try to mess with 
joins, as it's quite complicated in the multi-column case.

>
> In the attached I've left the GUC remaining. The reason for the GUC
> is for testing purposes and it should be removed before release. It
> should likely be documented though, even if we're planning to remove
> it later. I've not gotten around to that in this patch though.
>

Yes, removal of the GUC should go to the Open Items list.


I'd also like to make it clear that all the shitty parts of the patch 
are my fault, not David's. His name is on the patch as he provided a lot 
of useful ideas, pieces of code and feedback in general. But it was up 
to me to craft the final patch - and I clearly failed to do so. I think 
David still feels responsible for the patch as he made a lot of effort 
to salvage it over the last few days, so I'd like to set the record 
straight.


regards

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