Forking this thread for two tangential patches which I think are more
worthwhile than the original topic's patch.
https://www.postgresql.org/message-id/20200207143935.GP403%40telsasoft.com
Is there a better place to implement assertion from 0002 ?
On Fri, Feb 07, 2020 at 08:39:35AM -0600, Justin Pryzby wrote:
> From 7eea0a17e495fe13379ffd589b551f2f145f5672 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryz...@telsasoft.com>
> Date: Thu, 6 Feb 2020 21:48:13 -0600
> Subject: [PATCH v1 1/3] Update comment obsolete since b9b8831a
>
> ---
> src/backend/commands/cluster.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
> index e9d7a7f..3adcbeb 100644
> --- a/src/backend/commands/cluster.c
> +++ b/src/backend/commands/cluster.c
> @@ -1539,9 +1539,9 @@ get_tables_to_cluster(MemoryContext cluster_context)
>
> /*
> * Get all indexes that have indisclustered set and are owned by
> - * appropriate user. System relations or nailed-in relations cannot ever
> - * have indisclustered set, because CLUSTER will refuse to set it when
> - * called with one of them as argument.
> + * appropriate user. Shared relations cannot ever have indisclustered
> + * set, because CLUSTER will refuse to set it when called with one as
> + * an argument.
> */
> indRelation = table_open(IndexRelationId, AccessShareLock);
> ScanKeyInit(&entry,
> --
> 2.7.4
>
> From 4777be522a7aa8b8c77b13f765cbd02043438f2a Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryz...@telsasoft.com>
> Date: Fri, 7 Feb 2020 08:12:50 -0600
> Subject: [PATCH v1 2/3] Give developer a helpful kick in the pants if they
> change natts in one place but not another
>
> ---
> src/backend/bootstrap/bootstrap.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/src/backend/bootstrap/bootstrap.c
> b/src/backend/bootstrap/bootstrap.c
> index bfc629c..d5e1888 100644
> --- a/src/backend/bootstrap/bootstrap.c
> +++ b/src/backend/bootstrap/bootstrap.c
> @@ -25,7 +25,9 @@
> #include "access/xlog_internal.h"
> #include "bootstrap/bootstrap.h"
> #include "catalog/index.h"
> +#include "catalog/pg_class.h"
> #include "catalog/pg_collation.h"
> +#include "catalog/pg_proc.h"
> #include "catalog/pg_type.h"
> #include "common/link-canary.h"
> #include "libpq/pqsignal.h"
> @@ -49,6 +51,7 @@
> #include "utils/ps_status.h"
> #include "utils/rel.h"
> #include "utils/relmapper.h"
> +#include "utils/syscache.h"
>
> uint32 bootstrap_data_checksum_version = 0; /* No checksum
> */
>
> @@ -602,6 +605,26 @@ boot_openrel(char *relname)
> TableScanDesc scan;
> HeapTuple tup;
>
> + /* Check that pg_class data is consistent now, rather than failing
> obscurely later */
> + struct { Oid oid; int natts; }
> + checknatts[] = {
> + {RelationRelationId, Natts_pg_class,},
> + {TypeRelationId, Natts_pg_type,},
> + {AttributeRelationId, Natts_pg_attribute,},
> + {ProcedureRelationId, Natts_pg_proc,},
> + };
> +
> + for (int i=0; i<lengthof(checknatts); ++i) {
> + Form_pg_class classForm;
> + HeapTuple tuple;
> + tuple = SearchSysCache1(RELOID,
> ObjectIdGetDatum(checknatts[i].oid));
> + if (!HeapTupleIsValid(tuple))
> + elog(ERROR, "cache lookup failed for relation %u",
> checknatts[i].oid);
> + classForm = (Form_pg_class) GETSTRUCT(tuple);
> + Assert(checknatts[i].natts == classForm->relnatts);
> + ReleaseSysCache(tuple);
> + }
> +
> if (strlen(relname) >= NAMEDATALEN)
> relname[NAMEDATALEN - 1] = '\0';
>
> --
> 2.7.4
>
>From 4777be522a7aa8b8c77b13f765cbd02043438f2a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 7 Feb 2020 08:12:50 -0600
Subject: [PATCH v1 2/3] Give developer a helpful kick in the pants if they
change natts in one place but not another
---
src/backend/bootstrap/bootstrap.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index bfc629c..d5e1888 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -25,7 +25,9 @@
#include "access/xlog_internal.h"
#include "bootstrap/bootstrap.h"
#include "catalog/index.h"
+#include "catalog/pg_class.h"
#include "catalog/pg_collation.h"
+#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
#include "common/link-canary.h"
#include "libpq/pqsignal.h"
@@ -49,6 +51,7 @@
#include "utils/ps_status.h"
#include "utils/rel.h"
#include "utils/relmapper.h"
+#include "utils/syscache.h"
uint32 bootstrap_data_checksum_version = 0; /* No checksum */
@@ -602,6 +605,26 @@ boot_openrel(char *relname)
TableScanDesc scan;
HeapTuple tup;
+ /* Check that pg_class data is consistent now, rather than failing obscurely later */
+ struct { Oid oid; int natts; }
+ checknatts[] = {
+ {RelationRelationId, Natts_pg_class,},
+ {TypeRelationId, Natts_pg_type,},
+ {AttributeRelationId, Natts_pg_attribute,},
+ {ProcedureRelationId, Natts_pg_proc,},
+ };
+
+ for (int i=0; i<lengthof(checknatts); ++i) {
+ Form_pg_class classForm;
+ HeapTuple tuple;
+ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(checknatts[i].oid));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", checknatts[i].oid);
+ classForm = (Form_pg_class) GETSTRUCT(tuple);
+ Assert(checknatts[i].natts == classForm->relnatts);
+ ReleaseSysCache(tuple);
+ }
+
if (strlen(relname) >= NAMEDATALEN)
relname[NAMEDATALEN - 1] = '\0';
--
2.7.4
