public inbox for [email protected]  

help / color / mirror / Atom feed

Re: tupdesc: simplify assert in equalTupleDescs()
3+ messages / 3 participants
[nested] [flat]

* Re: tupdesc: simplify assert in equalTupleDescs()
@ 2026-03-17 08:12  Nazir Bilal Yavuz <[email protected]>
  1 sibling, 0 replies; 3+ messages in thread

From: Nazir Bilal Yavuz @ 2026-03-17 08:12 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: pgsql-hackers

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





^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: tupdesc: simplify assert in equalTupleDescs()
@ 2026-04-28 18:29  surya poondla <[email protected]>
  1 sibling, 1 reply; 3+ messages in thread

From: surya poondla @ 2026-04-28 18:29 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: pgsql-hackers

Hi Chao,

Thank you for working on this.

The patch looks good to me and makes the code more easy to read.

Regards,
Surya Poondla


^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: tupdesc: simplify assert in equalTupleDescs()
@ 2026-04-30 06:22  Chao Li <[email protected]>
  parent: surya poondla <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Chao Li @ 2026-04-30 06:22 UTC (permalink / raw)
  To: surya poondla <[email protected]>; +Cc: pgsql-hackers



> On Apr 29, 2026, at 02:29, surya poondla <[email protected]> wrote:
> 
> Hi Chao,
> 
> Thank you for working on this.
> 
> The patch looks good to me and makes the code more easy to read.
> 
> Regards,
> Surya Poondla

Thanks for your review.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









^ permalink  raw  reply  [nested|flat] 3+ messages in thread

end of thread, other threads:[~2026-04-30 06:22 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-17 08:12 ` Nazir Bilal Yavuz <[email protected]>
2026-04-28 18:29 ` surya poondla <[email protected]>
2026-04-30 06:22   ` Chao Li <[email protected]>

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