pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedget_constraint_index() and conindid
5+ messages / 4 participants
[nested] [flat]
* get_constraint_index() and conindid
@ 2020-12-07 10:09 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 5+ messages in thread
From: Peter Eisentraut @ 2020-12-07 10:09 UTC (permalink / raw)
To: pgsql-hackers
get_constraint_index() does its work by going through pg_depend. It was
added before pg_constraint.conindid was added, and some callers are
still not changed. Are there reasons for that? Probably not. The
attached patch changes get_constraint_index() to an lsyscache-style
lookup instead.
The nearby get_index_constraint() should probably also be changed to
scan pg_constraint instead of pg_depend, but that doesn't have a
syscache to use, so it would be a different approach, so I figured I'd
ask about get_constraint_index() first.
From 29371949b2e61f894f651eef990c3c6eabfa8145 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 7 Dec 2020 10:38:36 +0100
Subject: [PATCH] Change get_constraint_index() to use pg_constraint.conindid
It was still using a scan of pg_depend instead of using the conindid
column that has been added since.
---
src/backend/catalog/pg_depend.c | 69 ----------------------------
src/backend/commands/tablecmds.c | 1 -
src/backend/optimizer/util/plancat.c | 1 -
src/backend/utils/adt/ruleutils.c | 3 +-
src/backend/utils/cache/lsyscache.c | 27 +++++++++++
src/include/catalog/dependency.h | 2 -
src/include/utils/lsyscache.h | 1 +
7 files changed, 29 insertions(+), 75 deletions(-)
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 25290c821f..429791694f 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -968,75 +968,6 @@ getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok)
return linitial_oid(seqlist);
}
-/*
- * get_constraint_index
- * Given the OID of a unique, primary-key, or exclusion constraint,
- * return the OID of the underlying index.
- *
- * Return InvalidOid if the index couldn't be found; this suggests the
- * given OID is bogus, but we leave it to caller to decide what to do.
- */
-Oid
-get_constraint_index(Oid constraintId)
-{
- Oid indexId = InvalidOid;
- Relation depRel;
- ScanKeyData key[3];
- SysScanDesc scan;
- HeapTuple tup;
-
- /* Search the dependency table for the dependent index */
- depRel = table_open(DependRelationId, AccessShareLock);
-
- ScanKeyInit(&key[0],
- Anum_pg_depend_refclassid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ConstraintRelationId));
- ScanKeyInit(&key[1],
- Anum_pg_depend_refobjid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(constraintId));
- ScanKeyInit(&key[2],
- Anum_pg_depend_refobjsubid,
- BTEqualStrategyNumber, F_INT4EQ,
- Int32GetDatum(0));
-
- scan = systable_beginscan(depRel, DependReferenceIndexId, true,
- NULL, 3, key);
-
- while (HeapTupleIsValid(tup = systable_getnext(scan)))
- {
- Form_pg_depend deprec = (Form_pg_depend) GETSTRUCT(tup);
-
- /*
- * We assume any internal dependency of an index on the constraint
- * must be what we are looking for.
- */
- if (deprec->classid == RelationRelationId &&
- deprec->objsubid == 0 &&
- deprec->deptype == DEPENDENCY_INTERNAL)
- {
- char relkind = get_rel_relkind(deprec->objid);
-
- /*
- * This is pure paranoia; there shouldn't be any other relkinds
- * dependent on a constraint.
- */
- if (relkind != RELKIND_INDEX &&
- relkind != RELKIND_PARTITIONED_INDEX)
- continue;
-
- indexId = deprec->objid;
- break;
- }
- }
-
- systable_endscan(scan);
- table_close(depRel, AccessShareLock);
-
- return indexId;
-}
-
/*
* get_index_constraint
* Given the OID of an index, return the OID of the owning unique,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 46f1637e77..1fa9f19f08 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -26,7 +26,6 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
-#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 3e94256d34..daf1759623 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -26,7 +26,6 @@
#include "access/transam.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
-#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/pg_am.h"
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index c2c6df2a4f..ad582f99a5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -24,7 +24,6 @@
#include "access/relation.h"
#include "access/sysattr.h"
#include "access/table.h"
-#include "catalog/dependency.h"
#include "catalog/pg_aggregate.h"
#include "catalog/pg_am.h"
#include "catalog/pg_authid.h"
@@ -2140,7 +2139,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
appendStringInfoChar(&buf, ')');
- indexId = get_constraint_index(constraintId);
+ indexId = conForm->conindid;
/* Build including column list (from pg_index.indkeys) */
indtup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexId));
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index ae23299162..47a8365849 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1094,6 +1094,33 @@ get_constraint_name(Oid conoid)
return NULL;
}
+/*
+ * get_constraint_index
+ * Given the OID of a unique, primary-key, or exclusion constraint,
+ * return the OID of the underlying index.
+ *
+ * Return InvalidOid if the index couldn't be found; this suggests the
+ * given OID is bogus, but we leave it to caller to decide what to do.
+ */
+Oid
+get_constraint_index(Oid conoid)
+{
+ HeapTuple tp;
+
+ tp = SearchSysCache1(CONSTROID, ObjectIdGetDatum(conoid));
+ if (HeapTupleIsValid(tp))
+ {
+ Form_pg_constraint contup = (Form_pg_constraint) GETSTRUCT(tp);
+ Oid result;
+
+ result = contup->conindid;
+ ReleaseSysCache(tp);
+ return result;
+ }
+ else
+ return InvalidOid;
+}
+
/* ---------- LANGUAGE CACHE ---------- */
char *
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 901d5019cd..1fcc4a39ec 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -235,8 +235,6 @@ extern bool sequenceIsOwned(Oid seqId, char deptype, Oid *tableId, int32 *colId)
extern List *getOwnedSequences(Oid relid);
extern Oid getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok);
-extern Oid get_constraint_index(Oid constraintId);
-
extern Oid get_index_constraint(Oid indexId);
extern List *get_index_ref_constraints(Oid indexId);
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index fecfe1f4f6..c97e12dde8 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -96,6 +96,7 @@ extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok);
extern char *get_collation_name(Oid colloid);
extern bool get_collation_isdeterministic(Oid colloid);
extern char *get_constraint_name(Oid conoid);
+extern Oid get_constraint_index(Oid conoid);
extern char *get_language_name(Oid langoid, bool missing_ok);
extern Oid get_opclass_family(Oid opclass);
extern Oid get_opclass_input_type(Oid opclass);
--
2.29.2
Attachments:
[text/plain] 0001-Change-get_constraint_index-to-use-pg_constraint.con.patch (6.7K, ../../4688d55c-9a2e-9a5a-d166-5f24fe0bf8db@enterprisedb.com/2-0001-Change-get_constraint_index-to-use-pg_constraint.con.patch)
download | inline diff:
From 29371949b2e61f894f651eef990c3c6eabfa8145 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 7 Dec 2020 10:38:36 +0100
Subject: [PATCH] Change get_constraint_index() to use pg_constraint.conindid
It was still using a scan of pg_depend instead of using the conindid
column that has been added since.
---
src/backend/catalog/pg_depend.c | 69 ----------------------------
src/backend/commands/tablecmds.c | 1 -
src/backend/optimizer/util/plancat.c | 1 -
src/backend/utils/adt/ruleutils.c | 3 +-
src/backend/utils/cache/lsyscache.c | 27 +++++++++++
src/include/catalog/dependency.h | 2 -
src/include/utils/lsyscache.h | 1 +
7 files changed, 29 insertions(+), 75 deletions(-)
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 25290c821f..429791694f 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -968,75 +968,6 @@ getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok)
return linitial_oid(seqlist);
}
-/*
- * get_constraint_index
- * Given the OID of a unique, primary-key, or exclusion constraint,
- * return the OID of the underlying index.
- *
- * Return InvalidOid if the index couldn't be found; this suggests the
- * given OID is bogus, but we leave it to caller to decide what to do.
- */
-Oid
-get_constraint_index(Oid constraintId)
-{
- Oid indexId = InvalidOid;
- Relation depRel;
- ScanKeyData key[3];
- SysScanDesc scan;
- HeapTuple tup;
-
- /* Search the dependency table for the dependent index */
- depRel = table_open(DependRelationId, AccessShareLock);
-
- ScanKeyInit(&key[0],
- Anum_pg_depend_refclassid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ConstraintRelationId));
- ScanKeyInit(&key[1],
- Anum_pg_depend_refobjid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(constraintId));
- ScanKeyInit(&key[2],
- Anum_pg_depend_refobjsubid,
- BTEqualStrategyNumber, F_INT4EQ,
- Int32GetDatum(0));
-
- scan = systable_beginscan(depRel, DependReferenceIndexId, true,
- NULL, 3, key);
-
- while (HeapTupleIsValid(tup = systable_getnext(scan)))
- {
- Form_pg_depend deprec = (Form_pg_depend) GETSTRUCT(tup);
-
- /*
- * We assume any internal dependency of an index on the constraint
- * must be what we are looking for.
- */
- if (deprec->classid == RelationRelationId &&
- deprec->objsubid == 0 &&
- deprec->deptype == DEPENDENCY_INTERNAL)
- {
- char relkind = get_rel_relkind(deprec->objid);
-
- /*
- * This is pure paranoia; there shouldn't be any other relkinds
- * dependent on a constraint.
- */
- if (relkind != RELKIND_INDEX &&
- relkind != RELKIND_PARTITIONED_INDEX)
- continue;
-
- indexId = deprec->objid;
- break;
- }
- }
-
- systable_endscan(scan);
- table_close(depRel, AccessShareLock);
-
- return indexId;
-}
-
/*
* get_index_constraint
* Given the OID of an index, return the OID of the owning unique,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 46f1637e77..1fa9f19f08 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -26,7 +26,6 @@
#include "access/xact.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
-#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 3e94256d34..daf1759623 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -26,7 +26,6 @@
#include "access/transam.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
-#include "catalog/dependency.h"
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/pg_am.h"
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index c2c6df2a4f..ad582f99a5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -24,7 +24,6 @@
#include "access/relation.h"
#include "access/sysattr.h"
#include "access/table.h"
-#include "catalog/dependency.h"
#include "catalog/pg_aggregate.h"
#include "catalog/pg_am.h"
#include "catalog/pg_authid.h"
@@ -2140,7 +2139,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
appendStringInfoChar(&buf, ')');
- indexId = get_constraint_index(constraintId);
+ indexId = conForm->conindid;
/* Build including column list (from pg_index.indkeys) */
indtup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexId));
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index ae23299162..47a8365849 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1094,6 +1094,33 @@ get_constraint_name(Oid conoid)
return NULL;
}
+/*
+ * get_constraint_index
+ * Given the OID of a unique, primary-key, or exclusion constraint,
+ * return the OID of the underlying index.
+ *
+ * Return InvalidOid if the index couldn't be found; this suggests the
+ * given OID is bogus, but we leave it to caller to decide what to do.
+ */
+Oid
+get_constraint_index(Oid conoid)
+{
+ HeapTuple tp;
+
+ tp = SearchSysCache1(CONSTROID, ObjectIdGetDatum(conoid));
+ if (HeapTupleIsValid(tp))
+ {
+ Form_pg_constraint contup = (Form_pg_constraint) GETSTRUCT(tp);
+ Oid result;
+
+ result = contup->conindid;
+ ReleaseSysCache(tp);
+ return result;
+ }
+ else
+ return InvalidOid;
+}
+
/* ---------- LANGUAGE CACHE ---------- */
char *
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 901d5019cd..1fcc4a39ec 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -235,8 +235,6 @@ extern bool sequenceIsOwned(Oid seqId, char deptype, Oid *tableId, int32 *colId)
extern List *getOwnedSequences(Oid relid);
extern Oid getIdentitySequence(Oid relid, AttrNumber attnum, bool missing_ok);
-extern Oid get_constraint_index(Oid constraintId);
-
extern Oid get_index_constraint(Oid indexId);
extern List *get_index_ref_constraints(Oid indexId);
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index fecfe1f4f6..c97e12dde8 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -96,6 +96,7 @@ extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok);
extern char *get_collation_name(Oid colloid);
extern bool get_collation_isdeterministic(Oid colloid);
extern char *get_constraint_name(Oid conoid);
+extern Oid get_constraint_index(Oid conoid);
extern char *get_language_name(Oid langoid, bool missing_ok);
extern Oid get_opclass_family(Oid opclass);
extern Oid get_opclass_input_type(Oid opclass);
--
2.29.2
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: get_constraint_index() and conindid
@ 2020-12-08 14:52 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 5+ messages in thread
From: Matthias van de Meent @ 2020-12-08 14:52 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: pgsql-hackers
On Mon, 7 Dec 2020 at 11:09, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> get_constraint_index() does its work by going through pg_depend. It was
> added before pg_constraint.conindid was added, and some callers are
> still not changed. Are there reasons for that? Probably not. The
> attached patch changes get_constraint_index() to an lsyscache-style
> lookup instead.
This looks quite reasonable, and it passes "make installcheck-world".
Only thing I could think of is that it maybe could use a (small)
comment in the message on that/why get_constraint_index is moved to
utils/lsyscache from catalog/dependency, as that took me some time to
understand.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: get_constraint_index() and conindid
@ 2020-12-08 18:28 Tom Lane <tgl@sss.pgh.pa.us>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2020-12-08 18:28 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
> On Mon, 7 Dec 2020 at 11:09, Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
>> get_constraint_index() does its work by going through pg_depend. It was
>> added before pg_constraint.conindid was added, and some callers are
>> still not changed. Are there reasons for that? Probably not. The
>> attached patch changes get_constraint_index() to an lsyscache-style
>> lookup instead.
> This looks quite reasonable, and it passes "make installcheck-world".
+1, LGTM.
> Only thing I could think of is that it maybe could use a (small)
> comment in the message on that/why get_constraint_index is moved to
> utils/lsyscache from catalog/dependency, as that took me some time to
> understand.
commit message could reasonably say that maybe, but I don't think we
need to memorialize it in a comment. lsyscache.c *is* where one
would expect to find a simple catalog-field-fetch function like this.
The previous implementation was not that, so it didn't belong there.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: get_constraint_index() and conindid
@ 2020-12-09 06:37 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2020-12-09 06:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-hackers
On Tue, Dec 08, 2020 at 01:28:39PM -0500, Tom Lane wrote:
> Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
>> On Mon, 7 Dec 2020 at 11:09, Peter Eisentraut
>> <peter.eisentraut@enterprisedb.com> wrote:
>>> get_constraint_index() does its work by going through pg_depend. It was
>>> added before pg_constraint.conindid was added, and some callers are
>>> still not changed. Are there reasons for that? Probably not. The
>>> attached patch changes get_constraint_index() to an lsyscache-style
>>> lookup instead.
>
>> This looks quite reasonable, and it passes "make installcheck-world".
>
> +1, LGTM.
Nice cleanup!
>> Only thing I could think of is that it maybe could use a (small)
>> comment in the message on that/why get_constraint_index is moved to
>> utils/lsyscache from catalog/dependency, as that took me some time to
>> understand.
>
> commit message could reasonably say that maybe, but I don't think we
> need to memorialize it in a comment. lsyscache.c *is* where one
> would expect to find a simple catalog-field-fetch function like this.
> The previous implementation was not that, so it didn't belong there.
Agreed.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../X9BwvuLuWJrIreQE@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: get_constraint_index() and conindid
@ 2020-12-09 14:50 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 5+ messages in thread
From: Peter Eisentraut @ 2020-12-09 14:50 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; pgsql-hackers
On 2020-12-09 07:37, Michael Paquier wrote:
>>> Only thing I could think of is that it maybe could use a (small)
>>> comment in the message on that/why get_constraint_index is moved to
>>> utils/lsyscache from catalog/dependency, as that took me some time to
>>> understand.
>>
>> commit message could reasonably say that maybe, but I don't think we
>> need to memorialize it in a comment. lsyscache.c *is* where one
>> would expect to find a simple catalog-field-fetch function like this.
>> The previous implementation was not that, so it didn't belong there.
>
> Agreed.
Thanks, I committed it with an expanded commit message.
After further inspection, I'm not going to do anything about the nearby
get_index_constraint() at this item. The current implementation can use
an index on pg_depend. A scan of pg_constraint has no index available.
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2020-12-09 14:50 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 10:09 get_constraint_index() and conindid Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2020-12-08 14:52 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2020-12-08 18:28 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-12-09 06:37 ` Michael Paquier <michael@paquier.xyz>
2020-12-09 14:50 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox