Re: constraint exclusion and nulls in IN (..) clause

On 2018/03/10 13:40, Tom Lane wrote:
> I wrote:
>> I think it'd make more sense to see about incorporating that idea in
>> predicate_implied_by_simple_clause/predicate_refuted_by_simple_clause.
>
> After further thought, it seems like the place to deal with this is
> really operator_predicate_proof(), as in the attached draft patch
> against HEAD. This passes the smell test for me, in the sense that
> it's an arguably correct and general extension of the proof rules,
> but it could use more testing.

Thanks for the patch. I agree it handles the case I presented my patch
for in a more principled manner. So, I've marked the CF entry for my
patch as Rejected.

Looking at the patch, I guess it'd be right to think that the case for
which the following block of code will be executed only arises for
OpExpr's generated by predtest.c (that is, when iterating an SAOP):

if (clause_const->constisnull)
+ {
+ /* If clause_op isn't strict, we can't prove anything */
+ if (!op_strict(clause_op))
+ return false;
+
<snip>

That's because any other expr = null would've been reduced to
constant-false at some earlier point.

Then, in the same block I see that there is:

+ /*
+ * For weak implication, it's still possible for the proof to
succeed,
+ * if the predicate can also be proven NULL. In that case we've got
+ * NULL => NULL which is valid for this proof type.
+ */
+ if (pred_const->constisnull && op_strict(pred_op))
+ return true;

which, afaics, will never be able to return true, because
pred_const->constisnull will never be true here. That's because the
following code that will run before the above code will determine the
result of operator_predicate_proof() in that case:

if (equal(pred_leftop, clause_leftop))
{
if (equal(pred_rightop, clause_rightop))
{
/* We have x op1 y and x op2 y */
return operator_same_subexprs_proof(pred_op, clause_op,
refute_it);

I wonder if the result would be valid in that case, because the proof
cache should only be looked up for non-null sub-expressions, no?

> TBH, the change in the existing regression test case in inherit.sql
> makes me itch. We've got
>
> create table list_parted (
> a varchar
> ) partition by list (a);
> ...
> create table part_null_xy partition of list_parted for values in (null, 'xy');
> ...
> explain (costs off) select * from list_parted where a = 'ab' or a in (null, 'cd');
>
> Now, the fact that "null" is included in this query's IN clause is a
> complete no-op, because the IN is using a strict equality operator.
> So it's nice that the planner can see that and realize that there's
> no point in scanning part_null_xy ... but this means that the syntax
> that's been chosen for list partitioning is seriously misleading.
> "in (null, 'xy')" in the CREATE TABLE command has nothing to do with
> the semantics of that identical clause in any other context, and indeed
> it seems chosen in a way to confuse even (or especially?) seasoned
> experts.

Hmm, yeah, it is confusing. Actually, what it really internally becomes
is the following expression:

(a IS NULL) OR (a = 'xy')

or if the CREATE TABLE had "in (null, 'xy', 'yz'), the expression will become:

(a IS NULL) OR (a = ANY (ARRAY['xy'::text, 'yz'::text]))

which, as you say, is not the same thing as how the original expression is
interpreted elsewhere.

> I suppose it's too late to do anything about that now, but it sure
> seems like NULL should've been handled some other way.

Maybe, write something in the documentation about that?

Thanks,
Amit