Re: Proving IS NOT NULL inference for ScalarArrayOpExpr's

From: James Coleman <jtc331(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proving IS NOT NULL inference for ScalarArrayOpExpr's
Date: 2019-01-22 15:43:02
Message-ID: CAAaqYe-R2wYxQp=S1Ur=7VcWFJqF0mPfApivVLByD7hwDSfMog@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Tue, Jan 22, 2019 at 4:26 AM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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

Attachment Content-Type Size
saop_is_not_null-v8.patch application/octet-stream 26.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-01-22 15:46:29 Re: PSA: we lack TAP test coverage on NetBSD and OpenBSD
Previous Message Fabien COELHO 2019-01-22 15:36:04 Re: pg_dump multi VALUES INSERT