Re: A bug in mapping attributes in ATExecAttachPartition()

On Tue, Jun 13, 2017 at 5:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> OK, I think I see the problem here.  predicate_implied_by() and
>> predicate_refuted_by() differ in what they assume about the predicate
>> evaluating to NULL, but both of them assume that if the clause
>> evaluates to NULL, that's equivalent to false.  So there's actually no
>> option to get the behavior we want here, which is to treat both
>> operands using CHECK-semantics (null is true) rather than
>> WHERE-semantics (null is false).
>
>> Given that, Ashutosh's proposal of passing an additional flag to
>> predicate_implied_by() seems like the best option.  Here's a patch
>> implementing that.
>
> I've not reviewed the logic changes in predtest.c in detail, but
> I think this is a reasonable direction to go in.  Two suggestions:
>
> 1. predicate_refuted_by() should grow the extra argument at the
> same time.  There's no good reason to be asymmetric.

OK.

> 2. It might be clearer, and would certainly be shorter, to call the
> extra argument something like "null_is_true".

I think it's pretty darn important to make it clear that the argument
only applies to the clauses supplied as axioms, and not to the
predicate to be proven; if you want to control how the *predicate* is
handled with respect to nulls, change your selection as among
predicate_implied_by() and predicate_refuted_by().  For that reason, I
disesteem null_is_true, but I'm open to other suggestions.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company