database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00
Code Activity

Improve style of pg_upgrade task callback functions.

Browse Source 
I wanted to avoid adjusting this code too much when converting
these tasks to use the new parallelization framework (see commit
40e2e5e92b), which is why this is being done as a follow-up commit.
These stylistic adjustments result in fewer lines of code and fewer
levels of indentation in some places.

While at it, add names to the UpgradeTaskSlotState enum and the
UpgradeTaskSlot struct.  I'm not aware of any established project
policy in this area, but let's at least be consistent within the
same file.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/ZunW7XHLd2uTts4f%40nathan
This commit is contained in:
Nathan Bossart
2024-09-26 13:54:37 -05:00
parent 147bbc90f7
commit 9726653185
3 changed files with 107 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

222
src/bin/pg_upgrade/check.c
View File

@ -320,7 +320,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
struct data_type_check_state
{
DataTypesUsageChecks *check; /* the check for this step */
bool *result; /* true if check failed for any database */
bool result; /* true if check failed for any database */
PQExpBuffer *report; /* buffer for report on failed checks */
};
@ -390,69 +390,54 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void *arg)
{
struct data_type_check_state *state = (struct data_type_check_state *) arg;
int ntups = PQntuples(res);
char output_path[MAXPGPATH];
int i_nspname = PQfnumber(res, "nspname");
int i_relname = PQfnumber(res, "relname");
int i_attname = PQfnumber(res, "attname");
FILE *script = NULL;
AssertVariableIsOfType(&process_data_type_check, UpgradeTaskProcessCB);
if (ntups)
if (ntups == 0)
return;
snprintf(output_path, sizeof(output_path), "%s/%s",
log_opts.basedir,
state->check->report_filename);
/*
* Make sure we have a buffer to save reports to now that we found a first
* failing check.
*/
if (*state->report == NULL)
*state->report = createPQExpBuffer();
/*
* If this is the first time we see an error for the check in question
* then print a status message of the failure.
*/
if (!state->result)
{
char output_path[MAXPGPATH];
int i_nspname;
int i_relname;
int i_attname;
FILE *script = NULL;
bool db_used = false;
snprintf(output_path, sizeof(output_path), "%s/%s",
log_opts.basedir,
state->check->report_filename);
/*
* Make sure we have a buffer to save reports to now that we found a
* first failing check.
*/
if (*state->report == NULL)
*state->report = createPQExpBuffer();
/*
* If this is the first time we see an error for the check in question
* then print a status message of the failure.
*/
if (!(*state->result))
{
pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
appendPQExpBuffer(*state->report, "\n%s\n%s %s\n",
_(state->check->report_text),
_("A list of the problem columns is in the file:"),
output_path);
}
*state->result = true;
i_nspname = PQfnumber(res, "nspname");
i_relname = PQfnumber(res, "relname");
i_attname = PQfnumber(res, "attname");
for (int rowno = 0; rowno < ntups; rowno++)
{
if (script == NULL && (script = fopen_priv(output_path, "a")) == NULL)
pg_fatal("could not open file \"%s\": %m", output_path);
if (!db_used)
{
fprintf(script, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
fprintf(script, " %s.%s.%s\n",
PQgetvalue(res, rowno, i_nspname),
PQgetvalue(res, rowno, i_relname),
PQgetvalue(res, rowno, i_attname));
}
if (script)
{
fclose(script);
script = NULL;
}
pg_log(PG_REPORT, "failed check: %s", _(state->check->status));
appendPQExpBuffer(*state->report, "\n%s\n%s %s\n",
_(state->check->report_text),
_("A list of the problem columns is in the file:"),
output_path);
}
state->result = true;
if ((script = fopen_priv(output_path, "a")) == NULL)
pg_fatal("could not open file \"%s\": %m", output_path);
fprintf(script, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
fprintf(script, " %s.%s.%s\n",
PQgetvalue(res, rowno, i_nspname),
PQgetvalue(res, rowno, i_relname),
PQgetvalue(res, rowno, i_attname));
fclose(script);
}
/*
@ -477,7 +462,6 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void *arg)
static void
check_for_data_types_usage(ClusterInfo *cluster)
{
bool *results;
PQExpBuffer report = NULL;
DataTypesUsageChecks *tmp = data_types_usage_checks;
int n_data_types_usage_checks = 0;
@ -494,8 +478,7 @@ check_for_data_types_usage(ClusterInfo *cluster)
tmp++;
}
/* Prepare an array to store the results of checks in */
results = pg_malloc0(sizeof(bool) * n_data_types_usage_checks);
/* Allocate memory for queries and for task states */
queries = pg_malloc0(sizeof(char *) * n_data_types_usage_checks);
states = pg_malloc0(sizeof(struct data_type_check_state) * n_data_types_usage_checks);
@ -525,7 +508,6 @@ check_for_data_types_usage(ClusterInfo *cluster)
queries[i] = data_type_check_query(i);
states[i].check = check;
states[i].result = &results[i];
states[i].report = &report;
upgrade_task_add_step(task, queries[i], process_data_type_check,
@ -545,7 +527,6 @@ check_for_data_types_usage(ClusterInfo *cluster)
destroyPQExpBuffer(report);
}
pg_free(results);
for (int i = 0; i < n_data_types_usage_checks; i++)
{
if (queries[i])
@ -1234,7 +1215,6 @@ check_for_prepared_transactions(ClusterInfo *cluster)
static void
process_isn_and_int8_passing_mismatch(DbInfo *dbinfo, PGresult *res, void *arg)
{
bool db_used = false;
int ntups = PQntuples(res);
int i_nspname = PQfnumber(res, "nspname");
int i_proname = PQfnumber(res, "proname");
@ -1243,20 +1223,19 @@ process_isn_and_int8_passing_mismatch(DbInfo *dbinfo, PGresult *res, void *arg)
AssertVariableIsOfType(&process_isn_and_int8_passing_mismatch,
UpgradeTaskProcessCB);
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
fprintf(report->file, " %s.%s\n",
PQgetvalue(res, rowno, i_nspname),
PQgetvalue(res, rowno, i_proname));
}
}
/*
@ -1324,7 +1303,6 @@ process_user_defined_postfix_ops(DbInfo *dbinfo, PGresult *res, void *arg)
{
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
int ntups = PQntuples(res);
bool db_used = false;
int i_oproid = PQfnumber(res, "oproid");
int i_oprnsp = PQfnumber(res, "oprnsp");
int i_oprname = PQfnumber(res, "oprname");
@ -1334,26 +1312,22 @@ process_user_defined_postfix_ops(DbInfo *dbinfo, PGresult *res, void *arg)
AssertVariableIsOfType(&process_user_defined_postfix_ops,
UpgradeTaskProcessCB);
if (!ntups)
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
fprintf(report->file, " (oid=%s) %s.%s (%s.%s, NONE)\n",
PQgetvalue(res, rowno, i_oproid),
PQgetvalue(res, rowno, i_oprnsp),
PQgetvalue(res, rowno, i_oprname),
PQgetvalue(res, rowno, i_typnsp),
PQgetvalue(res, rowno, i_typname));
}
}
/*
@ -1422,7 +1396,6 @@ static void
process_incompat_polymorphics(DbInfo *dbinfo, PGresult *res, void *arg)
{
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
bool db_used = false;
int ntups = PQntuples(res);
int i_objkind = PQfnumber(res, "objkind");
int i_objname = PQfnumber(res, "objname");
@ -1430,21 +1403,19 @@ process_incompat_polymorphics(DbInfo *dbinfo, PGresult *res, void *arg)
AssertVariableIsOfType(&process_incompat_polymorphics,
UpgradeTaskProcessCB);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
fprintf(report->file, " %s: %s\n",
PQgetvalue(res, rowno, i_objkind),
PQgetvalue(res, rowno, i_objname));
}
}
/*
@ -1558,30 +1529,25 @@ static void
process_with_oids_check(DbInfo *dbinfo, PGresult *res, void *arg)
{
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
bool db_used = false;
int ntups = PQntuples(res);
int i_nspname = PQfnumber(res, "nspname");
int i_relname = PQfnumber(res, "relname");
AssertVariableIsOfType(&process_with_oids_check, UpgradeTaskProcessCB);
if (!ntups)
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
fprintf(report->file, " %s.%s\n",
PQgetvalue(res, rowno, i_nspname),
PQgetvalue(res, rowno, i_relname));
}
}
/*
@ -1693,7 +1659,6 @@ static void
process_user_defined_encoding_conversions(DbInfo *dbinfo, PGresult *res, void *arg)
{
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
bool db_used = false;
int ntups = PQntuples(res);
int i_conoid = PQfnumber(res, "conoid");
int i_conname = PQfnumber(res, "conname");
@ -1702,24 +1667,20 @@ process_user_defined_encoding_conversions(DbInfo *dbinfo, PGresult *res, void *a
AssertVariableIsOfType(&process_user_defined_encoding_conversions,
UpgradeTaskProcessCB);
if (!ntups)
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
fprintf(report->file, "In database: %s\n", dbinfo->db_name);
db_used = true;
}
fprintf(report->file, " (oid=%s) %s.%s\n",
PQgetvalue(res, rowno, i_conoid),
PQgetvalue(res, rowno, i_nspname),
PQgetvalue(res, rowno, i_conname));
}
}
/*
@ -1971,7 +1932,7 @@ static void
process_old_sub_state_check(DbInfo *dbinfo, PGresult *res, void *arg)
{
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
int ntup = PQntuples(res);
int ntups = PQntuples(res);
int i_srsubstate = PQfnumber(res, "srsubstate");
int i_subname = PQfnumber(res, "subname");
int i_nspname = PQfnumber(res, "nspname");
@ -1979,19 +1940,20 @@ process_old_sub_state_check(DbInfo *dbinfo, PGresult *res, void *arg)
AssertVariableIsOfType(&process_old_sub_state_check, UpgradeTaskProcessCB);
for (int i = 0; i < ntup; i++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (ntups == 0)
return;
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
for (int i = 0; i < ntups; i++)
fprintf(report->file, "The table sync state \"%s\" is not allowed for database:\"%s\" subscription:\"%s\" schema:\"%s\" relation:\"%s\"\n",
PQgetvalue(res, i, i_srsubstate),
dbinfo->db_name,
PQgetvalue(res, i, i_subname),
PQgetvalue(res, i, i_nspname),
PQgetvalue(res, i, i_relname));
}
}
/*

4
src/bin/pg_upgrade/task.c
View File

@ -89,7 +89,7 @@ struct UpgradeTask
/*
* The different states for a parallel slot.
*/
typedef enum
typedef enum UpgradeTaskSlotState
{
FREE, /* slot available for use in a new database */
CONNECTING, /* waiting for connection to be established */
@ -99,7 +99,7 @@ typedef enum
/*
* We maintain an array of user_opts.jobs slots to execute the task.
*/
typedef struct
typedef struct UpgradeTaskSlot
{
UpgradeTaskSlotState state; /* state of the slot */
int db_idx; /* index of the database assigned to slot */

29
src/bin/pg_upgrade/version.c
View File

@ -147,31 +147,28 @@ old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode)
static void
process_extension_updates(DbInfo *dbinfo, PGresult *res, void *arg)
{
bool db_used = false;
int ntups = PQntuples(res);
int i_name = PQfnumber(res, "name");
UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
PQExpBufferData connectbuf;
AssertVariableIsOfType(&process_extension_updates, UpgradeTaskProcessCB);
for (int rowno = 0; rowno < ntups; rowno++)
{
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
if (!db_used)
{
PQExpBufferData connectbuf;
if (ntups == 0)
return;
initPQExpBuffer(&connectbuf);
appendPsqlMetaConnect(&connectbuf, dbinfo->db_name);
fputs(connectbuf.data, report->file);
termPQExpBuffer(&connectbuf);
db_used = true;
}
if (report->file == NULL &&
(report->file = fopen_priv(report->path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", report->path);
initPQExpBuffer(&connectbuf);
appendPsqlMetaConnect(&connectbuf, dbinfo->db_name);
fputs(connectbuf.data, report->file);
termPQExpBuffer(&connectbuf);
for (int rowno = 0; rowno < ntups; rowno++)
fprintf(report->file, "ALTER EXTENSION %s UPDATE;\n",
quote_identifier(PQgetvalue(res, rowno, i_name)));
}
}
/*