On Tue, Jan 22, 2019 at 4:26 AM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > > Hello, I gave this patch a very quick scan. I didn't check the actual > logic behind it. > > This comment seems wrong: > > + * However weak implication fails: e.g., "NULL IS NOT NULL" is false, but > + * "NULL = ANY(ARRAY[NULL])" is NULL, so non-falsity does not imply > non-falsity. > > "non-falsity does not imply non-falsity"? I suppose one of those > negations should be different ...
Earlier in the file weak implication (comments above predicate_implied_by) is defined as "non-falsity of A implies non-falsity of B". In this example we have NULL for A (non-false) but false for B, so that definition doesn't hold. So I think the comment is accurate, but I can reword if you have an idea of what you'd like to see (I've tweaked a bit in the attached patch to start). > I think the name clause_proved_for_null_test() is a bit weird, being in > the past tense. I'd maybe change "proved" to "proves". Changed. > s/exppresions/expresions/ in the test files. Fixed. James Coleman
saop_is_not_null-v8.patch
Description: Binary data
