agora inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v33 5/5] Filling gaps in jsonb arrays
5+ messages / 2 participants
[nested] [flat]

* [PATCH v33 5/5] Filling gaps in jsonb arrays
@ 2020-08-04 15:41 Dmitrii Dolgov <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Dmitrii Dolgov @ 2020-08-04 15:41 UTC (permalink / raw)

Appending or prepending array elements on the specified position, gaps
filled with nulls (similar to JavaScript behavior). Originally proposed
by Nikita Glukhov based on polymorphic subscripting patch, but
transformed into an independent change.
---
 src/backend/utils/adt/jsonfuncs.c   | 43 +++++++++++++++++++++++++----
 src/test/regress/expected/jsonb.out | 24 ++++++++++++++++
 src/test/regress/sql/jsonb.sql      | 13 +++++++++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index f46a2828b3..6bb81f4c40 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -47,6 +47,7 @@
 #define JB_PATH_INSERT_AFTER			0x0010
 #define JB_PATH_CREATE_OR_INSERT \
 	(JB_PATH_INSERT_BEFORE | JB_PATH_INSERT_AFTER | JB_PATH_CREATE)
+#define JB_PATH_FILL_GAPS				0x0020
 
 /* state for json_object_keys */
 typedef struct OkeysState
@@ -1492,10 +1493,8 @@ get_jsonb_path_all(FunctionCallInfo fcinfo, bool as_text)
 static Datum
 jsonb_get_element(Jsonb *jb, Datum *path, int npath, bool *isnull, bool as_text)
 {
-	Jsonb		   *res;
 	JsonbContainer *container = &jb->root;
 	JsonbValue	   *jbvp = NULL;
-	JsonbValue		tv;
 	int				i;
 	bool			have_object = false,
 					have_array = false;
@@ -1657,13 +1656,24 @@ jsonb_set_element(Datum jsonbdatum, Datum *path, int path_len,
 	it = JsonbIteratorInit(&jb->root);
 
 	res = setPath(&it, path, path_nulls, path_len, &state, 0,
-				  newval, JB_PATH_CREATE);
+				  newval, JB_PATH_CREATE | JB_PATH_FILL_GAPS);
 
 	pfree(path_nulls);
 
 	PG_RETURN_JSONB_P(JsonbValueToJsonb(res));
 }
 
+static void
+push_null_elements(JsonbParseState **ps, int num)
+{
+		JsonbValue	null;
+
+		null.type = jbvNull;
+
+		while (num-- > 0)
+				pushJsonbValue(ps, WJB_ELEM, &null);
+}
+
 /*
  * Return the text representation of the given JsonbValue.
  */
@@ -4811,6 +4821,13 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
  * Bits JB_PATH_INSERT_BEFORE and JB_PATH_INSERT_AFTER in op_type
  * behave as JB_PATH_CREATE if new value is inserted in JsonbObject.
  *
+ * If JB_PATH_FILL_GAPS bit is set, this will change an assignment logic in
+ * case if target is an array. The assignment index will not be restricted by
+ * number of elements in the array, and if there are any empty slots between
+ * last element of the array and a new one they will be filled with nulls. If
+ * the index is negative, it still will be considered an an index from the end
+ * of the array.
+ *
  * All path elements before the last must already exist
  * whatever bits in op_type are set, or nothing is done.
  */
@@ -5012,15 +5029,21 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			idx = nelems + idx;
 	}
 
-	if (idx > 0 && idx > nelems)
-		idx = nelems;
+	/*
+	 * Filling the gaps means there are no limits on the positive index are
+	 * imposed, we can set any element. Otherwise limit the index by nelems.
+	 */
+	if (!(op_type & JB_PATH_FILL_GAPS))
+	{
+		if (idx > 0 && idx > nelems)
+			idx = nelems;
+	}
 
 	/*
 	 * if we're creating, and idx == INT_MIN, we prepend the new value to the
 	 * array also if the array is empty - in which case we don't really care
 	 * what the idx value is
 	 */
-
 	if ((idx == INT_MIN || nelems == 0) && (level == path_len - 1) &&
 		(op_type & JB_PATH_CREATE_OR_INSERT))
 	{
@@ -5086,10 +5109,18 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
 			if ((op_type & JB_PATH_CREATE_OR_INSERT) && !done &&
 				level == path_len - 1 && i == nelems - 1)
 			{
+				/*
+				 * If asked to fill the gaps, idx could be bigger than nelems,
+				 * so prepend the new element with nulls if that's the case.
+				 */
+				if (op_type & JB_PATH_FILL_GAPS && idx > nelems)
+					push_null_elements(st, idx - nelems);
+
 				(void) pushJsonbValue(st, WJB_ELEM, newval);
 			}
 		}
 	}
+
 }
 
 /*
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 04a146a7d0..4d52652688 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -4928,6 +4928,30 @@ select * from test_jsonb_subscript;
   2 | {"a": [1, 2, 3], "key": "value", "another_key": null}
 (2 rows)
 
+-- Fill the gaps logic
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[0]');
+update test_jsonb_subscript set test_json[5] = 1;
+select * from test_jsonb_subscript;
+ id |           test_json            
+----+--------------------------------
+  1 | [0, null, null, null, null, 1]
+(1 row)
+
+update test_jsonb_subscript set test_json[-4] = 1;
+select * from test_jsonb_subscript;
+ id |          test_json          
+----+-----------------------------
+  1 | [0, null, 1, null, null, 1]
+(1 row)
+
+update test_jsonb_subscript set test_json[-8] = 1;
+select * from test_jsonb_subscript;
+ id |           test_json            
+----+--------------------------------
+  1 | [1, 0, null, 1, null, null, 1]
+(1 row)
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
                                 to_tsvector                                
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 12541e7e50..584f145bab 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -1269,6 +1269,19 @@ update test_jsonb_subscript set test_json[NULL] = 1;
 update test_jsonb_subscript set test_json['another_key'] = NULL;
 select * from test_jsonb_subscript;
 
+-- Fill the gaps logic
+delete from test_jsonb_subscript;
+insert into test_jsonb_subscript values (1, '[0]');
+
+update test_jsonb_subscript set test_json[5] = 1;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json[-4] = 1;
+select * from test_jsonb_subscript;
+
+update test_jsonb_subscript set test_json[-8] = 1;
+select * from test_jsonb_subscript;
+
 -- jsonb to tsvector
 select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);
 
-- 
2.21.0


--u66dlpg6gaevsofr--





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

* [PATCH v3 3/7] move live_check variable to user_opts
@ 2024-05-21 21:35 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Nathan Bossart @ 2024-05-21 21:35 UTC (permalink / raw)

---
 src/bin/pg_upgrade/check.c       | 32 ++++++++++++++++----------------
 src/bin/pg_upgrade/controldata.c |  5 +++--
 src/bin/pg_upgrade/info.c        | 12 +++++-------
 src/bin/pg_upgrade/option.c      |  4 ++--
 src/bin/pg_upgrade/pg_upgrade.c  | 21 ++++++++++-----------
 src/bin/pg_upgrade/pg_upgrade.h  | 13 +++++++------
 6 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f653fa25a5..251f3d9017 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,7 +29,7 @@ static void check_for_new_tablespace_dir(void);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static void check_new_cluster_logical_replication_slots(void);
 static void check_new_cluster_subscription_configuration(void);
-static void check_old_cluster_for_valid_slots(bool live_check);
+static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 
 /*
@@ -555,9 +555,9 @@ fix_path_separator(char *path)
 }
 
 void
-output_check_banner(bool live_check)
+output_check_banner(void)
 {
-	if (user_opts.check && live_check)
+	if (user_opts.live_check)
 	{
 		pg_log(PG_REPORT,
 			   "Performing Consistency Checks on Old Live Server\n"
@@ -573,18 +573,18 @@ output_check_banner(bool live_check)
 
 
 void
-check_and_dump_old_cluster(bool live_check)
+check_and_dump_old_cluster(void)
 {
 	/* -- OLD -- */
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		start_postmaster(&old_cluster, true);
 
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
 	 */
-	get_db_rel_and_slot_infos(&old_cluster, live_check);
+	get_db_rel_and_slot_infos(&old_cluster);
 
 	init_tablespaces();
 
@@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
 		 * Logical replication slots can be migrated since PG17. See comments
 		 * atop get_old_cluster_logical_slot_infos().
 		 */
-		check_old_cluster_for_valid_slots(live_check);
+		check_old_cluster_for_valid_slots();
 
 		/*
 		 * Subscriptions and their dependencies can be migrated since PG17.
@@ -652,7 +652,7 @@ check_and_dump_old_cluster(bool live_check)
 	 */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
 	{
-		if (user_opts.check)
+		if (user_opts.live_check)
 			old_9_6_invalidate_hash_indexes(&old_cluster, true);
 	}
 
@@ -667,7 +667,7 @@ check_and_dump_old_cluster(bool live_check)
 	if (!user_opts.check)
 		generate_old_dump();
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		stop_postmaster(false);
 }
 
@@ -675,7 +675,7 @@ check_and_dump_old_cluster(bool live_check)
 void
 check_new_cluster(void)
 {
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 
 	check_new_cluster_is_empty();
 
@@ -826,14 +826,14 @@ check_cluster_versions(void)
 
 
 void
-check_cluster_compatibility(bool live_check)
+check_cluster_compatibility(void)
 {
 	/* get/check pg_control data of servers */
-	get_control_data(&old_cluster, live_check);
-	get_control_data(&new_cluster, false);
+	get_control_data(&old_cluster);
+	get_control_data(&new_cluster);
 	check_control_data(&old_cluster.controldata, &new_cluster.controldata);
 
-	if (live_check && old_cluster.port == new_cluster.port)
+	if (user_opts.live_check && old_cluster.port == new_cluster.port)
 		pg_fatal("When checking a live server, "
 				 "the old and new port numbers must be different.");
 }
@@ -1839,7 +1839,7 @@ check_new_cluster_subscription_configuration(void)
  * before shutdown.
  */
 static void
-check_old_cluster_for_valid_slots(bool live_check)
+check_old_cluster_for_valid_slots(void)
 {
 	char		output_path[MAXPGPATH];
 	FILE	   *script = NULL;
@@ -1878,7 +1878,7 @@ check_old_cluster_for_valid_slots(bool live_check)
 			 * Note: This can be satisfied only when the old cluster has been
 			 * shut down, so we skip this for live checks.
 			 */
-			if (!live_check && !slot->caught_up)
+			if (!user_opts.live_check && !slot->caught_up)
 			{
 				if (script == NULL &&
 					(script = fopen_priv(output_path, "w")) == NULL)
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed..cf665b9dee 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -33,7 +33,7 @@
  * return valid xid data for a running server.
  */
 void
-get_control_data(ClusterInfo *cluster, bool live_check)
+get_control_data(ClusterInfo *cluster)
 {
 	char		cmd[MAXPGPATH];
 	char		bufin[MAX_STRING];
@@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	uint32		segno = 0;
 	char	   *resetwal_bin;
 	int			rc;
+	bool		live_check = (cluster == &old_cluster && user_opts.live_check);
 
 	/*
 	 * Because we test the pg_resetwal output as strings, it has to be in
@@ -118,7 +119,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	/*
 	 * Check for clean shutdown
 	 */
-	if (!live_check || cluster == &new_cluster)
+	if (!live_check)
 	{
 		/* only pg_controldata outputs the cluster state */
 		snprintf(cmd, sizeof(cmd), "\"%s/pg_controldata\" \"%s\"",
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 95c22a7200..8f1777de59 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
 static void print_db_infos(DbInfoArr *db_arr);
 static void print_rel_infos(RelInfoArr *rel_arr);
 static void print_slot_infos(LogicalSlotInfoArr *slot_arr);
-static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check);
+static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo);
 static void get_db_subscription_count(DbInfo *dbinfo);
 
 
@@ -273,11 +273,9 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
  *
  * higher level routine to generate dbinfos for the database running
  * on the given "port". Assumes that server is already running.
- *
- * live_check would be used only when the target is the old cluster.
  */
 void
-get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
+get_db_rel_and_slot_infos(ClusterInfo *cluster)
 {
 	int			dbnum;
 
@@ -299,7 +297,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
 		 */
 		if (cluster == &old_cluster)
 		{
-			get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+			get_old_cluster_logical_slot_infos(pDbInfo);
 			get_db_subscription_count(pDbInfo);
 		}
 	}
@@ -645,7 +643,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  * are included.
  */
 static void
-get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
+get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
 {
 	PGconn	   *conn;
 	PGresult   *res;
@@ -681,7 +679,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
 							"WHERE slot_type = 'logical' AND "
 							"database = current_database() AND "
 							"temporary IS FALSE;",
-							live_check ? "FALSE" :
+							user_opts.live_check ? "FALSE" :
 							"(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
 							"ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
 							"END)");
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548ea4e623..6f41d63eed 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
  * directory.
  */
 void
-get_sock_dir(ClusterInfo *cluster, bool live_check)
+get_sock_dir(ClusterInfo *cluster)
 {
 #if !defined(WIN32)
-	if (!live_check)
+	if (!user_opts.live_check || cluster == &new_cluster)
 		cluster->sockdir = user_opts.socketdir;
 	else
 	{
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index af370768b6..3f4ad7d5cc 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -65,7 +65,7 @@ static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
-static void setup(char *argv0, bool *live_check);
+static void setup(char *argv0);
 static void create_logical_replication_slots(void);
 
 ClusterInfo old_cluster,
@@ -88,7 +88,6 @@ int
 main(int argc, char **argv)
 {
 	char	   *deletion_script_file_name = NULL;
-	bool		live_check = false;
 
 	/*
 	 * pg_upgrade doesn't currently use common/logging.c, but initialize it
@@ -123,18 +122,18 @@ main(int argc, char **argv)
 	 */
 	make_outputdirs(new_cluster.pgdata);
 
-	setup(argv[0], &live_check);
+	setup(argv[0]);
 
-	output_check_banner(live_check);
+	output_check_banner();
 
 	check_cluster_versions();
 
-	get_sock_dir(&old_cluster, live_check);
-	get_sock_dir(&new_cluster, false);
+	get_sock_dir(&old_cluster);
+	get_sock_dir(&new_cluster);
 
-	check_cluster_compatibility(live_check);
+	check_cluster_compatibility();
 
-	check_and_dump_old_cluster(live_check);
+	check_and_dump_old_cluster();
 
 
 	/* -- NEW -- */
@@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
 
 
 static void
-setup(char *argv0, bool *live_check)
+setup(char *argv0)
 {
 	/*
 	 * make sure the user has a clean environment, otherwise, we may confuse
@@ -378,7 +377,7 @@ setup(char *argv0, bool *live_check)
 				pg_fatal("There seems to be a postmaster servicing the old cluster.\n"
 						 "Please shutdown that postmaster and try again.");
 			else
-				*live_check = true;
+				user_opts.live_check = true;
 		}
 	}
 
@@ -648,7 +647,7 @@ create_new_objects(void)
 		set_frozenxids(true);
 
 	/* update new_cluster info now that we have objects in the databases */
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 }
 
 /*
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 1ebad3bd74..56d05d7eb9 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -322,6 +322,7 @@ typedef struct
 typedef struct
 {
 	bool		check;			/* check clusters only, don't change any data */
+	bool		live_check;		/* check clusters only, old server is running */
 	bool		do_sync;		/* flush changes to disk */
 	transferMode transfer_mode; /* copy files or link them? */
 	int			jobs;			/* number of processes/threads to use */
@@ -366,20 +367,20 @@ extern OSInfo os_info;
 
 /* check.c */
 
-void		output_check_banner(bool live_check);
-void		check_and_dump_old_cluster(bool live_check);
+void		output_check_banner(void);
+void		check_and_dump_old_cluster(void);
 void		check_new_cluster(void);
 void		report_clusters_compatible(void);
 void		issue_warnings_and_set_wal_level(void);
 void		output_completion_banner(char *deletion_script_file_name);
 void		check_cluster_versions(void);
-void		check_cluster_compatibility(bool live_check);
+void		check_cluster_compatibility(void);
 void		create_script_for_old_cluster_deletion(char **deletion_script_file_name);
 
 
 /* controldata.c */
 
-void		get_control_data(ClusterInfo *cluster, bool live_check);
+void		get_control_data(ClusterInfo *cluster);
 void		check_control_data(ControlData *oldctrl, ControlData *newctrl);
 void		disable_old_cluster(void);
 
@@ -428,7 +429,7 @@ void		check_loadable_libraries(void);
 FileNameMap *gen_db_file_maps(DbInfo *old_db,
 							  DbInfo *new_db, int *nmaps, const char *old_pgdata,
 							  const char *new_pgdata);
-void		get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check);
+void		get_db_rel_and_slot_infos(ClusterInfo *cluster);
 int			count_old_cluster_logical_slots(void);
 int			count_old_cluster_subscriptions(void);
 
@@ -436,7 +437,7 @@ int			count_old_cluster_subscriptions(void);
 
 void		parseCommandLine(int argc, char *argv[]);
 void		adjust_data_dir(ClusterInfo *cluster);
-void		get_sock_dir(ClusterInfo *cluster, bool live_check);
+void		get_sock_dir(ClusterInfo *cluster);
 
 /* relfilenumber.c */
 
-- 
2.39.3 (Apple Git-146)


--GKlW06AwR1M8iOg7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0004-use-new-pg_upgrade-async-API-for-retrieving-relin.patch"



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

* [PATCH v2 3/6] move live_check variable to user_opts
@ 2024-05-21 21:35 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Nathan Bossart @ 2024-05-21 21:35 UTC (permalink / raw)

---
 src/bin/pg_upgrade/check.c       | 32 ++++++++++++++++----------------
 src/bin/pg_upgrade/controldata.c |  5 +++--
 src/bin/pg_upgrade/info.c        | 12 +++++-------
 src/bin/pg_upgrade/option.c      |  4 ++--
 src/bin/pg_upgrade/pg_upgrade.c  | 21 ++++++++++-----------
 src/bin/pg_upgrade/pg_upgrade.h  | 13 +++++++------
 6 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f653fa25a5..251f3d9017 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,7 +29,7 @@ static void check_for_new_tablespace_dir(void);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static void check_new_cluster_logical_replication_slots(void);
 static void check_new_cluster_subscription_configuration(void);
-static void check_old_cluster_for_valid_slots(bool live_check);
+static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 
 /*
@@ -555,9 +555,9 @@ fix_path_separator(char *path)
 }
 
 void
-output_check_banner(bool live_check)
+output_check_banner(void)
 {
-	if (user_opts.check && live_check)
+	if (user_opts.live_check)
 	{
 		pg_log(PG_REPORT,
 			   "Performing Consistency Checks on Old Live Server\n"
@@ -573,18 +573,18 @@ output_check_banner(bool live_check)
 
 
 void
-check_and_dump_old_cluster(bool live_check)
+check_and_dump_old_cluster(void)
 {
 	/* -- OLD -- */
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		start_postmaster(&old_cluster, true);
 
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
 	 */
-	get_db_rel_and_slot_infos(&old_cluster, live_check);
+	get_db_rel_and_slot_infos(&old_cluster);
 
 	init_tablespaces();
 
@@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
 		 * Logical replication slots can be migrated since PG17. See comments
 		 * atop get_old_cluster_logical_slot_infos().
 		 */
-		check_old_cluster_for_valid_slots(live_check);
+		check_old_cluster_for_valid_slots();
 
 		/*
 		 * Subscriptions and their dependencies can be migrated since PG17.
@@ -652,7 +652,7 @@ check_and_dump_old_cluster(bool live_check)
 	 */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
 	{
-		if (user_opts.check)
+		if (user_opts.live_check)
 			old_9_6_invalidate_hash_indexes(&old_cluster, true);
 	}
 
@@ -667,7 +667,7 @@ check_and_dump_old_cluster(bool live_check)
 	if (!user_opts.check)
 		generate_old_dump();
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		stop_postmaster(false);
 }
 
@@ -675,7 +675,7 @@ check_and_dump_old_cluster(bool live_check)
 void
 check_new_cluster(void)
 {
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 
 	check_new_cluster_is_empty();
 
@@ -826,14 +826,14 @@ check_cluster_versions(void)
 
 
 void
-check_cluster_compatibility(bool live_check)
+check_cluster_compatibility(void)
 {
 	/* get/check pg_control data of servers */
-	get_control_data(&old_cluster, live_check);
-	get_control_data(&new_cluster, false);
+	get_control_data(&old_cluster);
+	get_control_data(&new_cluster);
 	check_control_data(&old_cluster.controldata, &new_cluster.controldata);
 
-	if (live_check && old_cluster.port == new_cluster.port)
+	if (user_opts.live_check && old_cluster.port == new_cluster.port)
 		pg_fatal("When checking a live server, "
 				 "the old and new port numbers must be different.");
 }
@@ -1839,7 +1839,7 @@ check_new_cluster_subscription_configuration(void)
  * before shutdown.
  */
 static void
-check_old_cluster_for_valid_slots(bool live_check)
+check_old_cluster_for_valid_slots(void)
 {
 	char		output_path[MAXPGPATH];
 	FILE	   *script = NULL;
@@ -1878,7 +1878,7 @@ check_old_cluster_for_valid_slots(bool live_check)
 			 * Note: This can be satisfied only when the old cluster has been
 			 * shut down, so we skip this for live checks.
 			 */
-			if (!live_check && !slot->caught_up)
+			if (!user_opts.live_check && !slot->caught_up)
 			{
 				if (script == NULL &&
 					(script = fopen_priv(output_path, "w")) == NULL)
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed..cf665b9dee 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -33,7 +33,7 @@
  * return valid xid data for a running server.
  */
 void
-get_control_data(ClusterInfo *cluster, bool live_check)
+get_control_data(ClusterInfo *cluster)
 {
 	char		cmd[MAXPGPATH];
 	char		bufin[MAX_STRING];
@@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	uint32		segno = 0;
 	char	   *resetwal_bin;
 	int			rc;
+	bool		live_check = (cluster == &old_cluster && user_opts.live_check);
 
 	/*
 	 * Because we test the pg_resetwal output as strings, it has to be in
@@ -118,7 +119,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	/*
 	 * Check for clean shutdown
 	 */
-	if (!live_check || cluster == &new_cluster)
+	if (!live_check)
 	{
 		/* only pg_controldata outputs the cluster state */
 		snprintf(cmd, sizeof(cmd), "\"%s/pg_controldata\" \"%s\"",
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 95c22a7200..8f1777de59 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
 static void print_db_infos(DbInfoArr *db_arr);
 static void print_rel_infos(RelInfoArr *rel_arr);
 static void print_slot_infos(LogicalSlotInfoArr *slot_arr);
-static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check);
+static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo);
 static void get_db_subscription_count(DbInfo *dbinfo);
 
 
@@ -273,11 +273,9 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
  *
  * higher level routine to generate dbinfos for the database running
  * on the given "port". Assumes that server is already running.
- *
- * live_check would be used only when the target is the old cluster.
  */
 void
-get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
+get_db_rel_and_slot_infos(ClusterInfo *cluster)
 {
 	int			dbnum;
 
@@ -299,7 +297,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
 		 */
 		if (cluster == &old_cluster)
 		{
-			get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+			get_old_cluster_logical_slot_infos(pDbInfo);
 			get_db_subscription_count(pDbInfo);
 		}
 	}
@@ -645,7 +643,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  * are included.
  */
 static void
-get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
+get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
 {
 	PGconn	   *conn;
 	PGresult   *res;
@@ -681,7 +679,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
 							"WHERE slot_type = 'logical' AND "
 							"database = current_database() AND "
 							"temporary IS FALSE;",
-							live_check ? "FALSE" :
+							user_opts.live_check ? "FALSE" :
 							"(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
 							"ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
 							"END)");
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548ea4e623..6f41d63eed 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
  * directory.
  */
 void
-get_sock_dir(ClusterInfo *cluster, bool live_check)
+get_sock_dir(ClusterInfo *cluster)
 {
 #if !defined(WIN32)
-	if (!live_check)
+	if (!user_opts.live_check || cluster == &new_cluster)
 		cluster->sockdir = user_opts.socketdir;
 	else
 	{
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index af370768b6..3f4ad7d5cc 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -65,7 +65,7 @@ static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
-static void setup(char *argv0, bool *live_check);
+static void setup(char *argv0);
 static void create_logical_replication_slots(void);
 
 ClusterInfo old_cluster,
@@ -88,7 +88,6 @@ int
 main(int argc, char **argv)
 {
 	char	   *deletion_script_file_name = NULL;
-	bool		live_check = false;
 
 	/*
 	 * pg_upgrade doesn't currently use common/logging.c, but initialize it
@@ -123,18 +122,18 @@ main(int argc, char **argv)
 	 */
 	make_outputdirs(new_cluster.pgdata);
 
-	setup(argv[0], &live_check);
+	setup(argv[0]);
 
-	output_check_banner(live_check);
+	output_check_banner();
 
 	check_cluster_versions();
 
-	get_sock_dir(&old_cluster, live_check);
-	get_sock_dir(&new_cluster, false);
+	get_sock_dir(&old_cluster);
+	get_sock_dir(&new_cluster);
 
-	check_cluster_compatibility(live_check);
+	check_cluster_compatibility();
 
-	check_and_dump_old_cluster(live_check);
+	check_and_dump_old_cluster();
 
 
 	/* -- NEW -- */
@@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
 
 
 static void
-setup(char *argv0, bool *live_check)
+setup(char *argv0)
 {
 	/*
 	 * make sure the user has a clean environment, otherwise, we may confuse
@@ -378,7 +377,7 @@ setup(char *argv0, bool *live_check)
 				pg_fatal("There seems to be a postmaster servicing the old cluster.\n"
 						 "Please shutdown that postmaster and try again.");
 			else
-				*live_check = true;
+				user_opts.live_check = true;
 		}
 	}
 
@@ -648,7 +647,7 @@ create_new_objects(void)
 		set_frozenxids(true);
 
 	/* update new_cluster info now that we have objects in the databases */
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 }
 
 /*
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 1ebad3bd74..56d05d7eb9 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -322,6 +322,7 @@ typedef struct
 typedef struct
 {
 	bool		check;			/* check clusters only, don't change any data */
+	bool		live_check;		/* check clusters only, old server is running */
 	bool		do_sync;		/* flush changes to disk */
 	transferMode transfer_mode; /* copy files or link them? */
 	int			jobs;			/* number of processes/threads to use */
@@ -366,20 +367,20 @@ extern OSInfo os_info;
 
 /* check.c */
 
-void		output_check_banner(bool live_check);
-void		check_and_dump_old_cluster(bool live_check);
+void		output_check_banner(void);
+void		check_and_dump_old_cluster(void);
 void		check_new_cluster(void);
 void		report_clusters_compatible(void);
 void		issue_warnings_and_set_wal_level(void);
 void		output_completion_banner(char *deletion_script_file_name);
 void		check_cluster_versions(void);
-void		check_cluster_compatibility(bool live_check);
+void		check_cluster_compatibility(void);
 void		create_script_for_old_cluster_deletion(char **deletion_script_file_name);
 
 
 /* controldata.c */
 
-void		get_control_data(ClusterInfo *cluster, bool live_check);
+void		get_control_data(ClusterInfo *cluster);
 void		check_control_data(ControlData *oldctrl, ControlData *newctrl);
 void		disable_old_cluster(void);
 
@@ -428,7 +429,7 @@ void		check_loadable_libraries(void);
 FileNameMap *gen_db_file_maps(DbInfo *old_db,
 							  DbInfo *new_db, int *nmaps, const char *old_pgdata,
 							  const char *new_pgdata);
-void		get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check);
+void		get_db_rel_and_slot_infos(ClusterInfo *cluster);
 int			count_old_cluster_logical_slots(void);
 int			count_old_cluster_subscriptions(void);
 
@@ -436,7 +437,7 @@ int			count_old_cluster_subscriptions(void);
 
 void		parseCommandLine(int argc, char *argv[]);
 void		adjust_data_dir(ClusterInfo *cluster);
-void		get_sock_dir(ClusterInfo *cluster, bool live_check);
+void		get_sock_dir(ClusterInfo *cluster);
 
 /* relfilenumber.c */
 
-- 
2.39.3 (Apple Git-146)


--/P4mQ+NFehN7eyNA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0004-use-new-pg_upgrade-async-API-for-retrieving-relin.patch"



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

* [PATCH v4 03/12] move live_check variable to user_opts
@ 2024-05-21 21:35 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Nathan Bossart @ 2024-05-21 21:35 UTC (permalink / raw)

---
 src/bin/pg_upgrade/check.c       | 32 ++++++++++++++++----------------
 src/bin/pg_upgrade/controldata.c |  5 +++--
 src/bin/pg_upgrade/info.c        | 12 +++++-------
 src/bin/pg_upgrade/option.c      |  4 ++--
 src/bin/pg_upgrade/pg_upgrade.c  | 21 ++++++++++-----------
 src/bin/pg_upgrade/pg_upgrade.h  | 13 +++++++------
 6 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f653fa25a5..251f3d9017 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,7 +29,7 @@ static void check_for_new_tablespace_dir(void);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static void check_new_cluster_logical_replication_slots(void);
 static void check_new_cluster_subscription_configuration(void);
-static void check_old_cluster_for_valid_slots(bool live_check);
+static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 
 /*
@@ -555,9 +555,9 @@ fix_path_separator(char *path)
 }
 
 void
-output_check_banner(bool live_check)
+output_check_banner(void)
 {
-	if (user_opts.check && live_check)
+	if (user_opts.live_check)
 	{
 		pg_log(PG_REPORT,
 			   "Performing Consistency Checks on Old Live Server\n"
@@ -573,18 +573,18 @@ output_check_banner(bool live_check)
 
 
 void
-check_and_dump_old_cluster(bool live_check)
+check_and_dump_old_cluster(void)
 {
 	/* -- OLD -- */
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		start_postmaster(&old_cluster, true);
 
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
 	 */
-	get_db_rel_and_slot_infos(&old_cluster, live_check);
+	get_db_rel_and_slot_infos(&old_cluster);
 
 	init_tablespaces();
 
@@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
 		 * Logical replication slots can be migrated since PG17. See comments
 		 * atop get_old_cluster_logical_slot_infos().
 		 */
-		check_old_cluster_for_valid_slots(live_check);
+		check_old_cluster_for_valid_slots();
 
 		/*
 		 * Subscriptions and their dependencies can be migrated since PG17.
@@ -652,7 +652,7 @@ check_and_dump_old_cluster(bool live_check)
 	 */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
 	{
-		if (user_opts.check)
+		if (user_opts.live_check)
 			old_9_6_invalidate_hash_indexes(&old_cluster, true);
 	}
 
@@ -667,7 +667,7 @@ check_and_dump_old_cluster(bool live_check)
 	if (!user_opts.check)
 		generate_old_dump();
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		stop_postmaster(false);
 }
 
@@ -675,7 +675,7 @@ check_and_dump_old_cluster(bool live_check)
 void
 check_new_cluster(void)
 {
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 
 	check_new_cluster_is_empty();
 
@@ -826,14 +826,14 @@ check_cluster_versions(void)
 
 
 void
-check_cluster_compatibility(bool live_check)
+check_cluster_compatibility(void)
 {
 	/* get/check pg_control data of servers */
-	get_control_data(&old_cluster, live_check);
-	get_control_data(&new_cluster, false);
+	get_control_data(&old_cluster);
+	get_control_data(&new_cluster);
 	check_control_data(&old_cluster.controldata, &new_cluster.controldata);
 
-	if (live_check && old_cluster.port == new_cluster.port)
+	if (user_opts.live_check && old_cluster.port == new_cluster.port)
 		pg_fatal("When checking a live server, "
 				 "the old and new port numbers must be different.");
 }
@@ -1839,7 +1839,7 @@ check_new_cluster_subscription_configuration(void)
  * before shutdown.
  */
 static void
-check_old_cluster_for_valid_slots(bool live_check)
+check_old_cluster_for_valid_slots(void)
 {
 	char		output_path[MAXPGPATH];
 	FILE	   *script = NULL;
@@ -1878,7 +1878,7 @@ check_old_cluster_for_valid_slots(bool live_check)
 			 * Note: This can be satisfied only when the old cluster has been
 			 * shut down, so we skip this for live checks.
 			 */
-			if (!live_check && !slot->caught_up)
+			if (!user_opts.live_check && !slot->caught_up)
 			{
 				if (script == NULL &&
 					(script = fopen_priv(output_path, "w")) == NULL)
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed..cf665b9dee 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -33,7 +33,7 @@
  * return valid xid data for a running server.
  */
 void
-get_control_data(ClusterInfo *cluster, bool live_check)
+get_control_data(ClusterInfo *cluster)
 {
 	char		cmd[MAXPGPATH];
 	char		bufin[MAX_STRING];
@@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	uint32		segno = 0;
 	char	   *resetwal_bin;
 	int			rc;
+	bool		live_check = (cluster == &old_cluster && user_opts.live_check);
 
 	/*
 	 * Because we test the pg_resetwal output as strings, it has to be in
@@ -118,7 +119,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	/*
 	 * Check for clean shutdown
 	 */
-	if (!live_check || cluster == &new_cluster)
+	if (!live_check)
 	{
 		/* only pg_controldata outputs the cluster state */
 		snprintf(cmd, sizeof(cmd), "\"%s/pg_controldata\" \"%s\"",
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 95c22a7200..8f1777de59 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
 static void print_db_infos(DbInfoArr *db_arr);
 static void print_rel_infos(RelInfoArr *rel_arr);
 static void print_slot_infos(LogicalSlotInfoArr *slot_arr);
-static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check);
+static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo);
 static void get_db_subscription_count(DbInfo *dbinfo);
 
 
@@ -273,11 +273,9 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
  *
  * higher level routine to generate dbinfos for the database running
  * on the given "port". Assumes that server is already running.
- *
- * live_check would be used only when the target is the old cluster.
  */
 void
-get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
+get_db_rel_and_slot_infos(ClusterInfo *cluster)
 {
 	int			dbnum;
 
@@ -299,7 +297,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
 		 */
 		if (cluster == &old_cluster)
 		{
-			get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+			get_old_cluster_logical_slot_infos(pDbInfo);
 			get_db_subscription_count(pDbInfo);
 		}
 	}
@@ -645,7 +643,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  * are included.
  */
 static void
-get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
+get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
 {
 	PGconn	   *conn;
 	PGresult   *res;
@@ -681,7 +679,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
 							"WHERE slot_type = 'logical' AND "
 							"database = current_database() AND "
 							"temporary IS FALSE;",
-							live_check ? "FALSE" :
+							user_opts.live_check ? "FALSE" :
 							"(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
 							"ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
 							"END)");
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548ea4e623..6f41d63eed 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
  * directory.
  */
 void
-get_sock_dir(ClusterInfo *cluster, bool live_check)
+get_sock_dir(ClusterInfo *cluster)
 {
 #if !defined(WIN32)
-	if (!live_check)
+	if (!user_opts.live_check || cluster == &new_cluster)
 		cluster->sockdir = user_opts.socketdir;
 	else
 	{
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 03eb738fd7..99f3d4543e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -65,7 +65,7 @@ static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
-static void setup(char *argv0, bool *live_check);
+static void setup(char *argv0);
 static void create_logical_replication_slots(void);
 
 ClusterInfo old_cluster,
@@ -88,7 +88,6 @@ int
 main(int argc, char **argv)
 {
 	char	   *deletion_script_file_name = NULL;
-	bool		live_check = false;
 
 	/*
 	 * pg_upgrade doesn't currently use common/logging.c, but initialize it
@@ -123,18 +122,18 @@ main(int argc, char **argv)
 	 */
 	make_outputdirs(new_cluster.pgdata);
 
-	setup(argv[0], &live_check);
+	setup(argv[0]);
 
-	output_check_banner(live_check);
+	output_check_banner();
 
 	check_cluster_versions();
 
-	get_sock_dir(&old_cluster, live_check);
-	get_sock_dir(&new_cluster, false);
+	get_sock_dir(&old_cluster);
+	get_sock_dir(&new_cluster);
 
-	check_cluster_compatibility(live_check);
+	check_cluster_compatibility();
 
-	check_and_dump_old_cluster(live_check);
+	check_and_dump_old_cluster();
 
 
 	/* -- NEW -- */
@@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
 
 
 static void
-setup(char *argv0, bool *live_check)
+setup(char *argv0)
 {
 	/*
 	 * make sure the user has a clean environment, otherwise, we may confuse
@@ -378,7 +377,7 @@ setup(char *argv0, bool *live_check)
 				pg_fatal("There seems to be a postmaster servicing the old cluster.\n"
 						 "Please shutdown that postmaster and try again.");
 			else
-				*live_check = true;
+				user_opts.live_check = true;
 		}
 	}
 
@@ -660,7 +659,7 @@ create_new_objects(void)
 		set_frozenxids(true);
 
 	/* update new_cluster info now that we have objects in the databases */
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 }
 
 /*
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 1ebad3bd74..56d05d7eb9 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -322,6 +322,7 @@ typedef struct
 typedef struct
 {
 	bool		check;			/* check clusters only, don't change any data */
+	bool		live_check;		/* check clusters only, old server is running */
 	bool		do_sync;		/* flush changes to disk */
 	transferMode transfer_mode; /* copy files or link them? */
 	int			jobs;			/* number of processes/threads to use */
@@ -366,20 +367,20 @@ extern OSInfo os_info;
 
 /* check.c */
 
-void		output_check_banner(bool live_check);
-void		check_and_dump_old_cluster(bool live_check);
+void		output_check_banner(void);
+void		check_and_dump_old_cluster(void);
 void		check_new_cluster(void);
 void		report_clusters_compatible(void);
 void		issue_warnings_and_set_wal_level(void);
 void		output_completion_banner(char *deletion_script_file_name);
 void		check_cluster_versions(void);
-void		check_cluster_compatibility(bool live_check);
+void		check_cluster_compatibility(void);
 void		create_script_for_old_cluster_deletion(char **deletion_script_file_name);
 
 
 /* controldata.c */
 
-void		get_control_data(ClusterInfo *cluster, bool live_check);
+void		get_control_data(ClusterInfo *cluster);
 void		check_control_data(ControlData *oldctrl, ControlData *newctrl);
 void		disable_old_cluster(void);
 
@@ -428,7 +429,7 @@ void		check_loadable_libraries(void);
 FileNameMap *gen_db_file_maps(DbInfo *old_db,
 							  DbInfo *new_db, int *nmaps, const char *old_pgdata,
 							  const char *new_pgdata);
-void		get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check);
+void		get_db_rel_and_slot_infos(ClusterInfo *cluster);
 int			count_old_cluster_logical_slots(void);
 int			count_old_cluster_subscriptions(void);
 
@@ -436,7 +437,7 @@ int			count_old_cluster_subscriptions(void);
 
 void		parseCommandLine(int argc, char *argv[]);
 void		adjust_data_dir(ClusterInfo *cluster);
-void		get_sock_dir(ClusterInfo *cluster, bool live_check);
+void		get_sock_dir(ClusterInfo *cluster);
 
 /* relfilenumber.c */
 
-- 
2.39.3 (Apple Git-146)


--+rVtD9QSctUEvLG2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0004-use-new-pg_upgrade-async-API-for-retrieving-relin.patch"



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

* [PATCH v5 02/13] move live_check variable to user_opts
@ 2024-05-21 21:35 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Nathan Bossart @ 2024-05-21 21:35 UTC (permalink / raw)

---
 src/bin/pg_upgrade/check.c       | 32 ++++++++++++++++----------------
 src/bin/pg_upgrade/controldata.c |  5 +++--
 src/bin/pg_upgrade/info.c        | 12 +++++-------
 src/bin/pg_upgrade/option.c      |  4 ++--
 src/bin/pg_upgrade/pg_upgrade.c  | 21 ++++++++++-----------
 src/bin/pg_upgrade/pg_upgrade.h  | 13 +++++++------
 6 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 39d14b7b92..d8888f5053 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -29,7 +29,7 @@ static void check_for_new_tablespace_dir(void);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static void check_new_cluster_logical_replication_slots(void);
 static void check_new_cluster_subscription_configuration(void);
-static void check_old_cluster_for_valid_slots(bool live_check);
+static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 
 /*
@@ -555,9 +555,9 @@ fix_path_separator(char *path)
 }
 
 void
-output_check_banner(bool live_check)
+output_check_banner(void)
 {
-	if (user_opts.check && live_check)
+	if (user_opts.live_check)
 	{
 		pg_log(PG_REPORT,
 			   "Performing Consistency Checks on Old Live Server\n"
@@ -573,18 +573,18 @@ output_check_banner(bool live_check)
 
 
 void
-check_and_dump_old_cluster(bool live_check)
+check_and_dump_old_cluster(void)
 {
 	/* -- OLD -- */
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		start_postmaster(&old_cluster, true);
 
 	/*
 	 * Extract a list of databases, tables, and logical replication slots from
 	 * the old cluster.
 	 */
-	get_db_rel_and_slot_infos(&old_cluster, live_check);
+	get_db_rel_and_slot_infos(&old_cluster);
 
 	init_tablespaces();
 
@@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
 		 * Logical replication slots can be migrated since PG17. See comments
 		 * atop get_old_cluster_logical_slot_infos().
 		 */
-		check_old_cluster_for_valid_slots(live_check);
+		check_old_cluster_for_valid_slots();
 
 		/*
 		 * Subscriptions and their dependencies can be migrated since PG17.
@@ -652,7 +652,7 @@ check_and_dump_old_cluster(bool live_check)
 	 */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
 	{
-		if (user_opts.check)
+		if (user_opts.live_check)
 			old_9_6_invalidate_hash_indexes(&old_cluster, true);
 	}
 
@@ -667,7 +667,7 @@ check_and_dump_old_cluster(bool live_check)
 	if (!user_opts.check)
 		generate_old_dump();
 
-	if (!live_check)
+	if (!user_opts.live_check)
 		stop_postmaster(false);
 }
 
@@ -675,7 +675,7 @@ check_and_dump_old_cluster(bool live_check)
 void
 check_new_cluster(void)
 {
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 
 	check_new_cluster_is_empty();
 
@@ -826,14 +826,14 @@ check_cluster_versions(void)
 
 
 void
-check_cluster_compatibility(bool live_check)
+check_cluster_compatibility(void)
 {
 	/* get/check pg_control data of servers */
-	get_control_data(&old_cluster, live_check);
-	get_control_data(&new_cluster, false);
+	get_control_data(&old_cluster);
+	get_control_data(&new_cluster);
 	check_control_data(&old_cluster.controldata, &new_cluster.controldata);
 
-	if (live_check && old_cluster.port == new_cluster.port)
+	if (user_opts.live_check && old_cluster.port == new_cluster.port)
 		pg_fatal("When checking a live server, "
 				 "the old and new port numbers must be different.");
 }
@@ -1836,7 +1836,7 @@ check_new_cluster_subscription_configuration(void)
  * before shutdown.
  */
 static void
-check_old_cluster_for_valid_slots(bool live_check)
+check_old_cluster_for_valid_slots(void)
 {
 	char		output_path[MAXPGPATH];
 	FILE	   *script = NULL;
@@ -1875,7 +1875,7 @@ check_old_cluster_for_valid_slots(bool live_check)
 			 * Note: This can be satisfied only when the old cluster has been
 			 * shut down, so we skip this for live checks.
 			 */
-			if (!live_check && !slot->caught_up)
+			if (!user_opts.live_check && !slot->caught_up)
 			{
 				if (script == NULL &&
 					(script = fopen_priv(output_path, "w")) == NULL)
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed..cf665b9dee 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -33,7 +33,7 @@
  * return valid xid data for a running server.
  */
 void
-get_control_data(ClusterInfo *cluster, bool live_check)
+get_control_data(ClusterInfo *cluster)
 {
 	char		cmd[MAXPGPATH];
 	char		bufin[MAX_STRING];
@@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	uint32		segno = 0;
 	char	   *resetwal_bin;
 	int			rc;
+	bool		live_check = (cluster == &old_cluster && user_opts.live_check);
 
 	/*
 	 * Because we test the pg_resetwal output as strings, it has to be in
@@ -118,7 +119,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	/*
 	 * Check for clean shutdown
 	 */
-	if (!live_check || cluster == &new_cluster)
+	if (!live_check)
 	{
 		/* only pg_controldata outputs the cluster state */
 		snprintf(cmd, sizeof(cmd), "\"%s/pg_controldata\" \"%s\"",
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index e43be79aa5..fc94de9ea4 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
 static void print_db_infos(DbInfoArr *db_arr);
 static void print_rel_infos(RelInfoArr *rel_arr);
 static void print_slot_infos(LogicalSlotInfoArr *slot_arr);
-static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check);
+static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo);
 static void get_subscription_count(ClusterInfo *cluster);
 
 
@@ -273,11 +273,9 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
  *
  * higher level routine to generate dbinfos for the database running
  * on the given "port". Assumes that server is already running.
- *
- * live_check would be used only when the target is the old cluster.
  */
 void
-get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
+get_db_rel_and_slot_infos(ClusterInfo *cluster)
 {
 	int			dbnum;
 
@@ -294,7 +292,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
 		get_rel_infos(cluster, pDbInfo);
 
 		if (cluster == &old_cluster)
-			get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+			get_old_cluster_logical_slot_infos(pDbInfo);
 	}
 
 	if (cluster == &old_cluster)
@@ -641,7 +639,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  * are included.
  */
 static void
-get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
+get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
 {
 	PGconn	   *conn;
 	PGresult   *res;
@@ -677,7 +675,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
 							"WHERE slot_type = 'logical' AND "
 							"database = current_database() AND "
 							"temporary IS FALSE;",
-							live_check ? "FALSE" :
+							user_opts.live_check ? "FALSE" :
 							"(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
 							"ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
 							"END)");
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548ea4e623..6f41d63eed 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
  * directory.
  */
 void
-get_sock_dir(ClusterInfo *cluster, bool live_check)
+get_sock_dir(ClusterInfo *cluster)
 {
 #if !defined(WIN32)
-	if (!live_check)
+	if (!user_opts.live_check || cluster == &new_cluster)
 		cluster->sockdir = user_opts.socketdir;
 	else
 	{
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 03eb738fd7..99f3d4543e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -65,7 +65,7 @@ static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
-static void setup(char *argv0, bool *live_check);
+static void setup(char *argv0);
 static void create_logical_replication_slots(void);
 
 ClusterInfo old_cluster,
@@ -88,7 +88,6 @@ int
 main(int argc, char **argv)
 {
 	char	   *deletion_script_file_name = NULL;
-	bool		live_check = false;
 
 	/*
 	 * pg_upgrade doesn't currently use common/logging.c, but initialize it
@@ -123,18 +122,18 @@ main(int argc, char **argv)
 	 */
 	make_outputdirs(new_cluster.pgdata);
 
-	setup(argv[0], &live_check);
+	setup(argv[0]);
 
-	output_check_banner(live_check);
+	output_check_banner();
 
 	check_cluster_versions();
 
-	get_sock_dir(&old_cluster, live_check);
-	get_sock_dir(&new_cluster, false);
+	get_sock_dir(&old_cluster);
+	get_sock_dir(&new_cluster);
 
-	check_cluster_compatibility(live_check);
+	check_cluster_compatibility();
 
-	check_and_dump_old_cluster(live_check);
+	check_and_dump_old_cluster();
 
 
 	/* -- NEW -- */
@@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
 
 
 static void
-setup(char *argv0, bool *live_check)
+setup(char *argv0)
 {
 	/*
 	 * make sure the user has a clean environment, otherwise, we may confuse
@@ -378,7 +377,7 @@ setup(char *argv0, bool *live_check)
 				pg_fatal("There seems to be a postmaster servicing the old cluster.\n"
 						 "Please shutdown that postmaster and try again.");
 			else
-				*live_check = true;
+				user_opts.live_check = true;
 		}
 	}
 
@@ -660,7 +659,7 @@ create_new_objects(void)
 		set_frozenxids(true);
 
 	/* update new_cluster info now that we have objects in the databases */
-	get_db_rel_and_slot_infos(&new_cluster, false);
+	get_db_rel_and_slot_infos(&new_cluster);
 }
 
 /*
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index e6dbbe6a93..08b2ad26d7 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -322,6 +322,7 @@ typedef struct
 typedef struct
 {
 	bool		check;			/* check clusters only, don't change any data */
+	bool		live_check;		/* check clusters only, old server is running */
 	bool		do_sync;		/* flush changes to disk */
 	transferMode transfer_mode; /* copy files or link them? */
 	int			jobs;			/* number of processes/threads to use */
@@ -366,20 +367,20 @@ extern OSInfo os_info;
 
 /* check.c */
 
-void		output_check_banner(bool live_check);
-void		check_and_dump_old_cluster(bool live_check);
+void		output_check_banner(void);
+void		check_and_dump_old_cluster(void);
 void		check_new_cluster(void);
 void		report_clusters_compatible(void);
 void		issue_warnings_and_set_wal_level(void);
 void		output_completion_banner(char *deletion_script_file_name);
 void		check_cluster_versions(void);
-void		check_cluster_compatibility(bool live_check);
+void		check_cluster_compatibility(void);
 void		create_script_for_old_cluster_deletion(char **deletion_script_file_name);
 
 
 /* controldata.c */
 
-void		get_control_data(ClusterInfo *cluster, bool live_check);
+void		get_control_data(ClusterInfo *cluster);
 void		check_control_data(ControlData *oldctrl, ControlData *newctrl);
 void		disable_old_cluster(void);
 
@@ -428,14 +429,14 @@ void		check_loadable_libraries(void);
 FileNameMap *gen_db_file_maps(DbInfo *old_db,
 							  DbInfo *new_db, int *nmaps, const char *old_pgdata,
 							  const char *new_pgdata);
-void		get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check);
+void		get_db_rel_and_slot_infos(ClusterInfo *cluster);
 int			count_old_cluster_logical_slots(void);
 
 /* option.c */
 
 void		parseCommandLine(int argc, char *argv[]);
 void		adjust_data_dir(ClusterInfo *cluster);
-void		get_sock_dir(ClusterInfo *cluster, bool live_check);
+void		get_sock_dir(ClusterInfo *cluster);
 
 /* relfilenumber.c */
 
-- 
2.39.3 (Apple Git-146)


--378n8yA1gW08N3vi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0003-introduce-framework-for-parallelizing-pg_upgrade-.patch"



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


end of thread, other threads:[~2024-05-21 21:35 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04 15:41 [PATCH v33 5/5] Filling gaps in jsonb arrays Dmitrii Dolgov <[email protected]>
2024-05-21 21:35 [PATCH v3 3/7] move live_check variable to user_opts Nathan Bossart <[email protected]>
2024-05-21 21:35 [PATCH v2 3/6] move live_check variable to user_opts Nathan Bossart <[email protected]>
2024-05-21 21:35 [PATCH v4 03/12] move live_check variable to user_opts Nathan Bossart <[email protected]>
2024-05-21 21:35 [PATCH v5 02/13] move live_check variable to user_opts Nathan Bossart <[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