Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> [ v4-0001-Disregard-nulls-in-SAOP-rightarg-array-list-durin.patch ]
This patch seems pretty wrong to me. The proposed proof rule is wrong
for !useOr expressions (that is, "scalar op ALL (array)"); you can't
just ignore null items in that case. It's also wrong when we need strong
refutation. I was about to point to the comment for predicate_refuted_by,
* An important fine point is that truth of the clauses must imply that
* the predicate returns FALSE, not that it does not return TRUE. This
* is normally used to try to refute CHECK constraints, and the only
* thing we can assume about a CHECK constraint is that it didn't return
* FALSE --- a NULL result isn't a violation per the SQL spec. (Someday
and reject the patch as unfixably wrong, when I noticed that in
fact somebody had added support for weak refutation to this code.
Now I'm just desperately unhappy about the lack of comment-updating
work in that commit (b08df9cab, looks like). It's not acceptable
to falsify comments and not update them. I have a nasty feeling that
there are outright bugs in the logic, too.
I'm inclined to think that you've attacked the problem at the wrong place
anyway. The notion that one arm of an OR reducing to NULL might not
prevent proving what we want is by no means specific to ScalarArrayOp.
I think it'd make more sense to see about incorporating that idea in
predicate_implied_by_simple_clause/predicate_refuted_by_simple_clause.
It might be necessary to change the boolean results in the recursive
logic to three-way, not sure.
I'm setting this patch back to Waiting On Author pending fixes
for the above issues. In the meantime I see a separate work item
here to do code review for b08df9cab.
regards, tom lane