public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nazir Bilal Yavuz <[email protected]>
To: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: tupdesc: simplify assert in equalTupleDescs()
Date: Tue, 17 Mar 2026 11:12:45 +0300
Message-ID: <CAN55FZ2rRmjAdV-yv6xO6HYpwT1HwXp0vUt1Bse-kmwew_qi-Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Hi,

On Tue, 17 Mar 2026 at 10:22, Chao Li <[email protected]> wrote:
>
> While reading the code, I saw these assertions in equalTupleDescs():
> ```
>                 CompactAttribute *cattr1 = TupleDescCompactAttr(tupdesc1, i);
>                 CompactAttribute *cattr2 = TupleDescCompactAttr(tupdesc2, i);
>
>                 Assert(cattr1->attnullability != ATTNULLABLE_UNKNOWN);
>                 Assert((cattr1->attnullability == ATTNULLABLE_UNKNOWN) ==
>                            (cattr2->attnullability == ATTNULLABLE_UNKNOWN));
>
> ```
>
> The first assertion already guarantees that cattr1->attnullability is not ATTNULLABLE_UNKNOWN, so in the second one the expression cattr1->attnullability == ATTNULLABLE_UNKNOWN will always be false, That means the second assertion is effectively just checking that cattr2->attnullability is also not ATTNULLABLE_UNKNOWN.
>
> So the current code is correct, but it feels a bit harder to read than necessary. This patch just simplifies the second assertion in a direct way.

Thank you for the report! You are right and the patch looks good to me.

Nitpick: It is still a bit hard to understand why
'cattr2->attnullability' should not be equal to 'ATTNULLABLE_UNKNOWN'.
It would be good to add a comment explaining that 'attr2->attnotnull'
should be true too because 'if (attr1->attnotnull !=
attr2->attnotnull)' is returning false.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: tupdesc: simplify assert in equalTupleDescs()
  In-Reply-To: <CAN55FZ2rRmjAdV-yv6xO6HYpwT1HwXp0vUt1Bse-kmwew_qi-Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox