From e6b5187a616bf3a37df7fd39cb71c710950bb826 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 8 May 2024 13:27:27 +0200
Subject: [PATCH 2/6] Mechanical move of not-null code out of getTableAttrs

---
 src/bin/pg_dump/pg_dump.c | 314 ++++++++++++++++++++------------------
 1 file changed, 166 insertions(+), 148 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 379debac24..1d8b137814 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -292,6 +292,8 @@ static void getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, c
 static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo);
 static void buildMatViewRefreshDependencies(Archive *fout);
 static void getTableDataFKConstraints(void);
+static void determineNotNullFlags(Archive *fout, PGresult *res, int r,
+								  TableInfo *tbinfo, int j, int *notnullcount);
 static char *format_function_arguments(const FuncInfo *finfo, const char *funcargs,
 									   bool is_agg);
 static char *format_function_signature(Archive *fout,
@@ -8702,10 +8704,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_attlen;
 	int			i_attalign;
 	int			i_attislocal;
-	int			i_notnull_name;
-	int			i_notnull_noinherit;
-	int			i_notnull_is_pk;
-	int			i_notnull_inh;
 	int			i_attoptions;
 	int			i_attcollation;
 	int			i_attcompression;
@@ -8900,10 +8898,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	i_attlen = PQfnumber(res, "attlen");
 	i_attalign = PQfnumber(res, "attalign");
 	i_attislocal = PQfnumber(res, "attislocal");
-	i_notnull_name = PQfnumber(res, "notnull_name");
-	i_notnull_noinherit = PQfnumber(res, "notnull_noinherit");
-	i_notnull_is_pk = PQfnumber(res, "notnull_is_pk");
-	i_notnull_inh = PQfnumber(res, "notnull_inh");
 	i_attoptions = PQfnumber(res, "attoptions");
 	i_attcollation = PQfnumber(res, "attcollation");
 	i_attcompression = PQfnumber(res, "attcompression");
@@ -8980,10 +8974,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 
 		for (int j = 0; j < numatts; j++, r++)
 		{
-			bool		use_named_notnull = false;
-			bool		use_unnamed_notnull = false;
-			bool		use_throwaway_notnull = false;
-
 			if (j + 1 != atoi(PQgetvalue(res, r, i_attnum)))
 				pg_fatal("invalid column numbering in table \"%s\"",
 						 tbinfo->dobj.name);
@@ -9003,142 +8993,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->attalign[j] = *(PQgetvalue(res, r, i_attalign));
 			tbinfo->attislocal[j] = (PQgetvalue(res, r, i_attislocal)[0] == 't');
 
-			/*
-			 * Not-null constraints require a jumping through a few hoops.
-			 * First, if the user has specified a constraint name that's not
-			 * the system-assigned default name, then we need to preserve
-			 * that. But if they haven't, then we don't want to use the
-			 * verbose syntax in the dump output. (Also, in versions prior to
-			 * 17, there was no constraint name at all.)
-			 *
-			 * (XXX Comparing the name this way to a supposed default name is
-			 * a bit of a hack, but it beats having to store a boolean flag in
-			 * pg_constraint just for this, or having to compute the knowledge
-			 * at pg_dump time from the server.)
-			 *
-			 * We also need to know if a column is part of the primary key. In
-			 * that case, we want to mark the column as not-null at table
-			 * creation time, so that the table doesn't have to be scanned to
-			 * check for nulls when the PK is created afterwards; this is
-			 * especially critical during pg_upgrade (where the data would not
-			 * be scanned at all otherwise.)  If the column is part of the PK
-			 * and does not have any other not-null constraint, then we
-			 * fabricate a throwaway constraint name that we later use to
-			 * remove the constraint after the PK has been created.
-			 *
-			 * For inheritance child tables, we don't want to print not-null
-			 * when the constraint was defined at the parent level instead of
-			 * locally.
-			 */
-
-			/*
-			 * We use notnull_inh to suppress unwanted not-null constraints in
-			 * inheritance children, when said constraints come from the
-			 * parent(s).
-			 */
-			tbinfo->notnull_inh[j] = PQgetvalue(res, r, i_notnull_inh)[0] == 't';
-
-			if (fout->remoteVersion < 170000)
-			{
-				if (!PQgetisnull(res, r, i_notnull_name) &&
-					dopt->binary_upgrade &&
-					!tbinfo->ispartition &&
-					tbinfo->notnull_inh[j])
-				{
-					use_named_notnull = true;
-					/* XXX should match ChooseConstraintName better */
-					tbinfo->notnull_constrs[j] =
-						psprintf("%s_%s_not_null", tbinfo->dobj.name,
-								 tbinfo->attnames[j]);
-				}
-				else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't')
-				{
-					/*
-					 * We want this flag to be set for columns of a primary
-					 * key in which data is going to be loaded by the dump we
-					 * produce; thus a partitioned table doesn't need it.
-					 */
-					if (tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
-						use_throwaway_notnull = true;
-				}
-				else if (!PQgetisnull(res, r, i_notnull_name))
-					use_unnamed_notnull = true;
-			}
-			else
-			{
-				if (!PQgetisnull(res, r, i_notnull_name))
-				{
-					/*
-					 * In binary upgrade of inheritance child tables, must
-					 * have a constraint name that we can UPDATE later.
-					 */
-					if (dopt->binary_upgrade &&
-						!tbinfo->ispartition &&
-						tbinfo->notnull_inh[j])
-					{
-						use_named_notnull = true;
-						tbinfo->notnull_constrs[j] =
-							pstrdup(PQgetvalue(res, r, i_notnull_name));
-
-					}
-					else
-					{
-						char	   *default_name;
-
-						/* XXX should match ChooseConstraintName better */
-						default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
-												tbinfo->attnames[j]);
-						if (strcmp(default_name,
-								   PQgetvalue(res, r, i_notnull_name)) == 0)
-							use_unnamed_notnull = true;
-						else
-						{
-							use_named_notnull = true;
-							tbinfo->notnull_constrs[j] =
-								pstrdup(PQgetvalue(res, r, i_notnull_name));
-						}
-					}
-				}
-				else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't')
-				{
-					/* see above */
-					if (tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
-						use_throwaway_notnull = true;
-				}
-			}
-
-			if (use_unnamed_notnull)
-			{
-				tbinfo->notnull_constrs[j] = "";
-				tbinfo->notnull_throwaway[j] = false;
-			}
-			else if (use_named_notnull)
-			{
-				/* The name itself has already been determined */
-				tbinfo->notnull_throwaway[j] = false;
-			}
-			else if (use_throwaway_notnull)
-			{
-				/*
-				 * Give this constraint a throwaway name.
-				 */
-				tbinfo->notnull_constrs[j] =
-					psprintf("pgdump_throwaway_notnull_%d", notnullcount++);
-				tbinfo->notnull_throwaway[j] = true;
-				tbinfo->notnull_inh[j] = false;
-			}
-			else
-			{
-				tbinfo->notnull_constrs[j] = NULL;
-				tbinfo->notnull_throwaway[j] = false;
-			}
-
-			/*
-			 * Throwaway constraints must always be NO INHERIT; otherwise do
-			 * what the catalog says.
-			 */
-			tbinfo->notnull_noinh[j] = use_throwaway_notnull ||
-				PQgetvalue(res, r, i_notnull_noinherit)[0] == 't';
+			/* Handle not-null constraint name flags separately */
+			determineNotNullFlags(fout, res, r,
+								  tbinfo, j, &notnullcount);
 
 			tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, r, i_attoptions));
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, r, i_attcollation));
@@ -9428,6 +9285,167 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	destroyPQExpBuffer(checkoids);
 }
 
+/*
+ * Based on the getTableAttrs query for one row, set the name and flags to
+ * handle its not-null constraint.
+ *
+ * Result row 'r' is for tbinfo's attribute 'j'.
+ */
+static void
+determineNotNullFlags(Archive *fout, PGresult *res, int r,
+					  TableInfo *tbinfo, int j, int *notnullcount)
+{
+	DumpOptions *dopt = fout->dopt;
+	int			i_notnull_name;
+	int			i_notnull_noinherit;
+	int			i_notnull_is_pk;
+	int			i_notnull_inh;
+	bool		use_named_notnull = false;
+	bool		use_unnamed_notnull = false;
+	bool		use_throwaway_notnull = false;
+
+	i_notnull_name = PQfnumber(res, "notnull_name");
+	i_notnull_noinherit = PQfnumber(res, "notnull_noinherit");
+	i_notnull_is_pk = PQfnumber(res, "notnull_is_pk");
+	i_notnull_inh = PQfnumber(res, "notnull_inh");
+
+	/*
+	 * Not-null constraints require a jumping through a few hoops.
+	 * First, if the user has specified a constraint name that's not
+	 * the system-assigned default name, then we need to preserve
+	 * that. But if they haven't, then we don't want to use the
+	 * verbose syntax in the dump output. (Also, in versions prior to
+	 * 17, there was no constraint name at all.)
+	 *
+	 * (XXX Comparing the name this way to a supposed default name is
+	 * a bit of a hack, but it beats having to store a boolean flag in
+	 * pg_constraint just for this, or having to compute the knowledge
+	 * at pg_dump time from the server.)
+	 *
+	 * We also need to know if a column is part of the primary key. In
+	 * that case, we want to mark the column as not-null at table
+	 * creation time, so that the table doesn't have to be scanned to
+	 * check for nulls when the PK is created afterwards; this is
+	 * especially critical during pg_upgrade (where the data would not
+	 * be scanned at all otherwise.)  If the column is part of the PK
+	 * and does not have any other not-null constraint, then we
+	 * fabricate a throwaway constraint name that we later use to
+	 * remove the constraint after the PK has been created.
+	 *
+	 * For inheritance child tables, we don't want to print not-null
+	 * when the constraint was defined at the parent level instead of
+	 * locally.
+	 */
+
+	/*
+	 * We use notnull_inh to suppress unwanted not-null constraints in
+	 * inheritance children, when said constraints come from the
+	 * parent(s).
+	 */
+	tbinfo->notnull_inh[j] = PQgetvalue(res, r, i_notnull_inh)[0] == 't';
+
+	if (fout->remoteVersion < 170000)
+	{
+		if (!PQgetisnull(res, r, i_notnull_name) &&
+			dopt->binary_upgrade &&
+			!tbinfo->ispartition &&
+			tbinfo->notnull_inh[j])
+		{
+			use_named_notnull = true;
+			/* XXX should match ChooseConstraintName better */
+			tbinfo->notnull_constrs[j] =
+				psprintf("%s_%s_not_null", tbinfo->dobj.name,
+						 tbinfo->attnames[j]);
+		}
+		else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't')
+		{
+			/*
+			 * We want this flag to be set for columns of a primary
+			 * key in which data is going to be loaded by the dump we
+			 * produce; thus a partitioned table doesn't need it.
+			 */
+			if (tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
+				use_throwaway_notnull = true;
+		}
+		else if (!PQgetisnull(res, r, i_notnull_name))
+			use_unnamed_notnull = true;
+	}
+	else
+	{
+		if (!PQgetisnull(res, r, i_notnull_name))
+		{
+			/*
+			 * In binary upgrade of inheritance child tables, must
+			 * have a constraint name that we can UPDATE later.
+			 */
+			if (dopt->binary_upgrade &&
+				!tbinfo->ispartition &&
+				tbinfo->notnull_inh[j])
+			{
+				use_named_notnull = true;
+				tbinfo->notnull_constrs[j] =
+					pstrdup(PQgetvalue(res, r, i_notnull_name));
+			}
+			else
+			{
+				char	   *default_name;
+
+				/* XXX should match ChooseConstraintName better */
+				default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
+										tbinfo->attnames[j]);
+				if (strcmp(default_name,
+						   PQgetvalue(res, r, i_notnull_name)) == 0)
+					use_unnamed_notnull = true;
+				else
+				{
+					use_named_notnull = true;
+					tbinfo->notnull_constrs[j] =
+						pstrdup(PQgetvalue(res, r, i_notnull_name));
+				}
+			}
+		}
+		else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't')
+		{
+			/* see above */
+			if (tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
+				use_throwaway_notnull = true;
+		}
+	}
+
+	if (use_unnamed_notnull)
+	{
+		tbinfo->notnull_constrs[j] = "";
+		tbinfo->notnull_throwaway[j] = false;
+	}
+	else if (use_named_notnull)
+	{
+		/* The name itself has already been determined */
+		tbinfo->notnull_throwaway[j] = false;
+	}
+	else if (use_throwaway_notnull)
+	{
+		/*
+		 * Give this constraint a throwaway name.
+		 */
+		tbinfo->notnull_constrs[j] =
+			psprintf("pgdump_throwaway_notnull_%d", (*notnullcount)++);
+		tbinfo->notnull_throwaway[j] = true;
+		tbinfo->notnull_inh[j] = false;
+	}
+	else
+	{
+		tbinfo->notnull_constrs[j] = NULL;
+		tbinfo->notnull_throwaway[j] = false;
+	}
+
+	/*
+	 * Throwaway constraints must always be NO INHERIT; otherwise do
+	 * what the catalog says.
+	 */
+	tbinfo->notnull_noinh[j] = use_throwaway_notnull ||
+		PQgetvalue(res, r, i_notnull_noinherit)[0] == 't';
+}
+
 /*
  * Test whether a column should be printed as part of table's CREATE TABLE.
  * Column number is zero-based.
-- 
2.39.2

