On Mon, Apr 3, 2017 at 8:52 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
>> One last observation:
>>
>> + /*
>> + * An IS NOT NULL test is a no-op if there's any other strict quals,
>> + * so if that's the case, then we'll only apply this, otherwise we'll
>> + * ignore it.
>> + */
>> + else if (cslist->selmask == CACHESEL_NOTNULLTEST)
>> + s1 *= cslist->notnullsel;
>>
>> In some paths, the range selectivity estimation code punts and returns
>> DEFAULT_RANGE_INEQ_SEL. In those cases, if a not null test was
>> present, it should still be applied. It could make a big difference if
>> the selectivity for the nulltest is high.
>
> I'd say that the location that applies the DEFAULT_RANGE_INEQ_SEL
> should apply the nullfrac. Seems wrong to rely on a IS NOT NULL test
> to exists to estimate that properly. I don't think that needs done as
> part of this patch. I'd rather limit the scope since "returned with
> feedback" is already knocking at the door of this patch.
I agree, but this patch regresses for those cases if the user
compensated with a not null test, leaving little recourse, so I'd fix
it in this patch to avoid the regression.
Maybe you're right that the null fraction should be applied regardless
of whether there's an explicit null check, though.