public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: jian he <[email protected]>
Subject: Re: overflow bug for inhcounts
Date: Tue, 08 Oct 2024 12:47:08 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Alvaro Herrera <[email protected]> writes:
> ... This is because ColumnDef->inhcount is a 32-bit int, but
> Form_pg_attribute->attinhcount is int16, so we didn't break the overflow
> test for ColumnDef inhcount, but attinhcount has overflowed during
> assignment.

Ugh ... somebody's ancient oversight there.  Or maybe attinhcount
was once int32, and we narrowed it for space reasons?

> From branch master, I propose we change those two members to int16
> (ColumnDef->inhcount and CookedConstraint->inhcount) to make those
> counters consistently use the same type; and then use
> pg_add_s16_overflow() instead of ++ for the increments, as in the
> attached patch.  With this patch, the child table creation fails as
> expected ("too many inheritance parents").

+1.  I didn't check if there were any other places to touch, but
this looks like a good solution for master.

> In stable branches, I see two possible approaches: we could use the same
> ptach as master (but add another int16 to the struct as padding, to
> avoid changing the struct layout),

That would not preserve ABI on machines with the wrong endianness.

> or, less intrusive, we could leave
> that alone and instead change the "overflow" after the addition to test
> inhcount > PG_INT16_MAX instead of < 0.  Or we could leave it all alone.

On the whole I'd leave it alone in back branches.  Nobody who's not
intentionally trying to break their table will hit this.

> (I'm not terribly enthused about adding a test that creates a child
> table with 2^16 parents, because of the added runtime -- on my machine
> the scripts above take about 4 seconds.)

Agreed, too expensive for the value.

			regards, tom lane

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], [email protected], [email protected]
  Subject: Re: overflow bug for inhcounts
  In-Reply-To: <[email protected]>

* 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