Two more minor things on this:
I found it a bit silly to mention the ALTER command in the error message
and then output only the three-part-qualified name in the output file
(and no specific command to run) -- so for an instant I was tempted to
write the full ALTER TABLE command in the output file instead. However
I then noticed that check.c does not use fmtId() or fmtQualifiedId()
anywhere, so that code would look out of place in check.c and perhaps
even be a problem for backpatching. Do people thing it'd be an
improvement to change it that way?
On 2017-Dec-13, Justin Pryzby wrote:
> Here are some spelling and grammar fixes to that patch:
>
> but nullabe in its children: nullable
> child column is not market: marked
> with adding: by adding
> and restart: and restarting
> the problem columns: the problematic columns
> 9.5, 9.6, 10: 9.5, 9.6, and 10
> restore, that will cause error.: restore phase of pg_upgrade, that would
> cause an error.
So the phrase "the problem columns" appears in multiple places in
pg_upgrade/check.c. Should we fix them all? It turns out that I
rewrote the message as submitted into completely different wording
(without realizing that this had been reported), so these words don't
appear anymore in the new commit. But we could fix these old ones while
we're here -- yes? (See below)
My rewrite is this:
pg_fatal("Your installation contains inconsistent NOT NULL
constraints.\n"
"If the parent column(s) are NOT NULL, then the child column
must\n"
"also be marked NOT NULL, or the upgrade will fail.\n"
"You can fix this by running\n"
" ALTER TABLE tablename ALTER column SET NOT NULL;\n"
"on each column listed in the file:\n"
" %s", report.path);
Here's what a patch looks like to change "problem columns" to
"problematic columns".
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 2106869b999..ad80f00c055 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -119,7 +119,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains system-defined
composite types in user tables.\n"
"These type OIDs are not stable across
PostgreSQL versions,\n"
"so this cluster cannot currently be
upgraded. You can drop the\n"
- "problem columns and restart the
upgrade.\n"),
+ "problematic columns and restart the
upgrade.\n"),
.threshold_version = ALL_VERSIONS
},
@@ -139,7 +139,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
"This data type changed its internal
and input/output format\n"
"between your old and new versions so
this\n"
"cluster cannot currently be upgraded.
You can\n"
- "drop the problem columns and restart
the upgrade.\n"),
+ "drop the problematic columns and
restart the upgrade.\n"),
.threshold_version = 903
},
@@ -183,7 +183,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains one of the reg* data
types in user tables.\n"
"These data types reference system
OIDs that are not preserved by\n"
"pg_upgrade, so this cluster cannot
currently be upgraded. You can\n"
- "drop the problem columns and restart
the upgrade.\n"),
+ "drop the problematic columns and
restart the upgrade.\n"),
.threshold_version = ALL_VERSIONS
},
@@ -200,7 +200,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains the \"aclitem\" data
type in user tables.\n"
"The internal format of \"aclitem\"
changed in PostgreSQL version 16\n"
"so this cluster cannot currently be
upgraded. You can drop the\n"
- "problem columns and restart the
upgrade.\n"),
+ "problematic columns and restart the
upgrade.\n"),
.threshold_version = 1500
},
@@ -223,7 +223,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
.report_text =
gettext_noop("Your installation contains the \"unknown\" data
type in user tables.\n"
"This data type is no longer allowed
in tables, so this cluster\n"
- "cannot currently be upgraded. You
can drop the problem columns\n"
+ "cannot currently be upgraded. You
can drop the problematic columns\n"
"and restart the upgrade.\n"),
.threshold_version = 906
},
@@ -279,7 +279,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains the \"abstime\" data
type in user tables.\n"
"The \"abstime\" type has been removed
in PostgreSQL version 12,\n"
"so this cluster cannot currently be
upgraded. You can drop the\n"
- "problem columns, or change them to
another data type, and restart\n"
+ "problematic columns, or change them
to another data type, and restart\n"
"the upgrade.\n"),
.threshold_version = 1100
},
@@ -292,7 +292,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains the \"reltime\" data
type in user tables.\n"
"The \"reltime\" type has been removed
in PostgreSQL version 12,\n"
"so this cluster cannot currently be
upgraded. You can drop the\n"
- "problem columns, or change them to
another data type, and restart\n"
+ "problematic columns, or change them
to another data type, and restart\n"
"the upgrade.\n"),
.threshold_version = 1100
},
@@ -305,7 +305,7 @@ static DataTypesUsageChecks data_types_usage_checks[] =
gettext_noop("Your installation contains the \"tinterval\" data
type in user tables.\n"
"The \"tinterval\" type has been
removed in PostgreSQL version 12,\n"
"so this cluster cannot currently be
upgraded. You can drop the\n"
- "problem columns, or change them to
another data type, and restart\n"
+ "problematic columns, or change them
to another data type, and restart\n"
"the upgrade.\n"),
.threshold_version = 1100
},
@@ -423,7 +423,7 @@ process_data_type_check(DbInfo *dbinfo, PGresult *res, void
*arg)
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:"),
+ _("A list of the problematic
columns is in the file:"),
output_path);
}
state->result = true;
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Ed is the standard text editor."
http://groups.google.com/group/alt.religion.emacs/msg/8d94ddab6a9b0ad3
