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
