Обсуждение: [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 data2)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, 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 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, 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 > > 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?) I fixed the problem along your lines. Please check. Wenjing > > -- > Álvaro Herrera https://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
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, 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
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
--
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
--
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 > > 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
Вложения
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] (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
--
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
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
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
> 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 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
>
> 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));
> }
>