Date: Tue, 6 Oct 2026 13:19:31 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Vik Fearing <vik@postgresfriends.org>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>,
	pgsql-hackers@lists.postgresql.org,
	Jacob Champion <jacob.champion@enterprisedb.com>,
	Isaac Morland <isaac.morland@gmail.com>
Subject: Re: Logical Implication
Message-ID: <asU7s4y_QzU7yJVZ@nathan>
References: <37c76707-e6fa-4ee3-b57f-aa0bf1b0bba8@postgresfriends.org>
 <aqQL78v7jr0AED_b@nathan>
 <b0edbd8b-66e5-44c0-9b90-516760df1209@postgresfriends.org>
 <arU0qXcp3KSj7M-g@nathan>
 <9ab35dbb-d9a4-4c1e-bae2-7763ddeb720c@postgresfriends.org>
 <CAN4CZFPUmkNyYP=MJbCE3OC21-De9AYhJXOD_wd8Q06E6xBzqQ@mail.gmail.com>
 <3d779ee0-6596-4dae-b79c-e35b237f83bf@postgresfriends.org>
 <ar0mGmQQz6vI6pts@nathan>
 <b70aba34-e799-4c09-aa3e-281a335ebf2f@postgresfriends.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b70aba34-e799-4c09-aa3e-281a335ebf2f@postgresfriends.org>
Archived-At: <https://www.postgresql.org/message-id/asU7s4y_QzU7yJVZ%40nathan>
Precedence: bulk

On Sat, Oct 03, 2026 at 06:18:43PM +0200, Vik Fearing wrote:
> Attached is a new patch set that makes IMPLIES non-associative, changes the
> tests, and also survives a round trip.  I put the latter in a separate patch
> in case it isn't wanted after all.

Thanks!

>      <acronym>SQL</acronym> uses a three-valued logic system with true,
> -    false, and <literal>null</literal>, which represents <quote>unknown</quote>.
> -    Observe the following truth tables:
> +    false, and unknown; the null value of a <type>boolean</type> and the truth
> +    value unknown are one and the same.  In the truth tables below the left
> +    operand of a binary operator selects the row, and the right operand the
> +    column:

I find the reorganization to be an improvement, and I intend to commit this
one sooner than later.  The only thing I'd change is the switch from NULL
to "unknown" in the tables.  Users deal with NULL at the command level, and
the rest of the docs use NULL, so I think the existing text has the right
idea by noting once that NULL represents "unknown" and then using NULL
throughout.

>     <para>
>      The operators <literal>AND</literal> and <literal>OR</literal> are
>      commutative, that is, you can switch the left and right operands
>      without affecting the result.  (However, it is not guaranteed that

This part goes on to mention that there is no guaranteed evaluation order
for AND and OR.  Maybe it should mention IMPLIES, too.

> +						case IMPLIES_EXPR:
> +
> +							/*
> +							 * The planner normally expands this, but in case
> +							 * it didn't, evaluate it as NOT a OR b.  The NOT
> +							 * step doesn't jump, so it needs no adjusting.
> +							 */
> +							Assert(nargs == 2);

IMHO this and the postgres_fdw equivalent should be replaced with
elog(ERROR, ...) instead.  Otherwise, they'll be untested dead code that
mask problems elsewhere.

For the tests, I'd probably trim those down a bit to what feels essential.
For example, we probably don't need to test that IMPLIES is unreserved.
I'll handle this part.

Note to self: this will need a catversion bump.

-- 
nathan