Amit Langote <langote_amit...@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