MIME-Version: 1.0
References: <6E6BD43A-89D5-457D-8825-E7282422802F@gmail.com>
In-Reply-To: <6E6BD43A-89D5-457D-8825-E7282422802F@gmail.com>
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Tue, 17 Mar 2026 11:12:45 +0300
Message-ID: 
 <CAN55FZ2rRmjAdV-yv6xO6HYpwT1HwXp0vUt1Bse-kmwew_qi-Q@mail.gmail.com>
Subject: Re: tupdesc: simplify assert in equalTupleDescs()
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAN55FZ2rRmjAdV-yv6xO6HYpwT1HwXp0vUt1Bse-kmwew_qi-Q%40mail.gmail.com>
Precedence: bulk

Hi,

On Tue, 17 Mar 2026 at 10:22, Chao Li <li.evan.chao@gmail.com> wrote:
>
> While reading the code, I saw these assertions in equalTupleDescs():
> ```
>                 CompactAttribute *cattr1 =3D TupleDescCompactAttr(tupdesc=
1, i);
>                 CompactAttribute *cattr2 =3D TupleDescCompactAttr(tupdesc=
2, i);
>
>                 Assert(cattr1->attnullability !=3D ATTNULLABLE_UNKNOWN);
>                 Assert((cattr1->attnullability =3D=3D ATTNULLABLE_UNKNOWN=
) =3D=3D
>                            (cattr2->attnullability =3D=3D ATTNULLABLE_UNK=
NOWN));
>
> ```
>
> The first assertion already guarantees that cattr1->attnullability is not=
 ATTNULLABLE_UNKNOWN, so in the second one the expression cattr1->attnullab=
ility =3D=3D ATTNULLABLE_UNKNOWN will always be false, That means the secon=
d assertion is effectively just checking that cattr2->attnullability is als=
o not ATTNULLABLE_UNKNOWN.
>
> So the current code is correct, but it feels a bit harder to read than ne=
cessary. 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 !=3D
attr2->attnotnull)' is returning false.

--=20
Regards,
Nazir Bilal Yavuz
Microsoft