[bug] Table not have typarray when created by single user mode
Hi
I found an exception using the latest master branch of PGSQL and wanted to check if it was a bug
Please refer this below scenario
1)initdb ./initdb -k -D data
2)Connect to server using single user mode ( ./postgres --single -D data postgres) and create a table
./postgres --single -D data Postgres
PostgreSQL stand-alone backend 13devel
backend>create table t_create_by_standalone(n int);
--Press Ctl+D to exit
3) use pg_ctl start database
pg_ctl -D data -c start
4) use psql connect database, and create a table
create table t_create_by_psql(n int);
5) check the pg_type info
postgres=# select oid,relname,reltype from pg_class where relname like 't_create%';
oid | relname | reltype
-------+------------------------+---------
13581 | t_create_by_standalone | 13582
16384 | t_create_by_psql | 16386
(2 rows)
postgres=# SELECT oid,typname, typarray FROM pg_catalog.pg_type WHERE oid in (13582,16386);
oid | typname | typarray
-------+------------------------+----------
13582 | t_create_by_standalone | 0
16386 | t_create_by_psql | 16385
(2 rows)
Use single user mode (t_create_by_standalone) typarray = 0, but use psql t_create_by_psql typarray has oid.
Is there something wrong to have different catalog information with the same sql?
wenjing <wjzeng2012@gmail.com> writes:
Use single user mode (t_create_by_standalone) typarray = 0, but use psql t_create_by_psql typarray has oid.
That's because of this in heap_create_with_catalog:
/*
* Decide whether to create an array type over the relation's rowtype. We
* do not create any array types for system catalogs (ie, those made
* during initdb). We do not create them where the use of a relation as
* such is an implementation detail: toast tables, sequences and indexes.
*/
if (IsUnderPostmaster && (relkind == RELKIND_RELATION ||
relkind == RELKIND_VIEW ||
relkind == RELKIND_MATVIEW ||
relkind == RELKIND_FOREIGN_TABLE ||
relkind == RELKIND_COMPOSITE_TYPE ||
relkind == RELKIND_PARTITIONED_TABLE))
new_array_oid = AssignTypeArrayOid();
Admittedly, "!IsUnderPostmaster" is not exactly the same thing as "running
during initdb", but I do not consider this a bug. You generally should
not be using single-user mode for anything except disaster recovery.
regards, tom lane
2020年4月13日 下午10:51，Tom Lane <tgl@sss.pgh.pa.us> 写道：
wenjing <wjzeng2012@gmail.com> writes:
Use single user mode (t_create_by_standalone) typarray = 0, but use psql t_create_by_psql typarray has oid.
That's because of this in heap_create_with_catalog:
/*
* Decide whether to create an array type over the relation's rowtype. We
* do not create any array types for system catalogs (ie, those made
* during initdb). We do not create them where the use of a relation as
* such is an implementation detail: toast tables, sequences and indexes.
*/
if (IsUnderPostmaster && (relkind == RELKIND_RELATION ||
relkind == RELKIND_VIEW ||
relkind == RELKIND_MATVIEW ||
relkind == RELKIND_FOREIGN_TABLE ||
relkind == RELKIND_COMPOSITE_TYPE ||
relkind == RELKIND_PARTITIONED_TABLE))
new_array_oid = AssignTypeArrayOid();Admittedly, "!IsUnderPostmaster" is not exactly the same thing as "running
during initdb", but I do not consider this a bug. You generally should
not be using single-user mode for anything except disaster recovery.regards, tom lane
Thanks for explain. I can understand your point.
However, if such a table exists, an error with pg_upgrade is further raised
./initdb -k -D datanew
./pg_upgrade -d data -d datanew - b. -b.
Restoring database schemas in the new cluster
postgres
*failure*
Consult the last few lines of "pg_upgrade_dump_13580.log" for
the probable cause of the failure.
Failure, exiting
pg_restore: from TOC entry 200; 1259 13581 TABLE t_create_by_standalone wenjing
pg_restore: error: could not execute query: ERROR: pg_type array OID value not set when in binary upgrade mode
Command was:
-- For binary upgrade, must preserve pg_type oid
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('13582'::pg_catalog.oid);
I wonder if there are any restrictions that need to be put in somewhere?
Wenjing
On 2020-Apr-14, wenjing wrote:
However, if such a table exists, an error with pg_upgrade is further raised
./initdb -k -D datanew
./pg_upgrade -d data -d datanew - b. -b.Restoring database schemas in the new cluster
postgres
*failure*Consult the last few lines of "pg_upgrade_dump_13580.log" for
the probable cause of the failure.
Failure, exitingpg_restore: from TOC entry 200; 1259 13581 TABLE t_create_by_standalone wenjing
pg_restore: error: could not execute query: ERROR: pg_type array OID value not set when in binary upgrade mode
Maybe the solution is to drop the table before pg_upgrade.
(Perhaps in --check mode pg_upgrade could warn you about such
situations. But then, should it warn you specifically about random
other instances of catalog corruption?)
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
2020年4月15日 上午2:00，Alvaro Herrera <alvherre@2ndquadrant.com> 写道：
On 2020-Apr-14, wenjing wrote:
However, if such a table exists, an error with pg_upgrade is further raised
./initdb -k -D datanew
./pg_upgrade -d data -d datanew - b. -b.Restoring database schemas in the new cluster
postgres
*failure*Consult the last few lines of "pg_upgrade_dump_13580.log" for
the probable cause of the failure.
Failure, exitingpg_restore: from TOC entry 200; 1259 13581 TABLE t_create_by_standalone wenjing
pg_restore: error: could not execute query: ERROR: pg_type array OID value not set when in binary upgrade modeMaybe the solution is to drop the table before pg_upgrade.
(Perhaps in --check mode pg_upgrade could warn you about such
situations. But then, should it warn you specifically about random
other instances of catalog corruption?)
I fixed the problem along your lines.
Please check.
Wenjing
Attachments:
check_for_incomplete_object_definition_v1.txttext/plain; name=check_for_incomplete_object_definition_v1.txt; x-unix-mode=0644Download
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 00aef85..abb5f8a 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -27,6 +27,7 @@ static void check_for_reg_data_type_usage(ClusterInfo *cluster);
static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
static void check_for_pg_role_prefix(ClusterInfo *cluster);
static char *get_canonical_locale_name(int category, const char *locale);
+static void check_for_incomplete_object_definition(ClusterInfo *cluster);
/*
@@ -142,6 +143,9 @@ check_and_dump_old_cluster(bool live_check)
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
new_9_0_populate_pg_largeobject_metadata(&old_cluster, true);
+ if (GET_MAJOR_VERSION(old_cluster.major_version) >= 802 && user_opts.check)
+ check_for_incomplete_object_definition(&old_cluster);
+
/*
* While not a check option, we do this now because this is the only time
* the old server is running.
@@ -1246,3 +1250,81 @@ get_canonical_locale_name(int category, const char *locale)
return res;
}
+
+/*
+ * check_for_incomplete_object_definition()
+ */
+static void
+check_for_incomplete_object_definition(ClusterInfo *cluster)
+{
+ int dbnum;
+ FILE *script = NULL;
+ bool found = false;
+ char output_path[MAXPGPATH];
+
+ prep_status("Checking for incomplete object definition");
+
+ snprintf(output_path, sizeof(output_path), "incomplete_object_definition.txt");
+
+ for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+ {
+ PGresult *res;
+ bool db_used = false;
+ int ntups;
+ int rowno;
+ int i_nspname,
+ i_relname;
+ DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
+ PGconn *conn = connectToServer(cluster, active_db->db_name);
+
+ res = executeQueryOrDie(conn,
+ "SELECT n.nspname, c.relname "
+ "FROM pg_catalog.pg_class c, "
+ " pg_catalog.pg_namespace n, "
+ " pg_catalog.pg_type t "
+ "WHERE c.relkind in('r', 'v', 'm', 'p', 'f') AND "
+ " t.oid = c.reltype AND "
+ " t.typarray = 0 AND "
+ " c.relnamespace = n.oid AND "
+ " n.nspname NOT IN ('pg_catalog', 'information_schema')");
+
+ ntups = PQntuples(res);
+ i_nspname = PQfnumber(res, "nspname");
+ i_relname = PQfnumber(res, "relname");
+ for (rowno = 0; rowno < ntups; rowno++)
+ {
+ found = true;
+ if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+ pg_fatal("could not open file \"%s\": %s\n",
+ output_path, strerror(errno));
+ if (!db_used)
+ {
+ fprintf(script, "In database: %s\n", active_db->db_name);
+ db_used = true;
+ }
+ fprintf(script, " %s.%s\n",
+ PQgetvalue(res, rowno, i_nspname),
+ PQgetvalue(res, rowno, i_relname));
+ }
+
+ PQclear(res);
+
+ PQfinish(conn);
+ }
+
+ if (script)
+ fclose(script);
+
+ if (found)
+ {
+ pg_log(PG_REPORT, "fatal\n");
+ pg_fatal("Your installation contains one or more incomplete table or view definitions.\n"
+ "They are probably created in single-user mode.\n"
+ "so this cluster cannot currently be upgraded.\n"
+ "You can remove the problem tables or views and restart the upgrade.\n"
+ "A list of the problem objects is in the file:\n"
+ " %s\n\n", output_path);
+ }
+ else
+ check_ok();
+}
Alvaro Herrera <alvherre@2ndquadrant.com> 于2020年5月19日周二 下午5:18写道：
On 2020-Apr-14, wenjing wrote:
However, if such a table exists, an error with pg_upgrade is further
raised
./initdb -k -D datanew
./pg_upgrade -d data -d datanew - b. -b.Restoring database schemas in the new cluster
postgres
*failure*Consult the last few lines of "pg_upgrade_dump_13580.log" for
the probable cause of the failure.
Failure, exitingpg_restore: from TOC entry 200; 1259 13581 TABLE t_create_by_standalone
wenjing
pg_restore: error: could not execute query: ERROR: pg_type array OID
value not set when in binary upgrade mode
Maybe the solution is to drop the table before pg_upgrade.
Hi, I don't understand about dropping the table.
Although single-user mode is used for bootstrapping by initdb. Sometimes it
is used for debugging or disaster recovery.
However, it is still possible for users to process data in this mode. If
only the table is deleted,
I worry that it will cause inconvenience to the user.
I don't understand why we must be IsUnderPostmaster to create an array type
too.
If we could create an array type in single-user mode, there is not this
issue.
Regards,
--
Shawn Wang
On 2020-May-19, shawn wang wrote:
Although single-user mode is used for bootstrapping by initdb. Sometimes it
is used for debugging or disaster recovery.
However, it is still possible for users to process data in this mode. If
only the table is deleted,
I worry that it will cause inconvenience to the user.
I don't understand why we must be IsUnderPostmaster to create an array type
too.
If we could create an array type in single-user mode, there is not this
issue.
Looking at the code again, there is one other possible solution: remove
the ereport(ERROR) from AssignTypeArrayOid. This means that we'll
return InvalidOid and the array type will be marked as 0 in the upgraded
cluster ... which is exactly the case in the original server. (Of
course, when array_oid is returned as invalid, the creation of the array
should be skipped, in callers of AssignTypeArrayOid.)
I think the argument to have that error check there, is that it's a
cross-check to avoid pg_upgrade bugs for cases where
binary_upgrade_next_array_type_oid is not set when it should have been
set. But I think we've hammered the pg_upgrade code sufficiently now,
that we don't need that check anymore. Any bugs that result in that
behavior will be very evident by lack of consistency on some upgrade
anyway.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I think the argument to have that error check there, is that it's a
cross-check to avoid pg_upgrade bugs for cases where
binary_upgrade_next_array_type_oid is not set when it should have been
set. But I think we've hammered the pg_upgrade code sufficiently now,
that we don't need that check anymore. Any bugs that result in that
behavior will be very evident by lack of consistency on some upgrade
anyway.
I don't buy that argument at all; that's a pretty critical cross-check
IMO, because it's quite important that pg_upgrade control all type OIDs
assigned in the new cluster. And I think it's probably easier to
break than you're hoping :-(
I think a safer fix is to replace the IsUnderPostmaster check in
heap_create_with_catalog with !IsBootstrapProcessingMode() or the
like. That would have the result that we'd create array types for
the information_schema views, as well as the system views made in
system_views.sql, which is slightly annoying but probably no real
harm in the big scheme of things. (I wonder if we ought to reverse
the sense of the adjacent relkind check, turning it into a blacklist,
while at it.)
I remain however of the opinion that doing this sort of thing in
single-user mode, or really much of anything beyond emergency
vacuuming, is unwise.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> 于2020年5月20日周三 上午12:09写道：
I think a safer fix is to replace the IsUnderPostmaster check in
heap_create_with_catalog with !IsBootstrapProcessingMode() or the
like. That would have the result that we'd create array types for
the information_schema views, as well as the system views made in
system_views.sql, which is slightly annoying but probably no real
harm in the big scheme of things. (I wonder if we ought to reverse
the sense of the adjacent relkind check, turning it into a blacklist,
while at it.)
Thank you for the explanation.
I prefer to change the conditions too.
I remain however of the opinion that doing this sort of thing in
single-user mode, or really much of anything beyond emergency
vacuuming, is unwise.
I do agree with you, but there is no clear point in the document (maybe I
did not read it all),
it is recommended to make it clear in the document.
Regards,
--
Shawn Wang
2020年5月20日 上午12:09，Tom Lane <tgl@sss.pgh.pa.us> 写道：
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I think the argument to have that error check there, is that it's a
cross-check to avoid pg_upgrade bugs for cases where
binary_upgrade_next_array_type_oid is not set when it should have been
set. But I think we've hammered the pg_upgrade code sufficiently now,
that we don't need that check anymore. Any bugs that result in that
behavior will be very evident by lack of consistency on some upgrade
anyway.I don't buy that argument at all; that's a pretty critical cross-check
IMO, because it's quite important that pg_upgrade control all type OIDs
assigned in the new cluster. And I think it's probably easier to
break than you're hoping :-(I think a safer fix is to replace the IsUnderPostmaster check in
heap_create_with_catalog with !IsBootstrapProcessingMode() or the
like. That would have the result that we'd create array types for
the information_schema views, as well as the system views made in
system_views.sql, which is slightly annoying but probably no real
harm in the big scheme of things. (I wonder if we ought to reverse
the sense of the adjacent relkind check, turning it into a blacklist,
while at it.)
Thanks for your help, This method passed all regression tests and pg_upgrade checks.
It looks perfect.
Wenjing
Attachments:
incomplete_array_type_in_single_user_mode_v2.patchapplication/octet-stream; name=incomplete_array_type_in_single_user_mode_v2.patch; x-unix-mode=0644Download
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index e393c93..ba5716b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1272,8 +1272,10 @@ heap_create_with_catalog(const char *relname,
* do not create any array types for system catalogs (ie, those made
* during initdb). We do not create them where the use of a relation as
* such is an implementation detail: toast tables, sequences and indexes.
+ *
+ * Only the Bootstrap mode is limited.
*/
- if (IsUnderPostmaster && (relkind == RELKIND_RELATION ||
+ if (!IsBootstrapProcessingMode() && (relkind == RELKIND_RELATION ||
relkind == RELKIND_VIEW ||
relkind == RELKIND_MATVIEW ||
relkind == RELKIND_FOREIGN_TABLE ||
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
I verified and found no problems.
On Wed, May 20, 2020 at 1:45 PM shawn wang <shawn.wang.pg@gmail.com> wrote:
Tom Lane <tgl@sss.pgh.pa.us> 于2020年5月20日周三 上午12:09写道：
I remain however of the opinion that doing this sort of thing in
single-user mode, or really much of anything beyond emergency
vacuuming, is unwise.I do agree with you, but there is no clear point in the document (maybe I did not read it all),
it is recommended to make it clear in the document.
It seems to be indicated in the docs [1]https://www.postgresql.org/docs/devel/app-postgres.html (see the paragraph starting
with "The postgres command can also be called in single-user mode
...") that it is used for debugging or disaster recovery.
[1]: https://www.postgresql.org/docs/devel/app-postgres.html
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Amit Kapila <amit.kapila16@gmail.com> 于2020年5月21日周四 下午7:37写道：
On Wed, May 20, 2020 at 1:45 PM shawn wang <shawn.wang.pg@gmail.com>
wrote:
Thank you for your reply.
It seems to be indicated in the docs [1] (see the paragraph starting
with "The postgres command can also be called in single-user mode
...") that it is used for debugging or disaster recovery.[1] - https://www.postgresql.org/docs/devel/app-postgres.html
The description here is what users should do, and my suggestion is to
clearly indicate what users cannot do.
Regards,
--
Shawn Wang
I poked at this patch a bit, and was reminded of the real reason why
we'd skipped making these array types in the first place: it bloats
pg_type noticeably. As of HEAD, a freshly initialized database has
411 rows in pg_type. As written this patch results in 543 entries,
or a 32% increase. That seems like kind of a lot. On the other hand,
in the big scheme of things maybe it's negligible. pg_type is still
far from the largest catalog:
postgres=# select relname, relpages from pg_class order by 2 desc;
relname | relpages
-----------------------------------------------+----------
pg_proc | 81
pg_toast_2618 | 60
pg_depend | 59
pg_attribute | 53
pg_depend_reference_index | 44
pg_description | 36
pg_depend_depender_index | 35
pg_collation | 32
pg_proc_proname_args_nsp_index | 32
pg_description_o_c_o_index | 21
pg_statistic | 19
pg_attribute_relid_attnam_index | 15
pg_operator | 14
pg_type | 14 <--- up from 10
pg_class | 13
pg_rewrite | 12
pg_proc_oid_index | 11
...
However, if we're going to go this far, I think there's a good
case to be made for going all the way and eliminating the policy
of not making array types for system catalogs. That was never
anything but a wart justified by space savings in pg_type, and
this patch already kills most of the space savings. If we
drop the system-state test in heap_create_with_catalog altogether,
we end up with 601 initial pg_type entries. That still leaves
the four bootstrap catalogs without array types, because they are
not created by heap_create_with_catalog; but we can manually add
those too for a total of 605 initial entries. (That brings initial
pg_type to 14 pages as I show above; I think it was 13 with the
original version of the patch.)
In short, if we're gonna do this, I think we should do it like
the attached. Or we could do nothing, but there is some appeal
to removing this old inconsistency.
regards, tom lane
Attachments:
build-array-types-for-system-catalogs-1.patchtext/x-diff; charset=us-ascii; name=build-array-types-for-system-catalogs-1.patchDownload
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index d279842d3c..fd04e82b20 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1262,17 +1262,14 @@ heap_create_with_catalog(const char *relname,
new_rel_desc->rd_rel->relrewrite = relrewrite;
/*
- * Decide whether to create an array type over the relation's rowtype. We
- * do not create any array types for system catalogs (ie, those made
- * during initdb). We do not create them where the use of a relation as
- * such is an implementation detail: toast tables, sequences and indexes.
- */
- if (IsUnderPostmaster && (relkind == RELKIND_RELATION ||
- relkind == RELKIND_VIEW ||
- relkind == RELKIND_MATVIEW ||
- relkind == RELKIND_FOREIGN_TABLE ||
- relkind == RELKIND_COMPOSITE_TYPE ||
- relkind == RELKIND_PARTITIONED_TABLE))
+ * Decide whether to create an array type over the relation's rowtype.
+ * Array types are made except where the use of a relation as such is an
+ * implementation detail: toast tables, sequences and indexes.
+ */
+ if (!(relkind == RELKIND_SEQUENCE ||
+ relkind == RELKIND_TOASTVALUE ||
+ relkind == RELKIND_INDEX ||
+ relkind == RELKIND_PARTITIONED_INDEX))
new_array_oid = AssignTypeArrayOid();
/*
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index e8be000835..b2cec07416 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -113,22 +113,22 @@
# hand-built rowtype entries for bootstrapped catalogs
# NB: OIDs assigned here must match the BKI_ROWTYPE_OID declarations
-{ oid => '71',
+{ oid => '71', array_type_oid => '210',
typname => 'pg_type', typlen => '-1', typbyval => 'f', typtype => 'c',
typcategory => 'C', typrelid => 'pg_type', typinput => 'record_in',
typoutput => 'record_out', typreceive => 'record_recv',
typsend => 'record_send', typalign => 'd', typstorage => 'x' },
-{ oid => '75',
+{ oid => '75', array_type_oid => '270',
typname => 'pg_attribute', typlen => '-1', typbyval => 'f', typtype => 'c',
typcategory => 'C', typrelid => 'pg_attribute', typinput => 'record_in',
typoutput => 'record_out', typreceive => 'record_recv',
typsend => 'record_send', typalign => 'd', typstorage => 'x' },
-{ oid => '81',
+{ oid => '81', array_type_oid => '272',
typname => 'pg_proc', typlen => '-1', typbyval => 'f', typtype => 'c',
typcategory => 'C', typrelid => 'pg_proc', typinput => 'record_in',
typoutput => 'record_out', typreceive => 'record_recv',
typsend => 'record_send', typalign => 'd', typstorage => 'x' },
-{ oid => '83',
+{ oid => '83', array_type_oid => '273',
typname => 'pg_class', typlen => '-1', typbyval => 'f', typtype => 'c',
typcategory => 'C', typrelid => 'pg_class', typinput => 'record_in',
typoutput => 'record_out', typreceive => 'record_recv',
I wrote:
However, if we're going to go this far, I think there's a good
case to be made for going all the way and eliminating the policy
of not making array types for system catalogs. That was never
anything but a wart justified by space savings in pg_type, and
this patch already kills most of the space savings. If we
drop the system-state test in heap_create_with_catalog altogether,
we end up with 601 initial pg_type entries. That still leaves
the four bootstrap catalogs without array types, because they are
not created by heap_create_with_catalog; but we can manually add
those too for a total of 605 initial entries. (That brings initial
pg_type to 14 pages as I show above; I think it was 13 with the
original version of the patch.)
In short, if we're gonna do this, I think we should do it like
the attached. Or we could do nothing, but there is some appeal
to removing this old inconsistency.
I pushed that, but while working on it I had a further thought:
why is it that we create composite types but not arrays over those
types for *any* relkinds? That is, we could create even more
consistency, as well as buying back some of the pg_type bloat added
here, by not creating pg_type entries at all for toast tables or
sequences. A little bit of hacking later, I have the attached.
One could argue it either way as to whether sequences should have
composite types. It's possible to demonstrate queries that will
fail without one:
regression=# create sequence seq1;
CREATE SEQUENCE
regression=# select s from seq1 s;
ERROR: relation "seq1" does not have a composite type
but it's pretty hard to believe anyone's using that in practice.
Also, we've talked more than once about changing the implementation
of sequences to not have a relation per sequence, in which case the
ability to do something like the above would go away anyway.
Comments?
regards, tom lane
Attachments:
remove-pg_type-entries-for-seqs-and-toast-tables.patchtext/x-diff; charset=us-ascii; name=remove-pg_type-entries-for-seqs-and-toast-tables.patchDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 003d278370..7471ba53f2 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1895,7 +1895,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para>
<para>
The OID of the data type that corresponds to this table's row type,
- if any (zero for indexes, which have no <structname>pg_type</structname> entry)
+ if any (zero for indexes, sequences, and toast tables, which have
+ no <structname>pg_type</structname> entry)
</para></entry>
</row>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index fd04e82b20..ae509d9d49 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1118,8 +1118,6 @@ heap_create_with_catalog(const char *relname,
Oid existing_relid;
Oid old_type_oid;
Oid new_type_oid;
- ObjectAddress new_type_addr;
- Oid new_array_oid = InvalidOid;
TransactionId relfrozenxid;
MultiXactId relminmxid;
@@ -1262,44 +1260,45 @@ heap_create_with_catalog(const char *relname,
new_rel_desc->rd_rel->relrewrite = relrewrite;
/*
- * Decide whether to create an array type over the relation's rowtype.
- * Array types are made except where the use of a relation as such is an
+ * Decide whether to create a pg_type entry for the relation's rowtype.
+ * These types are made except where the use of a relation as such is an
* implementation detail: toast tables, sequences and indexes.
*/
if (!(relkind == RELKIND_SEQUENCE ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_PARTITIONED_INDEX))
- new_array_oid = AssignTypeArrayOid();
-
- /*
- * Since defining a relation also defines a complex type, we add a new
- * system type corresponding to the new relation. The OID of the type can
- * be preselected by the caller, but if reltypeid is InvalidOid, we'll
- * generate a new OID for it.
- *
- * NOTE: we could get a unique-index failure here, in case someone else is
- * creating the same type name in parallel but hadn't committed yet when
- * we checked for a duplicate name above.
- */
- new_type_addr = AddNewRelationType(relname,
- relnamespace,
- relid,
- relkind,
- ownerid,
- reltypeid,
- new_array_oid);
- new_type_oid = new_type_addr.objectId;
- if (typaddress)
- *typaddress = new_type_addr;
-
- /*
- * Now make the array type if wanted.
- */
- if (OidIsValid(new_array_oid))
{
+ Oid new_array_oid;
+ ObjectAddress new_type_addr;
char *relarrayname;
+ /*
+ * We'll make an array over the composite type, too. For largely
+ * historical reasons, the array type's OID is assigned first.
+ */
+ new_array_oid = AssignTypeArrayOid();
+
+ /*
+ * The OID of the composite type can be preselected by the caller, but
+ * if reltypeid is InvalidOid, we'll generate a new OID for it.
+ *
+ * NOTE: we could get a unique-index failure here, in case someone
+ * else is creating the same type name in parallel but hadn't
+ * committed yet when we checked for a duplicate name above.
+ */
+ new_type_addr = AddNewRelationType(relname,
+ relnamespace,
+ relid,
+ relkind,
+ ownerid,
+ reltypeid,
+ new_array_oid);
+ new_type_oid = new_type_addr.objectId;
+ if (typaddress)
+ *typaddress = new_type_addr;
+
+ /* Now create the array type. */
relarrayname = makeArrayTypeName(relname, relnamespace);
TypeCreate(new_array_oid, /* force the type's OID to this */
@@ -1336,6 +1335,14 @@ heap_create_with_catalog(const char *relname,
pfree(relarrayname);
}
+ else
+ {
+ /* Caller should not be expecting a type to be created. */
+ Assert(reltypeid == InvalidOid);
+ Assert(typaddress == NULL);
+
+ new_type_oid = InvalidOid;
+ }
/*
* now create an entry in pg_class for the relation.
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 3f7ab8d389..8b8888af5e 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -34,9 +34,6 @@
#include "utils/rel.h"
#include "utils/syscache.h"
-/* Potentially set by pg_upgrade_support functions */
-Oid binary_upgrade_next_toast_pg_type_oid = InvalidOid;
-
static void CheckAndCreateToastTable(Oid relOid, Datum reloptions,
LOCKMODE lockmode, bool check);
static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
@@ -135,7 +132,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
Relation toast_rel;
Relation class_rel;
Oid toast_relid;
- Oid toast_typid = InvalidOid;
Oid namespaceid;
char toast_relname[NAMEDATALEN];
char toast_idxname[NAMEDATALEN];
@@ -181,8 +177,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
* problem that it might take up an OID that will conflict with some
* old-cluster table we haven't seen yet.
*/
- if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid) ||
- !OidIsValid(binary_upgrade_next_toast_pg_type_oid))
+ if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid))
return false;
}
@@ -234,17 +229,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
else
namespaceid = PG_TOAST_NAMESPACE;
- /*
- * Use binary-upgrade override for pg_type.oid, if supplied. We might be
- * in the post-schema-restore phase where we are doing ALTER TABLE to
- * create TOAST tables that didn't exist in the old cluster.
- */
- if (IsBinaryUpgrade && OidIsValid(binary_upgrade_next_toast_pg_type_oid))
- {
- toast_typid = binary_upgrade_next_toast_pg_type_oid;
- binary_upgrade_next_toast_pg_type_oid = InvalidOid;
- }
-
/* Toast table is shared if and only if its parent is. */
shared_relation = rel->rd_rel->relisshared;
@@ -255,7 +239,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
namespaceid,
rel->rd_rel->reltablespace,
toastOid,
- toast_typid,
+ InvalidOid,
InvalidOid,
rel->rd_rel->relowner,
table_relation_toast_am(rel),
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f79044f39f..4b2548f33f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12564,8 +12564,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
/*
* Also change the ownership of the table's row type, if it has one
*/
- if (tuple_class->relkind != RELKIND_INDEX &&
- tuple_class->relkind != RELKIND_PARTITIONED_INDEX)
+ if (OidIsValid(tuple_class->reltype))
AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);
/*
@@ -15009,9 +15008,10 @@ AlterTableNamespaceInternal(Relation rel, Oid oldNspOid, Oid nspOid,
AlterRelationNamespaceInternal(classRel, RelationGetRelid(rel), oldNspOid,
nspOid, true, objsMoved);
- /* Fix the table's row type too */
- AlterTypeNamespaceInternal(rel->rd_rel->reltype,
- nspOid, false, false, objsMoved);
+ /* Fix the table's row type too, if it has one */
+ if (OidIsValid(rel->rd_rel->reltype))
+ AlterTypeNamespaceInternal(rel->rd_rel->reltype,
+ nspOid, false, false, objsMoved);
/* Fix other dependent stuff */
if (rel->rd_rel->relkind == RELKIND_RELATION ||
@@ -15206,11 +15206,11 @@ AlterSeqNamespaces(Relation classRel, Relation rel,
true, objsMoved);
/*
- * Sequences have entries in pg_type. We need to be careful to move
- * them to the new namespace, too.
+ * Sequences used to have entries in pg_type, but no longer do. If we
+ * ever re-instate that, we'll need to move the pg_type entry to the
+ * new namespace, too (using AlterTypeNamespaceInternal).
*/
- AlterTypeNamespaceInternal(RelationGetForm(seqRel)->reltype,
- newNspOid, false, false, objsMoved);
+ Assert(RelationGetForm(seqRel)->reltype == InvalidOid);
/* Now we can close it. Keep the lock till end of transaction. */
relation_close(seqRel, NoLock);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index b442b5a29e..49de285f01 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -145,8 +145,10 @@ makeWholeRowVar(RangeTblEntry *rte,
/* relation: the rowtype is a named composite type */
toid = get_rel_type_id(rte->relid);
if (!OidIsValid(toid))
- elog(ERROR, "could not find type OID for relation %u",
- rte->relid);
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("relation \"%s\" does not have a composite type",
+ get_rel_name(rte->relid))));
result = makeVar(varno,
InvalidAttrNumber,
toid,
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index 18f2ee8226..14d9eb2b5b 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -51,17 +51,6 @@ binary_upgrade_set_next_array_pg_type_oid(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-Datum
-binary_upgrade_set_next_toast_pg_type_oid(PG_FUNCTION_ARGS)
-{
- Oid typoid = PG_GETARG_OID(0);
-
- CHECK_IS_BINARY_UPGRADE;
- binary_upgrade_next_toast_pg_type_oid = typoid;
-
- PG_RETURN_VOID();
-}
-
Datum
binary_upgrade_set_next_heap_pg_class_oid(PG_FUNCTION_ARGS)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a41a3db876..ee0947dda7 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -272,7 +272,7 @@ static void binary_upgrade_set_type_oids_by_type_oid(Archive *fout,
PQExpBuffer upgrade_buffer,
Oid pg_type_oid,
bool force_array_type);
-static bool binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
+static void binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
PQExpBuffer upgrade_buffer, Oid pg_rel_oid);
static void binary_upgrade_set_pg_class_oids(Archive *fout,
PQExpBuffer upgrade_buffer,
@@ -4493,7 +4493,7 @@ binary_upgrade_set_type_oids_by_type_oid(Archive *fout,
destroyPQExpBuffer(upgrade_query);
}
-static bool
+static void
binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
PQExpBuffer upgrade_buffer,
Oid pg_rel_oid)
@@ -4501,48 +4501,23 @@ binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
PQExpBuffer upgrade_query = createPQExpBuffer();
PGresult *upgrade_res;
Oid pg_type_oid;
- bool toast_set = false;
- /*
- * We only support old >= 8.3 for binary upgrades.
- *
- * We purposefully ignore toast OIDs for partitioned tables; the reason is
- * that versions 10 and 11 have them, but 12 does not, so emitting them
- * causes the upgrade to fail.
- */
appendPQExpBuffer(upgrade_query,
- "SELECT c.reltype AS crel, t.reltype AS trel "
+ "SELECT c.reltype AS crel "
"FROM pg_catalog.pg_class c "
- "LEFT JOIN pg_catalog.pg_class t ON "
- " (c.reltoastrelid = t.oid AND c.relkind <> '%c') "
"WHERE c.oid = '%u'::pg_catalog.oid;",
- RELKIND_PARTITIONED_TABLE, pg_rel_oid);
+ pg_rel_oid);
upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
pg_type_oid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "crel")));
- binary_upgrade_set_type_oids_by_type_oid(fout, upgrade_buffer,
- pg_type_oid, false);
-
- if (!PQgetisnull(upgrade_res, 0, PQfnumber(upgrade_res, "trel")))
- {
- /* Toast tables do not have pg_type array rows */
- Oid pg_type_toast_oid = atooid(PQgetvalue(upgrade_res, 0,
- PQfnumber(upgrade_res, "trel")));
-
- appendPQExpBufferStr(upgrade_buffer, "\n-- For binary upgrade, must preserve pg_type toast oid\n");
- appendPQExpBuffer(upgrade_buffer,
- "SELECT pg_catalog.binary_upgrade_set_next_toast_pg_type_oid('%u'::pg_catalog.oid);\n\n",
- pg_type_toast_oid);
-
- toast_set = true;
- }
+ if (OidIsValid(pg_type_oid))
+ binary_upgrade_set_type_oids_by_type_oid(fout, upgrade_buffer,
+ pg_type_oid, false);
PQclear(upgrade_res);
destroyPQExpBuffer(upgrade_query);
-
- return toast_set;
}
static void
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index 12d94fe1b3..02fecb90f7 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -16,7 +16,6 @@
extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
-extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_heap_pg_class_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_index_pg_class_oid;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 38295aca48..a995a104b6 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10306,10 +10306,6 @@
proname => 'binary_upgrade_set_next_array_pg_type_oid', provolatile => 'v',
proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
prosrc => 'binary_upgrade_set_next_array_pg_type_oid' },
-{ oid => '3585', descr => 'for use by pg_upgrade',
- proname => 'binary_upgrade_set_next_toast_pg_type_oid', provolatile => 'v',
- proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
- prosrc => 'binary_upgrade_set_next_toast_pg_type_oid' },
{ oid => '3586', descr => 'for use by pg_upgrade',
proname => 'binary_upgrade_set_next_heap_pg_class_oid', provolatile => 'v',
proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 828ff5a288..e7f4a5f291 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -1778,6 +1778,7 @@ PLpgSQL_type *
plpgsql_parse_wordrowtype(char *ident)
{
Oid classOid;
+ Oid typOid;
/*
* Look up the relation. Note that because relation rowtypes have the
@@ -1792,8 +1793,16 @@ plpgsql_parse_wordrowtype(char *ident)
(errcode(ERRCODE_UNDEFINED_TABLE),
errmsg("relation \"%s\" does not exist", ident)));
+ /* Some relkinds lack type OIDs */
+ typOid = get_rel_type_id(classOid);
+ if (!OidIsValid(typOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("relation \"%s\" does not have a composite type",
+ ident)));
+
/* Build and return the row type struct */
- return plpgsql_build_datatype(get_rel_type_id(classOid), -1, InvalidOid,
+ return plpgsql_build_datatype(typOid, -1, InvalidOid,
makeTypeName(ident));
}
@@ -1806,6 +1815,7 @@ PLpgSQL_type *
plpgsql_parse_cwordrowtype(List *idents)
{
Oid classOid;
+ Oid typOid;
RangeVar *relvar;
MemoryContext oldCxt;
@@ -1825,10 +1835,18 @@ plpgsql_parse_cwordrowtype(List *idents)
-1);
classOid = RangeVarGetRelid(relvar, NoLock, false);
+ /* Some relkinds lack type OIDs */
+ typOid = get_rel_type_id(classOid);
+ if (!OidIsValid(typOid))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("relation \"%s\" does not have a composite type",
+ strVal(lsecond(idents)))));
+
MemoryContextSwitchTo(oldCxt);
/* Build and return the row type struct */
- return plpgsql_build_datatype(get_rel_type_id(classOid), -1, InvalidOid,
+ return plpgsql_build_datatype(typOid, -1, InvalidOid,
makeTypeNameFromNameList(idents));
}
Thank you very much Tom. It looks perfect.
I don't have any more questions.
Wenjing
Show quoted text
2020年7月7日 上午4:22，Tom Lane <tgl@sss.pgh.pa.us> 写道：
I wrote:
However, if we're going to go this far, I think there's a good
case to be made for going all the way and eliminating the policy
of not making array types for system catalogs. That was never
anything but a wart justified by space savings in pg_type, and
this patch already kills most of the space savings. If we
drop the system-state test in heap_create_with_catalog altogether,
we end up with 601 initial pg_type entries. That still leaves
the four bootstrap catalogs without array types, because they are
not created by heap_create_with_catalog; but we can manually add
those too for a total of 605 initial entries. (That brings initial
pg_type to 14 pages as I show above; I think it was 13 with the
original version of the patch.)
In short, if we're gonna do this, I think we should do it like
the attached. Or we could do nothing, but there is some appeal
to removing this old inconsistency.I pushed that, but while working on it I had a further thought:
why is it that we create composite types but not arrays over those
types for *any* relkinds? That is, we could create even more
consistency, as well as buying back some of the pg_type bloat added
here, by not creating pg_type entries at all for toast tables or
sequences. A little bit of hacking later, I have the attached.One could argue it either way as to whether sequences should have
composite types. It's possible to demonstrate queries that will
fail without one:regression=# create sequence seq1;
CREATE SEQUENCE
regression=# select s from seq1 s;
ERROR: relation "seq1" does not have a composite typebut it's pretty hard to believe anyone's using that in practice.
Also, we've talked more than once about changing the implementation
of sequences to not have a relation per sequence, in which case the
ability to do something like the above would go away anyway.Comments?
regards, tom lane
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 003d278370..7471ba53f2 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1895,7 +1895,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l </para> <para> The OID of the data type that corresponds to this table's row type, - if any (zero for indexes, which have no <structname>pg_type</structname> entry) + if any (zero for indexes, sequences, and toast tables, which have + no <structname>pg_type</structname> entry) </para></entry> </row>diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index fd04e82b20..ae509d9d49 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1118,8 +1118,6 @@ heap_create_with_catalog(const char *relname, Oid existing_relid; Oid old_type_oid; Oid new_type_oid; - ObjectAddress new_type_addr; - Oid new_array_oid = InvalidOid; TransactionId relfrozenxid; MultiXactId relminmxid;@@ -1262,44 +1260,45 @@ heap_create_with_catalog(const char *relname,
new_rel_desc->rd_rel->relrewrite = relrewrite;/* - * Decide whether to create an array type over the relation's rowtype. - * Array types are made except where the use of a relation as such is an + * Decide whether to create a pg_type entry for the relation's rowtype. + * These types are made except where the use of a relation as such is an * implementation detail: toast tables, sequences and indexes. */ if (!(relkind == RELKIND_SEQUENCE || relkind == RELKIND_TOASTVALUE || relkind == RELKIND_INDEX || relkind == RELKIND_PARTITIONED_INDEX)) - new_array_oid = AssignTypeArrayOid(); - - /* - * Since defining a relation also defines a complex type, we add a new - * system type corresponding to the new relation. The OID of the type can - * be preselected by the caller, but if reltypeid is InvalidOid, we'll - * generate a new OID for it. - * - * NOTE: we could get a unique-index failure here, in case someone else is - * creating the same type name in parallel but hadn't committed yet when - * we checked for a duplicate name above. - */ - new_type_addr = AddNewRelationType(relname, - relnamespace, - relid, - relkind, - ownerid, - reltypeid, - new_array_oid); - new_type_oid = new_type_addr.objectId; - if (typaddress) - *typaddress = new_type_addr; - - /* - * Now make the array type if wanted. - */ - if (OidIsValid(new_array_oid)) { + Oid new_array_oid; + ObjectAddress new_type_addr; char *relarrayname;+ /* + * We'll make an array over the composite type, too. For largely + * historical reasons, the array type's OID is assigned first. + */ + new_array_oid = AssignTypeArrayOid(); + + /* + * The OID of the composite type can be preselected by the caller, but + * if reltypeid is InvalidOid, we'll generate a new OID for it. + * + * NOTE: we could get a unique-index failure here, in case someone + * else is creating the same type name in parallel but hadn't + * committed yet when we checked for a duplicate name above. + */ + new_type_addr = AddNewRelationType(relname, + relnamespace, + relid, + relkind, + ownerid, + reltypeid, + new_array_oid); + new_type_oid = new_type_addr.objectId; + if (typaddress) + *typaddress = new_type_addr; + + /* Now create the array type. */ relarrayname = makeArrayTypeName(relname, relnamespace);TypeCreate(new_array_oid, /* force the type's OID to this */
@@ -1336,6 +1335,14 @@ heap_create_with_catalog(const char *relname,pfree(relarrayname); } + else + { + /* Caller should not be expecting a type to be created. */ + Assert(reltypeid == InvalidOid); + Assert(typaddress == NULL); + + new_type_oid = InvalidOid; + }/* * now create an entry in pg_class for the relation. diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 3f7ab8d389..8b8888af5e 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -34,9 +34,6 @@ #include "utils/rel.h" #include "utils/syscache.h"-/* Potentially set by pg_upgrade_support functions */ -Oid binary_upgrade_next_toast_pg_type_oid = InvalidOid; - static void CheckAndCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode, bool check); static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, @@ -135,7 +132,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Relation toast_rel; Relation class_rel; Oid toast_relid; - Oid toast_typid = InvalidOid; Oid namespaceid; char toast_relname[NAMEDATALEN]; char toast_idxname[NAMEDATALEN]; @@ -181,8 +177,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, * problem that it might take up an OID that will conflict with some * old-cluster table we haven't seen yet. */ - if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid) || - !OidIsValid(binary_upgrade_next_toast_pg_type_oid)) + if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid)) return false; }@@ -234,17 +229,6 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
else
namespaceid = PG_TOAST_NAMESPACE;- /*
- * Use binary-upgrade override for pg_type.oid, if supplied. We might be
- * in the post-schema-restore phase where we are doing ALTER TABLE to
- * create TOAST tables that didn't exist in the old cluster.
- */
- if (IsBinaryUpgrade && OidIsValid(binary_upgrade_next_toast_pg_type_oid))
- {
- toast_typid = binary_upgrade_next_toast_pg_type_oid;
- binary_upgrade_next_toast_pg_type_oid = InvalidOid;
- }
-
/* Toast table is shared if and only if its parent is. */
shared_relation = rel->rd_rel->relisshared;@@ -255,7 +239,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, namespaceid, rel->rd_rel->reltablespace, toastOid, - toast_typid, + InvalidOid, InvalidOid, rel->rd_rel->relowner, table_relation_toast_am(rel), diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f79044f39f..4b2548f33f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12564,8 +12564,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock /* * Also change the ownership of the table's row type, if it has one */ - if (tuple_class->relkind != RELKIND_INDEX && - tuple_class->relkind != RELKIND_PARTITIONED_INDEX) + if (OidIsValid(tuple_class->reltype)) AlterTypeOwnerInternal(tuple_class->reltype, newOwnerId);/*
@@ -15009,9 +15008,10 @@ AlterTableNamespaceInternal(Relation rel, Oid oldNspOid, Oid nspOid,
AlterRelationNamespaceInternal(classRel, RelationGetRelid(rel), oldNspOid,
nspOid, true, objsMoved);- /* Fix the table's row type too */ - AlterTypeNamespaceInternal(rel->rd_rel->reltype, - nspOid, false, false, objsMoved); + /* Fix the table's row type too, if it has one */ + if (OidIsValid(rel->rd_rel->reltype)) + AlterTypeNamespaceInternal(rel->rd_rel->reltype, + nspOid, false, false, objsMoved);/* Fix other dependent stuff */
if (rel->rd_rel->relkind == RELKIND_RELATION ||
@@ -15206,11 +15206,11 @@ AlterSeqNamespaces(Relation classRel, Relation rel,
true, objsMoved);/* - * Sequences have entries in pg_type. We need to be careful to move - * them to the new namespace, too. + * Sequences used to have entries in pg_type, but no longer do. If we + * ever re-instate that, we'll need to move the pg_type entry to the + * new namespace, too (using AlterTypeNamespaceInternal). */ - AlterTypeNamespaceInternal(RelationGetForm(seqRel)->reltype, - newNspOid, false, false, objsMoved); + Assert(RelationGetForm(seqRel)->reltype == InvalidOid);/* Now we can close it. Keep the lock till end of transaction. */ relation_close(seqRel, NoLock); diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index b442b5a29e..49de285f01 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -145,8 +145,10 @@ makeWholeRowVar(RangeTblEntry *rte, /* relation: the rowtype is a named composite type */ toid = get_rel_type_id(rte->relid); if (!OidIsValid(toid)) - elog(ERROR, "could not find type OID for relation %u", - rte->relid); + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("relation \"%s\" does not have a composite type", + get_rel_name(rte->relid)))); result = makeVar(varno, InvalidAttrNumber, toid, diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c index 18f2ee8226..14d9eb2b5b 100644 --- a/src/backend/utils/adt/pg_upgrade_support.c +++ b/src/backend/utils/adt/pg_upgrade_support.c @@ -51,17 +51,6 @@ binary_upgrade_set_next_array_pg_type_oid(PG_FUNCTION_ARGS) PG_RETURN_VOID(); }-Datum -binary_upgrade_set_next_toast_pg_type_oid(PG_FUNCTION_ARGS) -{ - Oid typoid = PG_GETARG_OID(0); - - CHECK_IS_BINARY_UPGRADE; - binary_upgrade_next_toast_pg_type_oid = typoid; - - PG_RETURN_VOID(); -} - Datum binary_upgrade_set_next_heap_pg_class_oid(PG_FUNCTION_ARGS) { diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index a41a3db876..ee0947dda7 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -272,7 +272,7 @@ static void binary_upgrade_set_type_oids_by_type_oid(Archive *fout, PQExpBuffer upgrade_buffer, Oid pg_type_oid, bool force_array_type); -static bool binary_upgrade_set_type_oids_by_rel_oid(Archive *fout, +static void binary_upgrade_set_type_oids_by_rel_oid(Archive *fout, PQExpBuffer upgrade_buffer, Oid pg_rel_oid); static void binary_upgrade_set_pg_class_oids(Archive *fout, PQExpBuffer upgrade_buffer, @@ -4493,7 +4493,7 @@ binary_upgrade_set_type_oids_by_type_oid(Archive *fout, destroyPQExpBuffer(upgrade_query); }-static bool
+static void
binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
PQExpBuffer upgrade_buffer,
Oid pg_rel_oid)
@@ -4501,48 +4501,23 @@ binary_upgrade_set_type_oids_by_rel_oid(Archive *fout,
PQExpBuffer upgrade_query = createPQExpBuffer();
PGresult *upgrade_res;
Oid pg_type_oid;
- bool toast_set = false;- /* - * We only support old >= 8.3 for binary upgrades. - * - * We purposefully ignore toast OIDs for partitioned tables; the reason is - * that versions 10 and 11 have them, but 12 does not, so emitting them - * causes the upgrade to fail. - */ appendPQExpBuffer(upgrade_query, - "SELECT c.reltype AS crel, t.reltype AS trel " + "SELECT c.reltype AS crel " "FROM pg_catalog.pg_class c " - "LEFT JOIN pg_catalog.pg_class t ON " - " (c.reltoastrelid = t.oid AND c.relkind <> '%c') " "WHERE c.oid = '%u'::pg_catalog.oid;", - RELKIND_PARTITIONED_TABLE, pg_rel_oid); + pg_rel_oid);upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
pg_type_oid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "crel")));
- binary_upgrade_set_type_oids_by_type_oid(fout, upgrade_buffer, - pg_type_oid, false); - - if (!PQgetisnull(upgrade_res, 0, PQfnumber(upgrade_res, "trel"))) - { - /* Toast tables do not have pg_type array rows */ - Oid pg_type_toast_oid = atooid(PQgetvalue(upgrade_res, 0, - PQfnumber(upgrade_res, "trel"))); - - appendPQExpBufferStr(upgrade_buffer, "\n-- For binary upgrade, must preserve pg_type toast oid\n"); - appendPQExpBuffer(upgrade_buffer, - "SELECT pg_catalog.binary_upgrade_set_next_toast_pg_type_oid('%u'::pg_catalog.oid);\n\n", - pg_type_toast_oid); - - toast_set = true; - } + if (OidIsValid(pg_type_oid)) + binary_upgrade_set_type_oids_by_type_oid(fout, upgrade_buffer, + pg_type_oid, false);PQclear(upgrade_res);
destroyPQExpBuffer(upgrade_query);
-
- return toast_set;
}static void diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h index 12d94fe1b3..02fecb90f7 100644 --- a/src/include/catalog/binary_upgrade.h +++ b/src/include/catalog/binary_upgrade.h @@ -16,7 +16,6 @@extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
-extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;extern PGDLLIMPORT Oid binary_upgrade_next_heap_pg_class_oid; extern PGDLLIMPORT Oid binary_upgrade_next_index_pg_class_oid; diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 38295aca48..a995a104b6 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10306,10 +10306,6 @@ proname => 'binary_upgrade_set_next_array_pg_type_oid', provolatile => 'v', proparallel => 'r', prorettype => 'void', proargtypes => 'oid', prosrc => 'binary_upgrade_set_next_array_pg_type_oid' }, -{ oid => '3585', descr => 'for use by pg_upgrade', - proname => 'binary_upgrade_set_next_toast_pg_type_oid', provolatile => 'v', - proparallel => 'r', prorettype => 'void', proargtypes => 'oid', - prosrc => 'binary_upgrade_set_next_toast_pg_type_oid' }, { oid => '3586', descr => 'for use by pg_upgrade', proname => 'binary_upgrade_set_next_heap_pg_class_oid', provolatile => 'v', proparallel => 'r', prorettype => 'void', proargtypes => 'oid', diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 828ff5a288..e7f4a5f291 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -1778,6 +1778,7 @@ PLpgSQL_type * plpgsql_parse_wordrowtype(char *ident) { Oid classOid; + Oid typOid;/*
* Look up the relation. Note that because relation rowtypes have the
@@ -1792,8 +1793,16 @@ plpgsql_parse_wordrowtype(char *ident)
(errcode(ERRCODE_UNDEFINED_TABLE),
errmsg("relation \"%s\" does not exist", ident)));+ /* Some relkinds lack type OIDs */ + typOid = get_rel_type_id(classOid); + if (!OidIsValid(typOid)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("relation \"%s\" does not have a composite type", + ident))); + /* Build and return the row type struct */ - return plpgsql_build_datatype(get_rel_type_id(classOid), -1, InvalidOid, + return plpgsql_build_datatype(typOid, -1, InvalidOid, makeTypeName(ident)); }@@ -1806,6 +1815,7 @@ PLpgSQL_type *
plpgsql_parse_cwordrowtype(List *idents)
{
Oid classOid;
+ Oid typOid;
RangeVar *relvar;
MemoryContext oldCxt;@@ -1825,10 +1835,18 @@ plpgsql_parse_cwordrowtype(List *idents) -1); classOid = RangeVarGetRelid(relvar, NoLock, false);+ /* Some relkinds lack type OIDs */ + typOid = get_rel_type_id(classOid); + if (!OidIsValid(typOid)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("relation \"%s\" does not have a composite type", + strVal(lsecond(idents))))); + MemoryContextSwitchTo(oldCxt);/* Build and return the row type struct */ - return plpgsql_build_datatype(get_rel_type_id(classOid), -1, InvalidOid, + return plpgsql_build_datatype(typOid, -1, InvalidOid, makeTypeNameFromNameList(idents)); }