remove pg_class.relhaspkey
pg_class.relhaspkey doesn't seem to be used or useful for anything, so
can we remove it? See attached patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Remove-pg_class.relhaspkey.patchtext/plain; charset=UTF-8; name=0001-Remove-pg_class.relhaspkey.patch; x-mac-creator=0; x-mac-type=0Download
From 01004383b0a3d30d519b9dc219b54c8d753df8ee Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sat, 24 Feb 2018 21:25:08 -0500
Subject: [PATCH] Remove pg_class.relhaspkey
It's not used for anything internally, and it can't be relied on for
external uses, so it can just be removed.
---
doc/src/sgml/catalogs.sgml | 9 ---------
src/backend/catalog/heap.c | 1 -
src/backend/catalog/index.c | 32 ++-----------------------------
src/backend/commands/vacuum.c | 10 ----------
src/backend/rewrite/rewriteDefine.c | 1 -
src/include/catalog/pg_class.h | 38 ++++++++++++++++++-------------------
6 files changed, 20 insertions(+), 71 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 71e20f2740..b93eef4efc 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1848,15 +1848,6 @@ <title><structname>pg_class</structname> Columns</title>
</entry>
</row>
- <row>
- <entry><structfield>relhaspkey</structfield></entry>
- <entry><type>bool</type></entry>
- <entry></entry>
- <entry>
- True if the table has (or once had) a primary key
- </entry>
- </row>
-
<row>
<entry><structfield>relhasrules</structfield></entry>
<entry><type>bool</type></entry>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index cf36ce4add..3d80ff9e5b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -798,7 +798,6 @@ InsertPgClassTuple(Relation pg_class_desc,
values[Anum_pg_class_relnatts - 1] = Int16GetDatum(rd_rel->relnatts);
values[Anum_pg_class_relchecks - 1] = Int16GetDatum(rd_rel->relchecks);
values[Anum_pg_class_relhasoids - 1] = BoolGetDatum(rd_rel->relhasoids);
- values[Anum_pg_class_relhaspkey - 1] = BoolGetDatum(rd_rel->relhaspkey);
values[Anum_pg_class_relhasrules - 1] = BoolGetDatum(rd_rel->relhasrules);
values[Anum_pg_class_relhastriggers - 1] = BoolGetDatum(rd_rel->relhastriggers);
values[Anum_pg_class_relrowsecurity - 1] = BoolGetDatum(rd_rel->relrowsecurity);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 564f2069cf..6268c10e11 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -125,7 +125,7 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
bool isvalid,
bool isready);
static void index_update_stats(Relation rel,
- bool hasindex, bool isprimary,
+ bool hasindex,
double reltuples);
static void IndexCheckExclusion(Relation heapRelation,
Relation indexRelation,
@@ -1162,7 +1162,6 @@ index_create(Relation heapRelation,
*/
index_update_stats(heapRelation,
true,
- isprimary,
-1.0);
/* Make the above update visible */
CommandCounterIncrement();
@@ -1364,21 +1363,6 @@ index_constraint_create(Relation heapRelation,
InvalidOid, conOid, indexRelationId, true);
}
- /*
- * If needed, mark the table as having a primary key. We assume it can't
- * have been so marked already, so no need to clear the flag in the other
- * case.
- *
- * Note: this might better be done by callers. We do it here to avoid
- * exposing index_update_stats() globally, but that wouldn't be necessary
- * if relhaspkey went away.
- */
- if (mark_as_primary)
- index_update_stats(heapRelation,
- true,
- true,
- -1.0);
-
/*
* If needed, mark the index as primary and/or deferred in pg_index.
*
@@ -2041,7 +2025,6 @@ FormIndexDatum(IndexInfo *indexInfo,
* to ensure we can do all the necessary work in just one update.
*
* hasindex: set relhasindex to this value
- * isprimary: if true, set relhaspkey true; else no change
* reltuples: if >= 0, set reltuples to this value; else no change
*
* If reltuples >= 0, relpages and relallvisible are also updated (using
@@ -2058,7 +2041,6 @@ FormIndexDatum(IndexInfo *indexInfo,
static void
index_update_stats(Relation rel,
bool hasindex,
- bool isprimary,
double reltuples)
{
Oid relid = RelationGetRelid(rel);
@@ -2088,7 +2070,7 @@ index_update_stats(Relation rel,
* It is safe to use a non-transactional update even though our
* transaction could still fail before committing. Setting relhasindex
* true is safe even if there are no indexes (VACUUM will eventually fix
- * it), likewise for relhaspkey. And of course the new relpages and
+ * it). And of course the new relpages and
* reltuples counts are correct regardless. However, we don't want to
* change relpages (or relallvisible) if the caller isn't providing an
* updated reltuples count, because that would bollix the
@@ -2140,14 +2122,6 @@ index_update_stats(Relation rel,
rd_rel->relhasindex = hasindex;
dirty = true;
}
- if (isprimary)
- {
- if (!rd_rel->relhaspkey)
- {
- rd_rel->relhaspkey = true;
- dirty = true;
- }
- }
if (reltuples >= 0)
{
@@ -2356,11 +2330,9 @@ index_build(Relation heapRelation,
*/
index_update_stats(heapRelation,
true,
- isprimary,
stats->heap_tuples);
index_update_stats(indexRelation,
- false,
false,
stats->index_tuples);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7aca69a0ba..1fad073192 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -929,16 +929,6 @@ vac_update_relstats(Relation relation,
dirty = true;
}
- /*
- * If we have discovered that there are no indexes, then there's no
- * primary key either. This could be done more thoroughly...
- */
- if (pgcform->relhaspkey && !hasindex)
- {
- pgcform->relhaspkey = false;
- dirty = true;
- }
-
/* We also clear relhasrules and relhastriggers if needed */
if (pgcform->relhasrules && relation->rd_rules == NULL)
{
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index f3a9b639a8..679be605f1 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -618,7 +618,6 @@ DefineQueryRewrite(const char *rulename,
classForm->relhasindex = false;
classForm->relkind = RELKIND_VIEW;
classForm->relhasoids = false;
- classForm->relhaspkey = false;
classForm->relfrozenxid = InvalidTransactionId;
classForm->relminmxid = InvalidMultiXactId;
classForm->relreplident = REPLICA_IDENTITY_NOTHING;
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 26b1866c69..97026bfc2e 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -61,7 +61,6 @@ CATALOG(pg_class,1259) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83) BKI_SCHEMA_MACRO
*/
int16 relchecks; /* # of CHECK constraints for class */
bool relhasoids; /* T if we generate OIDs for rows of rel */
- bool relhaspkey; /* has (or has had) PRIMARY KEY index */
bool relhasrules; /* has (or has had) any rules */
bool relhastriggers; /* has (or has had) any TRIGGERs */
bool relhassubclass; /* has (or has had) derived classes */
@@ -99,7 +98,7 @@ typedef FormData_pg_class *Form_pg_class;
* ----------------
*/
-#define Natts_pg_class 33
+#define Natts_pg_class 32
#define Anum_pg_class_relname 1
#define Anum_pg_class_relnamespace 2
#define Anum_pg_class_reltype 3
@@ -119,20 +118,19 @@ typedef FormData_pg_class *Form_pg_class;
#define Anum_pg_class_relnatts 17
#define Anum_pg_class_relchecks 18
#define Anum_pg_class_relhasoids 19
-#define Anum_pg_class_relhaspkey 20
-#define Anum_pg_class_relhasrules 21
-#define Anum_pg_class_relhastriggers 22
-#define Anum_pg_class_relhassubclass 23
-#define Anum_pg_class_relrowsecurity 24
-#define Anum_pg_class_relforcerowsecurity 25
-#define Anum_pg_class_relispopulated 26
-#define Anum_pg_class_relreplident 27
-#define Anum_pg_class_relispartition 28
-#define Anum_pg_class_relfrozenxid 29
-#define Anum_pg_class_relminmxid 30
-#define Anum_pg_class_relacl 31
-#define Anum_pg_class_reloptions 32
-#define Anum_pg_class_relpartbound 33
+#define Anum_pg_class_relhasrules 20
+#define Anum_pg_class_relhastriggers 21
+#define Anum_pg_class_relhassubclass 22
+#define Anum_pg_class_relrowsecurity 23
+#define Anum_pg_class_relforcerowsecurity 24
+#define Anum_pg_class_relispopulated 25
+#define Anum_pg_class_relreplident 26
+#define Anum_pg_class_relispartition 27
+#define Anum_pg_class_relfrozenxid 28
+#define Anum_pg_class_relminmxid 29
+#define Anum_pg_class_relacl 30
+#define Anum_pg_class_reloptions 31
+#define Anum_pg_class_relpartbound 32
/* ----------------
* initial contents of pg_class
@@ -147,13 +145,13 @@ typedef FormData_pg_class *Form_pg_class;
* Note: "3" in the relfrozenxid column stands for FirstNormalTransactionId;
* similarly, "1" in relminmxid stands for FirstMultiXactId
*/
-DATA(insert OID = 1247 ( pg_type PGNSP 71 0 PGUID 0 0 0 0 0 0 0 f f p r 30 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1247 ( pg_type PGNSP 71 0 PGUID 0 0 0 0 0 0 0 f f p r 30 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1249 ( pg_attribute PGNSP 75 0 PGUID 0 0 0 0 0 0 0 f f p r 22 0 f f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1249 ( pg_attribute PGNSP 75 0 PGUID 0 0 0 0 0 0 0 f f p r 22 0 f f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 29 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 29 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 33 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 32 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
--
2.16.2
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
pg_class.relhaspkey doesn't seem to be used or useful for anything, so
can we remove it? See attached patch.
We've discussed that at least twice before, and not pulled the trigger
for fear of breaking client code.
/messages/by-id/CAA-aLv7sszmU+Fz-xLo6cOiASUiX0mCRwAMF2FB=2j-5MKqb+A@mail.gmail.com
/messages/by-id/20140317185255.20724.49675@wrigleys.postgresql.org
Not sure that the situation has changed any since then ... it still
comes down to whether we want an API break for client code.
regards, tom lane
On Sat, Feb 24, 2018 at 10:21:44PM -0500, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
pg_class.relhaspkey doesn't seem to be used or useful for anything, so
can we remove it? See attached patch.We've discussed that at least twice before, and not pulled the trigger
for fear of breaking client code./messages/by-id/CAA-aLv7sszmU+Fz-xLo6cOiASUiX0mCRwAMF2FB=2j-5MKqb+A@mail.gmail.com
/messages/by-id/20140317185255.20724.49675@wrigleys.postgresql.orgNot sure that the situation has changed any since then ... it still
comes down to whether we want an API break for client code.
I would be of the opinion to drop them. Even if some client code rely
on this information, it means that they also rely on some information
which may be incorrect if those are not updated when they should be for
certain DDL combinations. And in my opinion that's bad. It seems to me
that if we would want to do some cleanup, this should happen for all the
flags that are updated in a lazy fashion. For most of them, it would be
easy enough to replace them by subqueries which scan other catalog
information for correct and actually up-to-date information. So in this
category enter as well relhassubclass, relhastriggers and relhasrules.
Speaking of which, I have looked at where relhaspkey is being used. And
there are a couple of things using it:
- Greenplum has a consistency checker tool using it.
- https://github.com/no0p/pgsampler
- https://searchcode.com/codesearch/view/54937539/
- http://codegist.net/code/postgres%20drop%20tables/
- https://hackage.haskell.org/package/relational-schemas-0.1.3.4/src/src/Database/Relational/Schema/PgCatalog/PgClass.hs
So the answer is yes, there would be code breakages if those get
removed. Still I would imagine that such applications ought to use
subquery patterns and not rely on the existing flags, as this causes in
the worst case extra round trips between the server and the client as
the client is unsure if the information is up-to-date or not.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Sat, Feb 24, 2018 at 10:21:44PM -0500, Tom Lane wrote:
We've discussed that at least twice before, and not pulled the trigger
for fear of breaking client code.
Speaking of which, I have looked at where relhaspkey is being used. And
there are a couple of things using it:
- Greenplum has a consistency checker tool using it.
- https://github.com/no0p/pgsampler
- https://searchcode.com/codesearch/view/54937539/
- http://codegist.net/code/postgres%20drop%20tables/
- https://hackage.haskell.org/package/relational-schemas-0.1.3.4/src/src/Database/Relational/Schema/PgCatalog/PgClass.hs
Thanks for poking around. Did you happen to notice how many of these
clients are taking pains to deal with the existing inaccuracy of
relhaspkey (ie, that it remains set after the pkey has been dropped)?
I think there's possibly an argument that relhaspkey should be dropped
because it's an attractive nuisance, encouraging clients to assume
more than they should about what it means. But we don't have a lot
of evidence for such an argument right now.
regards, tom lane
On Mon, Feb 26, 2018 at 12:45:48AM -0500, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
On Sat, Feb 24, 2018 at 10:21:44PM -0500, Tom Lane wrote:
We've discussed that at least twice before, and not pulled the trigger
for fear of breaking client code.Speaking of which, I have looked at where relhaspkey is being used. And
there are a couple of things using it:
- Greenplum has a consistency checker tool using it.
- https://github.com/no0p/pgsampler
- https://searchcode.com/codesearch/view/54937539/
- http://codegist.net/code/postgres%20drop%20tables/
- https://hackage.haskell.org/package/relational-schemas-0.1.3.4/src/src/Database/Relational/Schema/PgCatalog/PgClass.hsThanks for poking around. Did you happen to notice how many of these
clients are taking pains to deal with the existing inaccuracy of
relhaspkey (ie, that it remains set after the pkey has been dropped)?
As far as I looked at things. Those clients rely on how optimistic
relhaspkey is. In short, if it is set to true, there can be primary
key definitions. If set to false, then it is sure that no primary key
definition can be found. If the flag is true, then those clients just
do an extra lookup on pg_index with indisprimary. I think that this
just complicates the code involved though. If looking for primary keys
it is way better to just scan directly pg_index.
I think there's possibly an argument that relhaspkey should be dropped
because it's an attractive nuisance, encouraging clients to assume
more than they should about what it means. But we don't have a lot
of evidence for such an argument right now.
The only breakage I could imagine here is an application which thinks
relhaspkey set to true implies that a primary key *has* to be present.
I have to admit that I have not found such a case. Still I would not be
surprised if there are such applications unaware of being broken,
particularly plpgsql functions.
--
Michael
On Mon, Feb 26, 2018 at 12:36 AM, Michael Paquier <michael@paquier.xyz> wrote:
I would be of the opinion to drop them.
+1. On this point, I am in agreement with the gentleman who wrote
/messages/by-id/7903.1310671199@sss.pgh.pa.us
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 02/26/2018 07:23 AM, Michael Paquier wrote:
On Mon, Feb 26, 2018 at 12:45:48AM -0500, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
On Sat, Feb 24, 2018 at 10:21:44PM -0500, Tom Lane wrote:
We've discussed that at least twice before, and not pulled the trigger
for fear of breaking client code.Speaking of which, I have looked at where relhaspkey is being used. And
there are a couple of things using it:
- Greenplum has a consistency checker tool using it.
- https://github.com/no0p/pgsampler
- https://searchcode.com/codesearch/view/54937539/
- http://codegist.net/code/postgres%20drop%20tables/
- https://hackage.haskell.org/package/relational-schemas-0.1.3.4/src/src/Database/Relational/Schema/PgCatalog/PgClass.hsThanks for poking around. Did you happen to notice how many of these
clients are taking pains to deal with the existing inaccuracy of
relhaspkey (ie, that it remains set after the pkey has been dropped)?As far as I looked at things. Those clients rely on how optimistic
relhaspkey is. In short, if it is set to true, there can be primary
key definitions. If set to false, then it is sure that no primary key
definition can be found. If the flag is true, then those clients just
do an extra lookup on pg_index with indisprimary. I think that this
just complicates the code involved though. If looking for primary keys
it is way better to just scan directly pg_index.I think there's possibly an argument that relhaspkey should be dropped
because it's an attractive nuisance, encouraging clients to assume
more than they should about what it means. But we don't have a lot
of evidence for such an argument right now.The only breakage I could imagine here is an application which
thinks relhaspkey set to true implies that a primary key *has* to be
present. I have to admit that I have not found such a case. Still I
would not be surprised if there are such applications unaware of
being broken, particularly plpgsql functions.
I agree with this sentiment - I don't think those flags are particularly
helpful for client applications, and would vote +1 for removal.
Even if the application handles them correctly (i.e. rechecks existence
when relhaspkey=true), the assumption is that this saves a measurable
amount of work. I'm not so sure about that, considering pretty much all
tables have both primary keys and indexes. OTOH it certainly does make
the code more complicated.
For internal usage it might have made a difference back when those flags
were introduced, but the relcache should deal with this efficiently now,
as pointed out in [1]/messages/by-id/CA+TgmoYJu24Y8uUAJ4zeQAhoYxLmFxcy+5Hdij9ehjoxKo3j3g@mail.gmail.com. But as pointed out, we're not using relhaspkey
internally at all. So +1 to get rid of it.
For the other flags we would probably need to test what impact would it
have (e.g. table with no indexes, many indexes on other tables, and
something calling get_relation_info a lot). But this patch proposes to
remove only relhaspkey.
[1]: /messages/by-id/CA+TgmoYJu24Y8uUAJ4zeQAhoYxLmFxcy+5Hdij9ehjoxKo3j3g@mail.gmail.com
/messages/by-id/CA+TgmoYJu24Y8uUAJ4zeQAhoYxLmFxcy+5Hdij9ehjoxKo3j3g@mail.gmail.com
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Mar 10, 2018 at 01:52:56PM +0100, Tomas Vondra wrote:
I agree with this sentiment - I don't think those flags are particularly
helpful for client applications, and would vote +1 for removal.
OK, so I can see that we are moving to a consensus here.
For the other flags we would probably need to test what impact would it
have (e.g. table with no indexes, many indexes on other tables, and
something calling get_relation_info a lot). But this patch proposes to
remove only relhaspkey.
Yes, you are right here. Peter, would you do that within this commit
fest or not? As we are half-way through the commit fest, we could also
cut the apple in half and just remove relhaspkey for now as that's a
no-brainer. So I would suggest to just do the latter and consider this
patch as done.
Attached is a rebased patch, there were some conflicts in pg_class.h by
the way.
--
Michael
Attachments:
0001-Remove-pg_class.relhaspkey.patchtext/x-diff; charset=us-asciiDownload
From 1601b84f9ef2e8d0467b109c0b1d3df435edb59a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 14 Mar 2018 14:46:43 +0900
Subject: [PATCH] Remove pg_class.relhaspkey
It's not used for anything internally, and it can't be relied on for
external uses, so it can just be removed.
Patch by Peter Eisentraut.
---
doc/src/sgml/catalogs.sgml | 9 ---------
src/backend/catalog/heap.c | 1 -
src/backend/catalog/index.c | 32 ++-----------------------------
src/backend/commands/vacuum.c | 10 ----------
src/backend/rewrite/rewriteDefine.c | 1 -
src/include/catalog/pg_class.h | 38 ++++++++++++++++++-------------------
6 files changed, 20 insertions(+), 71 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a0e6d7062b..30e6741305 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1848,15 +1848,6 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</entry>
</row>
- <row>
- <entry><structfield>relhaspkey</structfield></entry>
- <entry><type>bool</type></entry>
- <entry></entry>
- <entry>
- True if the table has (or once had) a primary key
- </entry>
- </row>
-
<row>
<entry><structfield>relhasrules</structfield></entry>
<entry><type>bool</type></entry>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index cf36ce4add..3d80ff9e5b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -798,7 +798,6 @@ InsertPgClassTuple(Relation pg_class_desc,
values[Anum_pg_class_relnatts - 1] = Int16GetDatum(rd_rel->relnatts);
values[Anum_pg_class_relchecks - 1] = Int16GetDatum(rd_rel->relchecks);
values[Anum_pg_class_relhasoids - 1] = BoolGetDatum(rd_rel->relhasoids);
- values[Anum_pg_class_relhaspkey - 1] = BoolGetDatum(rd_rel->relhaspkey);
values[Anum_pg_class_relhasrules - 1] = BoolGetDatum(rd_rel->relhasrules);
values[Anum_pg_class_relhastriggers - 1] = BoolGetDatum(rd_rel->relhastriggers);
values[Anum_pg_class_relrowsecurity - 1] = BoolGetDatum(rd_rel->relrowsecurity);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 431bc31969..9e2dd0e729 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -125,7 +125,7 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
bool isvalid,
bool isready);
static void index_update_stats(Relation rel,
- bool hasindex, bool isprimary,
+ bool hasindex,
double reltuples);
static void IndexCheckExclusion(Relation heapRelation,
Relation indexRelation,
@@ -1162,7 +1162,6 @@ index_create(Relation heapRelation,
*/
index_update_stats(heapRelation,
true,
- isprimary,
-1.0);
/* Make the above update visible */
CommandCounterIncrement();
@@ -1364,21 +1363,6 @@ index_constraint_create(Relation heapRelation,
InvalidOid, conOid, indexRelationId, true);
}
- /*
- * If needed, mark the table as having a primary key. We assume it can't
- * have been so marked already, so no need to clear the flag in the other
- * case.
- *
- * Note: this might better be done by callers. We do it here to avoid
- * exposing index_update_stats() globally, but that wouldn't be necessary
- * if relhaspkey went away.
- */
- if (mark_as_primary)
- index_update_stats(heapRelation,
- true,
- true,
- -1.0);
-
/*
* If needed, mark the index as primary and/or deferred in pg_index.
*
@@ -2041,7 +2025,6 @@ FormIndexDatum(IndexInfo *indexInfo,
* to ensure we can do all the necessary work in just one update.
*
* hasindex: set relhasindex to this value
- * isprimary: if true, set relhaspkey true; else no change
* reltuples: if >= 0, set reltuples to this value; else no change
*
* If reltuples >= 0, relpages and relallvisible are also updated (using
@@ -2058,7 +2041,6 @@ FormIndexDatum(IndexInfo *indexInfo,
static void
index_update_stats(Relation rel,
bool hasindex,
- bool isprimary,
double reltuples)
{
Oid relid = RelationGetRelid(rel);
@@ -2088,7 +2070,7 @@ index_update_stats(Relation rel,
* It is safe to use a non-transactional update even though our
* transaction could still fail before committing. Setting relhasindex
* true is safe even if there are no indexes (VACUUM will eventually fix
- * it), likewise for relhaspkey. And of course the new relpages and
+ * it). And of course the new relpages and
* reltuples counts are correct regardless. However, we don't want to
* change relpages (or relallvisible) if the caller isn't providing an
* updated reltuples count, because that would bollix the
@@ -2140,14 +2122,6 @@ index_update_stats(Relation rel,
rd_rel->relhasindex = hasindex;
dirty = true;
}
- if (isprimary)
- {
- if (!rd_rel->relhaspkey)
- {
- rd_rel->relhaspkey = true;
- dirty = true;
- }
- }
if (reltuples >= 0)
{
@@ -2356,11 +2330,9 @@ index_build(Relation heapRelation,
*/
index_update_stats(heapRelation,
true,
- isprimary,
stats->heap_tuples);
index_update_stats(indexRelation,
- false,
false,
stats->index_tuples);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7aca69a0ba..1fad073192 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -929,16 +929,6 @@ vac_update_relstats(Relation relation,
dirty = true;
}
- /*
- * If we have discovered that there are no indexes, then there's no
- * primary key either. This could be done more thoroughly...
- */
- if (pgcform->relhaspkey && !hasindex)
- {
- pgcform->relhaspkey = false;
- dirty = true;
- }
-
/* We also clear relhasrules and relhastriggers if needed */
if (pgcform->relhasrules && relation->rd_rules == NULL)
{
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index f3a9b639a8..679be605f1 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -618,7 +618,6 @@ DefineQueryRewrite(const char *rulename,
classForm->relhasindex = false;
classForm->relkind = RELKIND_VIEW;
classForm->relhasoids = false;
- classForm->relhaspkey = false;
classForm->relfrozenxid = InvalidTransactionId;
classForm->relminmxid = InvalidMultiXactId;
classForm->relreplident = REPLICA_IDENTITY_NOTHING;
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 01cf59e7a3..7fc355acb8 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -61,7 +61,6 @@ CATALOG(pg_class,1259) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83) BKI_SCHEMA_MACRO
*/
int16 relchecks; /* # of CHECK constraints for class */
bool relhasoids; /* T if we generate OIDs for rows of rel */
- bool relhaspkey; /* has (or has had) PRIMARY KEY index */
bool relhasrules; /* has (or has had) any rules */
bool relhastriggers; /* has (or has had) any TRIGGERs */
bool relhassubclass; /* has (or has had) derived classes */
@@ -99,7 +98,7 @@ typedef FormData_pg_class *Form_pg_class;
* ----------------
*/
-#define Natts_pg_class 33
+#define Natts_pg_class 32
#define Anum_pg_class_relname 1
#define Anum_pg_class_relnamespace 2
#define Anum_pg_class_reltype 3
@@ -119,20 +118,19 @@ typedef FormData_pg_class *Form_pg_class;
#define Anum_pg_class_relnatts 17
#define Anum_pg_class_relchecks 18
#define Anum_pg_class_relhasoids 19
-#define Anum_pg_class_relhaspkey 20
-#define Anum_pg_class_relhasrules 21
-#define Anum_pg_class_relhastriggers 22
-#define Anum_pg_class_relhassubclass 23
-#define Anum_pg_class_relrowsecurity 24
-#define Anum_pg_class_relforcerowsecurity 25
-#define Anum_pg_class_relispopulated 26
-#define Anum_pg_class_relreplident 27
-#define Anum_pg_class_relispartition 28
-#define Anum_pg_class_relfrozenxid 29
-#define Anum_pg_class_relminmxid 30
-#define Anum_pg_class_relacl 31
-#define Anum_pg_class_reloptions 32
-#define Anum_pg_class_relpartbound 33
+#define Anum_pg_class_relhasrules 20
+#define Anum_pg_class_relhastriggers 21
+#define Anum_pg_class_relhassubclass 22
+#define Anum_pg_class_relrowsecurity 23
+#define Anum_pg_class_relforcerowsecurity 24
+#define Anum_pg_class_relispopulated 25
+#define Anum_pg_class_relreplident 26
+#define Anum_pg_class_relispartition 27
+#define Anum_pg_class_relfrozenxid 28
+#define Anum_pg_class_relminmxid 29
+#define Anum_pg_class_relacl 30
+#define Anum_pg_class_reloptions 31
+#define Anum_pg_class_relpartbound 32
/* ----------------
* initial contents of pg_class
@@ -147,13 +145,13 @@ typedef FormData_pg_class *Form_pg_class;
* Note: "3" in the relfrozenxid column stands for FirstNormalTransactionId;
* similarly, "1" in relminmxid stands for FirstMultiXactId
*/
-DATA(insert OID = 1247 ( pg_type PGNSP 71 0 PGUID 0 0 0 0 0 0 0 f f p r 30 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1247 ( pg_type PGNSP 71 0 PGUID 0 0 0 0 0 0 0 f f p r 30 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1249 ( pg_attribute PGNSP 75 0 PGUID 0 0 0 0 0 0 0 f f p r 22 0 f f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1249 ( pg_attribute PGNSP 75 0 PGUID 0 0 0 0 0 0 0 f f p r 22 0 f f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 28 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 28 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 33 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 32 0 t f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
--
2.16.2
On 3/14/18 01:47, Michael Paquier wrote:
For the other flags we would probably need to test what impact would it
have (e.g. table with no indexes, many indexes on other tables, and
something calling get_relation_info a lot). But this patch proposes to
remove only relhaspkey.Yes, you are right here. Peter, would you do that within this commit
fest or not?
Not terribly interested in those other ones.
As we are half-way through the commit fest, we could also
cut the apple in half and just remove relhaspkey for now as that's a
no-brainer. So I would suggest to just do the latter and consider this
patch as done.
done
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services