overflow bug for inhcounts

Per a review comment on the not-null constraint patch from Jian He, I
played a bit with the various inhcount variables in the tree, with this
simple test which creates a child table and 2^16+1 parents:

for i in `seq 1 $((2 ** 16+1))`; do echo "create table parent_$i (a int);"; done | psql

(echo "create table child () inherits (" ;
for i in `seq 1 $(( 2**16 ))`; do
echo -n "parent_$i,";
done;
echo "parent_$((2**16 + 1))) ;" ) | psql

(Needs a larger-than-default max_locks_per_transaction). With current
master, this command finishes successfully, but sets attinhcount to 1
for the column:

select attname, attnum, attinhcount from pg_attribute where attrelid = 'child'::regclass and attnum > 0;
attname │ attnum │ attinhcount
─────────┼────────┼─────────────
a │ 1 │ 1

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.

At this point we can disinherit the table from a single parent (which
will bring the attinhcount to 0) and then drop the column. This breaks
any query that examines one of the other 2^16 parents,

=# alter table child no inherit parent_1;
ALTER TABLE
=# alter table child drop column a;
ALTER TABLE
=# select * from parent_2;
ERROR: could not find inherited attribute "a" of relation "child"

I suppose the same could happen with CookedConstraint->inhcount, which
is also 32-bit int, but I didn't build an example for that.

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").

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), 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.

Oh and actually, we could change all these variables to be unsigned,
since there's no use for negative inhcounts. The patch doesn't do that;
it'd require changing the subtraction paths to use overflow-protected
ops as well.

Opinions?

(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.)

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Las cosas son buenas o malas segun las hace nuestra opinión" (Lisias)