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 = 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
theexpression cattr1->attnullability == ATTNULLABLE_UNKNOWN will always be false, That means the second assertion is
effectivelyjust 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
secondassertion 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
