Re: Bad estimate with partial index

On 4/20/22 16:15, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
>> The whole idea is that instead of bailing out for non-RestrictInfo case,
>> it calculates the necessary information for the clause from scratch.
>> This means relids and pseudoconstant flag, which are checked to decide
>> if the clause is compatible with extended stats.
> 
> Right.
> 
>> But when inspecting how to calculate pseudoconstant, I realized that
>> maybe that's not really needed. Per distribute_qual_to_rels() we only
>> set it to 'true' when bms_is_empty(relids), and we already check that
>> relids is a singleton, so it can't be empty - which means pseudoconstant
>> can't be true either.
> 
> Yeah, I would not bother with the pseudoconstant-related tests for a
> bare clause.  Patch looks reasonably sane in a quick once-over otherwise,
> and the fact that it fixes the presented test case is promising.

Attached is a slightly more polished patch, adding a couple comments and
removing the unnecessary pseudoconstant checks.

> (If you set enable_indexscan = off, you can verify that the estimate
> for the number of index entries retrieved is now sane.) 

I did that. Sorry for not mentioning that explicitly in my message.

> I did not look to see if there were any other RestrictInfo
> dependencies, though.

I checked the places messing with RestrictInfo in clausesel.c and
src/backend/statististics. Code in clausesel.c seems fine and mcv.c
seems fine to (it merely strips the RestrictInfo).

But dependencies.c might need a fix too, although the issue is somewhat
inverse to this one, because it looks like this:

    if (IsA(clause, RestrictInfo))
    {
        ... do some checks ...
    }

so if there's no RestrictInfo on top, we just accept the clause. I guess
this should do the same thing with checking relids like the fix, but
I've been unable to construct an example demonstrating the issue (it'd
have to be either pseudoconstant or reference multiple rels, which seems
hard to get in btcostestimate).


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company