Unbreak overflow test for attinhcount/coninhcount

Commit 90189ee narrowed pg_attribute.attinhcount and
pg_constraint.coninhcount from 32 to 16 bits, but kept other related
structs with 32-bit wide fields: ColumnDef and CookedConstraint contain
an int 'inhcount' field which is itself checked for overflow on
increments, but there's no check that the values aren't above INT16_MAX
before assigning to the catalog columns.  This means that a creative
user can get a inconsistent table definition and override some
protections.

Fix it by changing those other structs to also use int16.

Also, modernize style by using pg_add_s16_overflow for overflow testing
instead of checking for negative values.

We also have Constraint.inhcount, which is here removed completely.
This was added by commit b0e96f3 and not removed by its revert at
6f8bb7c.  It is not needed by the upcoming not-null constraints
patch.

This is mostly academic, so we agreed not to backpatch to avoid ABI
problems.

Bump catversion because of the changes to parse nodes.

Co-authored-by: Álvaro Herrera <[email protected]>
Co-authored-by: 何建 (jian he) <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/[email protected]

Author: alvherre

src/backend/catalog/heap.c

@@ -102,7 +102,7 @@ static ObjectAddress AddNewRelationType(const char *typeName,
 										Oid new_array_type);
 static void RelationRemoveInheritance(Oid relid);
 static Oid	StoreRelCheck(Relation rel, const char *ccname, Node *expr,
-						  bool is_validated, bool is_local, int inhcount,
+						  bool is_validated, bool is_local, int16 inhcount,
 						  bool is_no_inherit, bool is_internal);
 static void StoreConstraints(Relation rel, List *cooked_constraints,
 							 bool is_internal);
@@ -2072,7 +2072,7 @@ SetAttrMissing(Oid relid, char *attname, char *value)
  */
 static Oid
 StoreRelCheck(Relation rel, const char *ccname, Node *expr,
-			  bool is_validated, bool is_local, int inhcount,
+			  bool is_validated, bool is_local, int16 inhcount,
 			  bool is_no_inherit, bool is_internal)
 {
 	char	   *ccbin;
@@ -2624,10 +2624,8 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
 		{
 			if (is_local)
 				con->conislocal = true;
-			else
-				con->coninhcount++;
-
-			if (con->coninhcount < 0)
+			else if (pg_add_s16_overflow(con->coninhcount, 1,
+										 &con->coninhcount))
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));

src/backend/catalog/index.c

@@ -1900,7 +1900,7 @@ index_constraint_create(Relation heapRelation,
 	bool		islocal;
 	bool		noinherit;
 	bool		is_without_overlaps;
-	int			inhcount;
+	int16		inhcount;
 
 	deferrable = (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) != 0;
 	initdeferred = (constr_flags & INDEX_CONSTR_CREATE_INIT_DEFERRED) != 0;

src/backend/catalog/pg_constraint.c

@@ -27,6 +27,7 @@
 #include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
+#include "common/int.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -74,7 +75,7 @@ CreateConstraintEntry(const char *constraintName,
 					  Node *conExpr,
 					  const char *conBin,
 					  bool conIsLocal,
-					  int conInhCount,
+					  int16 conInhCount,
 					  bool conNoInherit,
 					  bool conPeriod,
 					  bool is_internal)
@@ -849,11 +850,12 @@ ConstraintSetParentConstraint(Oid childConstrId,
 				 childConstrId);
 
 		constrForm->conislocal = false;
-		constrForm->coninhcount++;
-		if (constrForm->coninhcount < 0)
+		if (pg_add_s16_overflow(constrForm->coninhcount, 1,
+								&constrForm->coninhcount))
 			ereport(ERROR,
 					errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 					errmsg("too many inheritance parents"));
+
 		constrForm->conparentid = parentConstrId;
 
 		CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);

src/backend/commands/tablecmds.c

@@ -66,6 +66,7 @@
 #include "commands/typecmds.h"
 #include "commands/user.h"
 #include "commands/vacuum.h"
+#include "common/int.h"
 #include "executor/executor.h"
 #include "foreign/fdwapi.h"
 #include "foreign/foreign.h"
@@ -3044,8 +3045,8 @@ MergeCheckConstraint(List *constraints, const char *name, Node *expr)
 		if (equal(expr, ccon->expr))
 		{
 			/* OK to merge constraint with existing */
-			ccon->inhcount++;
-			if (ccon->inhcount < 0)
+			if (pg_add_s16_overflow(ccon->inhcount, 1,
+									&ccon->inhcount))
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));
@@ -3347,8 +3348,8 @@ MergeInheritedAttribute(List *inh_columns,
 	 * Default and other constraints are handled by the caller.
 	 */
 
-	prevdef->inhcount++;
-	if (prevdef->inhcount < 0)
+	if (pg_add_s16_overflow(prevdef->inhcount, 1,
+							&prevdef->inhcount))
 		ereport(ERROR,
 				errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 				errmsg("too many inheritance parents"));
@@ -7089,8 +7090,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 								   get_collation_name(childatt->attcollation))));
 
 			/* Bump the existing child att's inhcount */
-			childatt->attinhcount++;
-			if (childatt->attinhcount < 0)
+			if (pg_add_s16_overflow(childatt->attinhcount, 1,
+									&childatt->attinhcount))
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));
@@ -10170,7 +10171,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 	Oid			constrOid;
 	char	   *conname;
 	bool		conislocal;
-	int			coninhcount;
+	int16		coninhcount;
 	bool		connoinherit;
 	Oid			deleteTriggerOid,
 				updateTriggerOid;
@@ -10549,9 +10550,9 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
 									  NULL,
 									  NULL,
 									  NULL,
-									  false,
-									  1,
-									  false,
+									  false,	/* conIsLocal */
+									  1,	/* conInhCount */
+									  false,	/* conNoInherit */
 									  with_period,	/* conPeriod */
 									  false);
 
@@ -11076,8 +11077,8 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 								  NULL,
 								  NULL,
 								  NULL,
-								  false,	/* islocal */
-								  1,	/* inhcount */
+								  false,	/* conIsLocal */
+								  1,	/* conInhCount */
 								  false,	/* conNoInherit */
 								  with_period,	/* conPeriod */
 								  true);
@@ -15944,8 +15945,8 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispart
 			 * OK, bump the child column's inheritance count.  (If we fail
 			 * later on, this change will just roll back.)
 			 */
-			child_att->attinhcount++;
-			if (child_att->attinhcount < 0)
+			if (pg_add_s16_overflow(child_att->attinhcount, 1,
+									&child_att->attinhcount))
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));
@@ -16075,8 +16076,9 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
 			 */
 			child_copy = heap_copytuple(child_tuple);
 			child_con = (Form_pg_constraint) GETSTRUCT(child_copy);
-			child_con->coninhcount++;
-			if (child_con->coninhcount < 0)
+
+			if (pg_add_s16_overflow(child_con->coninhcount, 1,
+									&child_con->coninhcount))
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));

src/include/catalog/catversion.h

@@ -57,6 +57,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202410111
+#define CATALOG_VERSION_NO	202410112
 
 #endif

src/include/catalog/heap.h

@@ -41,7 +41,7 @@ typedef struct CookedConstraint
 	Node	   *expr;			/* transformed default or check expr */
 	bool		skip_validation;	/* skip validation? (only for CHECK) */
 	bool		is_local;		/* constraint has local (non-inherited) def */
-	int			inhcount;		/* number of times constraint is inherited */
+	int16		inhcount;		/* number of times constraint is inherited */
 	bool		is_no_inherit;	/* constraint has local def and cannot be
 								 * inherited */
 } CookedConstraint;

src/include/catalog/pg_constraint.h

@@ -245,7 +245,7 @@ extern Oid	CreateConstraintEntry(const char *constraintName,
 								  Node *conExpr,
 								  const char *conBin,
 								  bool conIsLocal,
-								  int conInhCount,
+								  int16 conInhCount,
 								  bool conNoInherit,
 								  bool conPeriod,
 								  bool is_internal);

src/include/nodes/parsenodes.h

@@ -728,7 +728,7 @@ typedef struct ColumnDef
 	char	   *colname;		/* name of column */
 	TypeName   *typeName;		/* type of column */
 	char	   *compression;	/* compression method for column */
-	int			inhcount;		/* number of times column is inherited */
+	int16		inhcount;		/* number of times column is inherited */
 	bool		is_local;		/* column has local (non-inherited) def'n */
 	bool		is_not_null;	/* NOT NULL constraint specified? */
 	bool		is_from_type;	/* column definition came from table type */
@@ -2754,8 +2754,6 @@ typedef struct Constraint
 	char	   *cooked_expr;	/* CHECK or DEFAULT expression, as
 								 * nodeToString representation */
 	char		generated_when; /* ALWAYS or BY DEFAULT */
-	int			inhcount;		/* initial inheritance count to apply, for
-								 * "raw" NOT NULL constraints */
 	bool		nulls_not_distinct; /* null treatment for UNIQUE constraints */
 	List	   *keys;			/* String nodes naming referenced key
 								 * column(s); for UNIQUE/PK/NOT NULL */