Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 54a2330

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix some more omissions in pg_upgrade's tests for non-upgradable types.
Commits 29aeda6 et al closed up some oversights involving not checking for non-upgradable types within container types, such as arrays and ranges. However, I only looked at version.c, failing to notice that there were substantially-equivalent tests in check.c. (The division of responsibility between those files is less than clear...) In addition, because genbki.pl does not guarantee that auto-generated rowtype OIDs will hold still across versions, we need to consider that the composite type associated with a system catalog or view is non-upgradable. It seems unlikely that someone would have a user column declared that way, but if they did, trying to read it in another PG version would likely draw "no such pg_type OID" failures, thanks to the type OID embedded in composite Datums. To support the composite and reg*-type cases, extend the recursive query that does the search to allow any base query that returns a column of pg_type OIDs, rather than limiting it to exactly one starting type. As before, back-patch to all supported branches. Discussion: https://postgr.es/m/2798740.1619622555@sss.pgh.pa.us
1 parent f6171e6 commit 54a2330
Copy full SHA for 54a2330

File tree

Expand file treeCollapse file tree

3 files changed

+133
-148
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+133
-148
lines changed

‎src/bin/pg_upgrade/check.c

Copy file name to clipboardExpand all lines: src/bin/pg_upgrade/check.c
+84-138Lines changed: 84 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ static void check_is_install_user(ClusterInfo *cluster);
2323
static void check_proper_datallowconn(ClusterInfo *cluster);
2424
static void check_for_prepared_transactions(ClusterInfo *cluster);
2525
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
26+
static void check_for_composite_data_type_usage(ClusterInfo *cluster);
2627
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
2728
static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
2829
static void check_for_pg_role_prefix(ClusterInfo *cluster);
@@ -97,6 +98,7 @@ check_and_dump_old_cluster(bool live_check)
9798
check_is_install_user(&old_cluster);
9899
check_proper_datallowconn(&old_cluster);
99100
check_for_prepared_transactions(&old_cluster);
101+
check_for_composite_data_type_usage(&old_cluster);
100102
check_for_reg_data_type_usage(&old_cluster);
101103
check_for_isn_and_int8_passing_mismatch(&old_cluster);
102104

@@ -887,6 +889,63 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
887889
}
888890

889891

892+
/*
893+
* check_for_composite_data_type_usage()
894+
* Check for system-defined composite types used in user tables.
895+
*
896+
* The OIDs of rowtypes of system catalogs and information_schema views
897+
* can change across major versions; unlike user-defined types, we have
898+
* no mechanism for forcing them to be the same in the new cluster.
899+
* Hence, if any user table uses one, that's problematic for pg_upgrade.
900+
*/
901+
static void
902+
check_for_composite_data_type_usage(ClusterInfo *cluster)
903+
{
904+
bool found;
905+
Oid firstUserOid;
906+
char output_path[MAXPGPATH];
907+
char *base_query;
908+
909+
prep_status("Checking for system-defined composite types in user tables");
910+
911+
snprintf(output_path, sizeof(output_path), "tables_using_composite.txt");
912+
913+
/*
914+
* Look for composite types that were made during initdb *or* belong to
915+
* information_schema; that's important in case information_schema was
916+
* dropped and reloaded.
917+
*
918+
* The cutoff OID here should match the source cluster's value of
919+
* FirstNormalObjectId. We hardcode it rather than using that C #define
920+
* because, if that #define is ever changed, our own version's value is
921+
* NOT what to use. Eventually we may need a test on the source cluster's
922+
* version to select the correct value.
923+
*/
924+
firstUserOid = 16384;
925+
926+
base_query = psprintf("SELECT t.oid FROM pg_catalog.pg_type t "
927+
"LEFT JOIN pg_catalog.pg_namespace n ON t.typnamespace = n.oid "
928+
" WHERE typtype = 'c' AND (t.oid < %u OR nspname = 'information_schema')",
929+
firstUserOid);
930+
931+
found = check_for_data_types_usage(cluster, base_query, output_path);
932+
933+
free(base_query);
934+
935+
if (found)
936+
{
937+
pg_log(PG_REPORT, "fatal\n");
938+
pg_fatal("Your installation contains system-defined composite type(s) in user tables.\n"
939+
"These type OIDs are not stable across PostgreSQL versions,\n"
940+
"so this cluster cannot currently be upgraded. You can\n"
941+
"drop the problem columns and restart the upgrade.\n"
942+
"A list of the problem columns is in the file:\n"
943+
" %s\n\n", output_path);
944+
}
945+
else
946+
check_ok();
947+
}
948+
890949
/*
891950
* check_for_reg_data_type_usage()
892951
* pg_upgrade only preserves these system values:
@@ -901,87 +960,36 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
901960
static void
902961
check_for_reg_data_type_usage(ClusterInfo *cluster)
903962
{
904-
int dbnum;
905-
FILE *script = NULL;
906-
bool found = false;
963+
bool found;
907964
char output_path[MAXPGPATH];
908965

909966
prep_status("Checking for reg* system OID user data types");
910967

911968
snprintf(output_path, sizeof(output_path), "tables_using_reg.txt");
912969

913-
for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
914-
{
915-
PGresult *res;
916-
bool db_used = false;
917-
int ntups;
918-
int rowno;
919-
int i_nspname,
920-
i_relname,
921-
i_attname;
922-
DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
923-
PGconn *conn = connectToServer(cluster, active_db->db_name);
924-
925-
/*
926-
* While several relkinds don't store any data, e.g. views, they can
927-
* be used to define data types of other columns, so we check all
928-
* relkinds.
929-
*/
930-
res = executeQueryOrDie(conn,
931-
"SELECT n.nspname, c.relname, a.attname "
932-
"FROM pg_catalog.pg_class c, "
933-
" pg_catalog.pg_namespace n, "
934-
" pg_catalog.pg_attribute a, "
935-
" pg_catalog.pg_type t "
936-
"WHERE c.oid = a.attrelid AND "
937-
" NOT a.attisdropped AND "
938-
" a.atttypid = t.oid AND "
939-
" t.typnamespace = "
940-
" (SELECT oid FROM pg_namespace "
941-
" WHERE nspname = 'pg_catalog') AND"
942-
" t.typname IN ( "
943-
/* regclass.oid is preserved, so 'regclass' is OK */
944-
" 'regconfig', "
945-
" 'regdictionary', "
946-
" 'regnamespace', "
947-
" 'regoper', "
948-
" 'regoperator', "
949-
" 'regproc', "
950-
" 'regprocedure' "
951-
/* regrole.oid is preserved, so 'regrole' is OK */
952-
/* regtype.oid is preserved, so 'regtype' is OK */
953-
" ) AND "
954-
" c.relnamespace = n.oid AND "
955-
" n.nspname NOT IN ('pg_catalog', 'information_schema')");
956-
957-
ntups = PQntuples(res);
958-
i_nspname = PQfnumber(res, "nspname");
959-
i_relname = PQfnumber(res, "relname");
960-
i_attname = PQfnumber(res, "attname");
961-
for (rowno = 0; rowno < ntups; rowno++)
962-
{
963-
found = true;
964-
if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
965-
pg_fatal("could not open file \"%s\": %s\n",
966-
output_path, strerror(errno));
967-
if (!db_used)
968-
{
969-
fprintf(script, "Database: %s\n", active_db->db_name);
970-
db_used = true;
971-
}
972-
fprintf(script, " %s.%s.%s\n",
973-
PQgetvalue(res, rowno, i_nspname),
974-
PQgetvalue(res, rowno, i_relname),
975-
PQgetvalue(res, rowno, i_attname));
976-
}
977-
978-
PQclear(res);
979-
980-
PQfinish(conn);
981-
}
982-
983-
if (script)
984-
fclose(script);
970+
/*
971+
* Note: older servers will not have all of these reg* types, so we have
972+
* to write the query like this rather than depending on casts to regtype.
973+
*/
974+
found = check_for_data_types_usage(cluster,
975+
"SELECT oid FROM pg_catalog.pg_type t "
976+
"WHERE t.typnamespace = "
977+
" (SELECT oid FROM pg_catalog.pg_namespace "
978+
" WHERE nspname = 'pg_catalog') "
979+
" AND t.typname IN ( "
980+
/* pg_class.oid is preserved, so 'regclass' is OK */
981+
" 'regcollation', "
982+
" 'regconfig', "
983+
" 'regdictionary', "
984+
" 'regnamespace', "
985+
" 'regoper', "
986+
" 'regoperator', "
987+
" 'regproc', "
988+
" 'regprocedure' "
989+
/* pg_authid.oid is preserved, so 'regrole' is OK */
990+
/* pg_type.oid is (mostly) preserved, so 'regtype' is OK */
991+
" )",
992+
output_path);
985993

986994
if (found)
987995
{
@@ -1006,75 +1014,13 @@ check_for_reg_data_type_usage(ClusterInfo *cluster)
10061014
static void
10071015
check_for_jsonb_9_4_usage(ClusterInfo *cluster)
10081016
{
1009-
int dbnum;
1010-
FILE *script = NULL;
1011-
bool found = false;
10121017
char output_path[MAXPGPATH];
10131018

10141019
prep_status("Checking for JSONB user data types");
10151020

10161021
snprintf(output_path, sizeof(output_path), "tables_using_jsonb.txt");
10171022

1018-
for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
1019-
{
1020-
PGresult *res;
1021-
bool db_used = false;
1022-
int ntups;
1023-
int rowno;
1024-
int i_nspname,
1025-
i_relname,
1026-
i_attname;
1027-
DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
1028-
PGconn *conn = connectToServer(cluster, active_db->db_name);
1029-
1030-
/*
1031-
* While several relkinds don't store any data, e.g. views, they can
1032-
* be used to define data types of other columns, so we check all
1033-
* relkinds.
1034-
*/
1035-
res = executeQueryOrDie(conn,
1036-
"SELECT n.nspname, c.relname, a.attname "
1037-
"FROM pg_catalog.pg_class c, "
1038-
" pg_catalog.pg_namespace n, "
1039-
" pg_catalog.pg_attribute a "
1040-
"WHERE c.oid = a.attrelid AND "
1041-
" NOT a.attisdropped AND "
1042-
" a.atttypid = 'pg_catalog.jsonb'::pg_catalog.regtype AND "
1043-
" c.relnamespace = n.oid AND "
1044-
/* exclude possible orphaned temp tables */
1045-
" n.nspname !~ '^pg_temp_' AND "
1046-
" n.nspname NOT IN ('pg_catalog', 'information_schema')");
1047-
1048-
ntups = PQntuples(res);
1049-
i_nspname = PQfnumber(res, "nspname");
1050-
i_relname = PQfnumber(res, "relname");
1051-
i_attname = PQfnumber(res, "attname");
1052-
for (rowno = 0; rowno < ntups; rowno++)
1053-
{
1054-
found = true;
1055-
if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
1056-
pg_fatal("could not open file \"%s\": %s\n",
1057-
output_path, strerror(errno));
1058-
if (!db_used)
1059-
{
1060-
fprintf(script, "Database: %s\n", active_db->db_name);
1061-
db_used = true;
1062-
}
1063-
fprintf(script, " %s.%s.%s\n",
1064-
PQgetvalue(res, rowno, i_nspname),
1065-
PQgetvalue(res, rowno, i_relname),
1066-
PQgetvalue(res, rowno, i_attname));
1067-
}
1068-
1069-
PQclear(res);
1070-
1071-
PQfinish(conn);
1072-
}
1073-
1074-
if (script)
1075-
fclose(script);
1076-
1077-
if (found)
1023+
if (check_for_data_type_usage(cluster, "pg_catalog.jsonb", output_path))
10781024
{
10791025
pg_log(PG_REPORT, "fatal\n");
10801026
pg_fatal("Your installation contains one of the JSONB data types in user tables.\n"

‎src/bin/pg_upgrade/pg_upgrade.h

Copy file name to clipboardExpand all lines: src/bin/pg_upgrade/pg_upgrade.h
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,12 @@ void pg_putenv(const char *var, const char *val);
441441

442442
/* version.c */
443443

444+
bool check_for_data_types_usage(ClusterInfo *cluster,
445+
const char *base_query,
446+
const char *output_path);
447+
bool check_for_data_type_usage(ClusterInfo *cluster,
448+
const char *typename,
449+
const char *output_path);
444450
void new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster,
445451
bool check_mode);
446452
void old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster);

‎src/bin/pg_upgrade/version.c

Copy file name to clipboardExpand all lines: src/bin/pg_upgrade/version.c
+43-10Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,22 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster, bool check_mode)
100100

101101

102102
/*
103-
* check_for_data_type_usage
104-
* Detect whether there are any stored columns depending on the given type
103+
* check_for_data_types_usage()
104+
* Detect whether there are any stored columns depending on given type(s)
105105
*
106106
* If so, write a report to the given file name, and return true.
107107
*
108-
* We check for the type in tables, matviews, and indexes, but not views;
108+
* base_query should be a SELECT yielding a single column named "oid",
109+
* containing the pg_type OIDs of one or more types that are known to have
110+
* inconsistent on-disk representations across server versions.
111+
*
112+
* We check for the type(s) in tables, matviews, and indexes, but not views;
109113
* there's no storage involved in a view.
110114
*/
111-
static bool
112-
check_for_data_type_usage(ClusterInfo *cluster, const char *typename,
113-
char *output_path)
115+
bool
116+
check_for_data_types_usage(ClusterInfo *cluster,
117+
const char *base_query,
118+
const char *output_path)
114119
{
115120
bool found = false;
116121
FILE *script = NULL;
@@ -130,16 +135,16 @@ check_for_data_type_usage(ClusterInfo *cluster, const char *typename,
130135
i_attname;
131136

132137
/*
133-
* The type of interest might be wrapped in a domain, array,
138+
* The type(s) of interest might be wrapped in a domain, array,
134139
* composite, or range, and these container types can be nested (to
135140
* varying extents depending on server version, but that's not of
136141
* concern here). To handle all these cases we need a recursive CTE.
137142
*/
138143
initPQExpBuffer(&querybuf);
139144
appendPQExpBuffer(&querybuf,
140145
"WITH RECURSIVE oids AS ( "
141-
/* the target type itself */
142-
" SELECT '%s'::pg_catalog.regtype AS oid "
146+
/* start with the type(s) returned by base_query */
147+
" %s "
143148
" UNION ALL "
144149
" SELECT * FROM ( "
145150
/* inner WITH because we can only reference the CTE once */
@@ -157,7 +162,7 @@ check_for_data_type_usage(ClusterInfo *cluster, const char *typename,
157162
" c.oid = a.attrelid AND "
158163
" NOT a.attisdropped AND "
159164
" a.atttypid = x.oid ",
160-
typename);
165+
base_query);
161166

162167
/* Ranges came in in 9.2 */
163168
if (GET_MAJOR_VERSION(cluster->major_version) >= 902)
@@ -225,6 +230,34 @@ check_for_data_type_usage(ClusterInfo *cluster, const char *typename,
225230
return found;
226231
}
227232

233+
/*
234+
* check_for_data_type_usage()
235+
* Detect whether there are any stored columns depending on the given type
236+
*
237+
* If so, write a report to the given file name, and return true.
238+
*
239+
* typename should be a fully qualified type name. This is just a
240+
* trivial wrapper around check_for_data_types_usage() to convert a
241+
* type name into a base query.
242+
*/
243+
bool
244+
check_for_data_type_usage(ClusterInfo *cluster,
245+
const char *typename,
246+
const char *output_path)
247+
{
248+
bool found;
249+
char *base_query;
250+
251+
base_query = psprintf("SELECT '%s'::pg_catalog.regtype AS oid",
252+
typename);
253+
254+
found = check_for_data_types_usage(cluster, base_query, output_path);
255+
256+
free(base_query);
257+
258+
return found;
259+
}
260+
228261

229262
/*
230263
* old_9_3_check_for_line_data_type_usage()

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.