I've been working on integrating Mark's "Index AM API cleanup" patch set
with the existing gist strategy number mapping from Paul's application
time patch set. Here is what I've come up with.
The previously committed patch (v19.1) already changed the gist strategy
number mapping to use the (Row)CompareType, as in Mark's proposal.
In this patch set, I'm attaching the existing standalone gist translate
function as the index AM API function for the gist index AM. And then
all existing callers are changed to call through the index AM API
functions provided by Mark's patch set.
Patches 0001, 0002, 0003 are some preparatory renaming and refactoring
patches.
Patches 0004 and 0005 are patch v19-0008 from Mark's (via Andrew) v19
patch set, split into two patches, and with some function renaming from
my side.
Patch 0006 then pulls it all together. The key change is that we also
need to pass the opclass to the index AM API functions, so that access
methods like gist can use it. Actually, I changed that to pass opfamily
and opcintype instead. I think this matches better with the rest of the
"Index AM API cleanup" patch set, because it's more common to have the
opfamily and type handy than the opclass. (And internally, the gist
support function is attached to the opfamily anyway, so it's actually
simpler that way.)
I think this fits together quite well now. Several places where gist
was hardcoded are now fully (or mostly) independent of gist. Also, the
somewhat hackish get_equal_strategy_number() in the logical replication
code disappears completely and is replaced by a proper index AM API
function. (This also takes care of patch v19-0011 from Mark's patch set.)
Also, since we have already built out test coverage for the
GistTranslate* stuff, the new index-AM-level translate functionality
gets to use this test coverage for free.
Thoughts?
[0]:
https://www.postgresql.org/message-id/ad628976-8b33-468d-8e52-3fbfcff89103%40dunslane.netFrom 0b595189077c4310cc6ce6d8b08ffa5b40a0f1cc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 15:49:40 +0100
Subject: [PATCH v19.2 1/6] Rename GistTranslateStratnum() to
GistTranslateCompareType()
Follow up to commit 630f9a43cec. The previous name had become
confusing, because it doesn't actually translate a strategy number but
a CompareType into a strategy number. We might add the inverse at
some point, which would then probably be called something like
GistTranslateStratnum.
---
src/backend/access/gist/gistutil.c | 2 +-
src/backend/commands/indexcmds.c | 2 +-
src/backend/commands/tablecmds.c | 4 ++--
src/backend/executor/execReplication.c | 2 +-
src/include/access/gist.h | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/gist/gistutil.c
b/src/backend/access/gist/gistutil.c
index 48db718b904..4d3b6dfa32b 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -1095,7 +1095,7 @@ gist_stratnum_common(PG_FUNCTION_ARGS)
* Returns InvalidStrategy if the function is not defined.
*/
StrategyNumber
-GistTranslateStratnum(Oid opclass, CompareType cmptype)
+GistTranslateCompareType(Oid opclass, CompareType cmptype)
{
Oid opfamily;
Oid opcintype;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 59c836fc24d..0aa15a42efe 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2455,7 +2455,7 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype,
CompareType cmptype,
* For now we only need GiST support, but this could support
other
* indexams if we wanted.
*/
- *strat = GistTranslateStratnum(opclass, cmptype);
+ *strat = GistTranslateCompareType(opclass, cmptype);
if (*strat == InvalidStrategy)
{
HeapTuple tuple;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 57662fd7662..d51298284f6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10016,7 +10016,7 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
* ask the opclass what number it actually uses instead
of our RT*
* constants.
*/
- eqstrategy = GistTranslateStratnum(opclasses[i],
cmptype);
+ eqstrategy = GistTranslateCompareType(opclasses[i],
cmptype);
if (eqstrategy == InvalidStrategy)
{
HeapTuple tuple;
@@ -10041,7 +10041,7 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
* other index AMs support unique indexes. If we ever
did have
* other types of unique indexes, we'd need a way to
determine
* which operator strategy number is equality. (We
could use
- * something like GistTranslateStratnum.)
+ * something like GistTranslateCompareType.)
*/
if (amid != BTREE_AM_OID)
elog(ERROR, "only b-tree indexes are supported
for foreign keys");
diff --git a/src/backend/executor/execReplication.c
b/src/backend/executor/execReplication.c
index 3985e84d3a6..2dac4bd363b 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -57,7 +57,7 @@ get_equal_strategy_number(Oid opclass)
ret = HTEqualStrategyNumber;
break;
case GIST_AM_OID:
- ret = GistTranslateStratnum(opclass, COMPARE_EQ);
+ ret = GistTranslateCompareType(opclass, COMPARE_EQ);
break;
default:
ret = InvalidStrategy;
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index 2f419a3b309..eff019f7515 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -248,6 +248,6 @@ typedef struct
do { (e).key = (k); (e).rel = (r); (e).page = (pg); \
(e).offset = (o); (e).leafkey = (l); } while (0)
-extern StrategyNumber GistTranslateStratnum(Oid opclass, CompareType cmp);
+extern StrategyNumber GistTranslateCompareType(Oid opclass, CompareType
cmptype);
#endif /* GIST_H */
--
2.48.1
From 48a087657a1bdcecc99a86d54576da282f4d3b72 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 22:58:13 +0100
Subject: [PATCH v19.2 2/6] Add get_opfamily_name() function
---
src/backend/utils/cache/lsyscache.c | 27 +++++++++++++++++++++++++++
src/include/utils/lsyscache.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/src/backend/utils/cache/lsyscache.c
b/src/backend/utils/cache/lsyscache.c
index 7a9af03c960..bcfa5cb4add 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -30,6 +30,7 @@
#include "catalog/pg_language.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_opclass.h"
+#include "catalog/pg_opfamily.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_publication.h"
@@ -1273,6 +1274,32 @@ get_opclass_method(Oid opclass)
return result;
}
+/* ---------- OPFAMILY CACHE ----------
*/
+
+char *
+get_opfamily_name(Oid opfid, bool missing_ok)
+{
+ HeapTuple tup;
+ char *opfname;
+ Form_pg_opfamily opfform;
+
+ tup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfid));
+
+ if (!HeapTupleIsValid(tup))
+ {
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for operator family
%u", opfid);
+ return NULL;
+ }
+
+ opfform = (Form_pg_opfamily) GETSTRUCT(tup);
+ opfname = pstrdup(NameStr(opfform->opfname));
+
+ ReleaseSysCache(tup);
+
+ return opfname;
+}
+
/* ---------- OPERATOR CACHE ----------
*/
/*
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index b23deb9662f..6fab7aa6009 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -108,6 +108,7 @@ extern Oid get_opclass_input_type(Oid opclass);
extern bool get_opclass_opfamily_and_input_type(Oid opclass,
Oid *opfamily, Oid *opcintype);
extern Oid get_opclass_method(Oid opclass);
+extern char *get_opfamily_name(Oid opfid, bool missing_ok);
extern RegProcedure get_opcode(Oid opno);
extern char *get_opname(Oid opno);
extern Oid get_op_rettype(Oid opno);
--
2.48.1
From e5bb5355f0f877576a5ea12d3644c0ec5ca0809e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 22:59:23 +0100
Subject: [PATCH v19.2 3/6] Add some uses of get_opfamily_name()
This refactors and simplifies various existing code to make use of the
new function.
---
contrib/bloom/blvalidate.c | 12 ++--------
src/backend/access/brin/brin_validate.c | 12 ++--------
src/backend/access/gin/ginvalidate.c | 12 ++--------
src/backend/access/gist/gistvalidate.c | 11 +--------
src/backend/access/hash/hashvalidate.c | 11 +--------
src/backend/access/nbtree/nbtvalidate.c | 11 +--------
src/backend/access/spgist/spgvalidate.c | 11 +--------
src/backend/commands/indexcmds.c | 30 ++-----------------------
8 files changed, 12 insertions(+), 98 deletions(-)
diff --git a/contrib/bloom/blvalidate.c b/contrib/bloom/blvalidate.c
index acaf9d2be48..7fc9db0e56f 100644
--- a/contrib/bloom/blvalidate.c
+++ b/contrib/bloom/blvalidate.c
@@ -18,8 +18,8 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
+#include "utils/lsyscache.h"
#include "utils/regproc.h"
#include "utils/syscache.h"
@@ -36,8 +36,6 @@ blvalidate(Oid opclassoid)
Oid opcintype;
Oid opckeytype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -60,12 +58,7 @@ blvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -216,7 +209,6 @@ blvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/brin/brin_validate.c
b/src/backend/access/brin/brin_validate.c
index a49a9009626..915b8628b46 100644
--- a/src/backend/access/brin/brin_validate.c
+++ b/src/backend/access/brin/brin_validate.c
@@ -19,9 +19,9 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
#include "utils/builtins.h"
+#include "utils/lsyscache.h"
#include "utils/regproc.h"
#include "utils/syscache.h"
@@ -42,8 +42,6 @@ brinvalidate(Oid opclassoid)
Oid opfamilyoid;
Oid opcintype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -65,12 +63,7 @@ brinvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -274,7 +267,6 @@ brinvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/gin/ginvalidate.c
b/src/backend/access/gin/ginvalidate.c
index 6bd94e44854..5b0bfe8cc1d 100644
--- a/src/backend/access/gin/ginvalidate.c
+++ b/src/backend/access/gin/ginvalidate.c
@@ -19,8 +19,8 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
+#include "utils/lsyscache.h"
#include "utils/regproc.h"
#include "utils/syscache.h"
@@ -37,8 +37,6 @@ ginvalidate(Oid opclassoid)
Oid opcintype;
Oid opckeytype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -61,12 +59,7 @@ ginvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -264,7 +257,6 @@ ginvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/gist/gistvalidate.c
b/src/backend/access/gist/gistvalidate.c
index bb86b559486..ffefa12d97a 100644
--- a/src/backend/access/gist/gistvalidate.c
+++ b/src/backend/access/gist/gistvalidate.c
@@ -19,7 +19,6 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
#include "utils/lsyscache.h"
#include "utils/regproc.h"
@@ -39,8 +38,6 @@ gistvalidate(Oid opclassoid)
Oid opcintype;
Oid opckeytype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -63,12 +60,7 @@ gistvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -282,7 +274,6 @@ gistvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/hash/hashvalidate.c
b/src/backend/access/hash/hashvalidate.c
index b54ecde9224..06ac832ba10 100644
--- a/src/backend/access/hash/hashvalidate.c
+++ b/src/backend/access/hash/hashvalidate.c
@@ -21,7 +21,6 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
@@ -46,8 +45,6 @@ hashvalidate(Oid opclassoid)
Oid opfamilyoid;
Oid opcintype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -68,12 +65,7 @@ hashvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -258,7 +250,6 @@ hashvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/nbtree/nbtvalidate.c
b/src/backend/access/nbtree/nbtvalidate.c
index b87c959a2fd..dd6f5a15c65 100644
--- a/src/backend/access/nbtree/nbtvalidate.c
+++ b/src/backend/access/nbtree/nbtvalidate.c
@@ -21,7 +21,6 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
@@ -46,8 +45,6 @@ btvalidate(Oid opclassoid)
Oid opfamilyoid;
Oid opcintype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -69,12 +66,7 @@ btvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -280,7 +272,6 @@ btvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/access/spgist/spgvalidate.c
b/src/backend/access/spgist/spgvalidate.c
index 7d05e43720b..e9964fab4f4 100644
--- a/src/backend/access/spgist/spgvalidate.c
+++ b/src/backend/access/spgist/spgvalidate.c
@@ -19,7 +19,6 @@
#include "catalog/pg_amop.h"
#include "catalog/pg_amproc.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_type.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
@@ -45,8 +44,6 @@ spgvalidate(Oid opclassoid)
Oid opcintype;
Oid opckeytype;
char *opclassname;
- HeapTuple familytup;
- Form_pg_opfamily familyform;
char *opfamilyname;
CatCList *proclist,
*oprlist;
@@ -72,12 +69,7 @@ spgvalidate(Oid opclassoid)
opclassname = NameStr(classform->opcname);
/* Fetch opfamily information */
- familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
- if (!HeapTupleIsValid(familytup))
- elog(ERROR, "cache lookup failed for operator family %u",
opfamilyoid);
- familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
-
- opfamilyname = NameStr(familyform->opfname);
+ opfamilyname = get_opfamily_name(opfamilyoid, false);
/* Fetch all operators and support functions of the opfamily */
oprlist = SearchSysCacheList1(AMOPSTRATEGY,
ObjectIdGetDatum(opfamilyoid));
@@ -319,7 +311,6 @@ spgvalidate(Oid opclassoid)
ReleaseCatCacheList(proclist);
ReleaseCatCacheList(oprlist);
- ReleaseSysCache(familytup);
ReleaseSysCache(classtup);
return result;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0aa15a42efe..f788b8f29b8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -35,7 +35,6 @@
#include "catalog/pg_inherits.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_opclass.h"
-#include "catalog/pg_opfamily.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
#include "commands/comment.h"
@@ -2147,29 +2146,12 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
opfamily = get_opclass_family(opclassOids[attn]);
strat = get_op_opfamily_strategy(opid, opfamily);
if (strat == 0)
- {
- HeapTuple opftuple;
- Form_pg_opfamily opfform;
-
- /*
- * attribute->opclass might not explicitly name
the opfamily,
- * so fetch the name of the selected opfamily
for use in the
- * error message.
- */
- opftuple = SearchSysCache1(OPFAMILYOID,
-
ObjectIdGetDatum(opfamily));
- if (!HeapTupleIsValid(opftuple))
- elog(ERROR, "cache lookup failed for
opfamily %u",
- opfamily);
- opfform = (Form_pg_opfamily)
GETSTRUCT(opftuple);
-
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("operator %s is not a
member of operator family \"%s\"",
format_operator(opid),
-
NameStr(opfform->opfname)),
+
get_opfamily_name(opfamily, false)),
errdetail("The exclusion
operator must be related to the index operator class for the constraint.")));
- }
indexInfo->ii_ExclusionOps[attn] = opid;
indexInfo->ii_ExclusionProcs[attn] = get_opcode(opid);
@@ -2484,21 +2466,13 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype,
CompareType cmptype,
}
if (!OidIsValid(*opid))
- {
- HeapTuple tuple;
-
- tuple = SearchSysCache1(OPFAMILYOID,
ObjectIdGetDatum(opfamily));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for operator family
%u", opfamily);
-
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
cmptype == COMPARE_EQ ? errmsg("could not
identify an equality operator for type %s", format_type_be(opcintype)) :
cmptype == COMPARE_OVERLAP ? errmsg("could not
identify an overlaps operator for type %s", format_type_be(opcintype)) :
cmptype == COMPARE_CONTAINED_BY ? errmsg("could
not identify a contained-by operator for type %s", format_type_be(opcintype)) :
0,
errdetail("There is no suitable operator in
operator family \"%s\" for access method \"%s\".",
- NameStr(((Form_pg_opfamily)
GETSTRUCT(tuple))->opfname), "gist"));
- }
+ get_opfamily_name(opfamily,
false), "gist"));
}
/*
--
2.48.1
From 541a93828f4d1d1ec266f353ed2b5a96ae3cdb13 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 23:00:49 +0100
Subject: [PATCH v19.2 4/6] Move CompareType to separate header file
We'll want to make use of it in more places, and we'd prefer to not
have to include all of primnodes.h everywhere.
Author: Mark Dilger <mark.dil...@enterprisedb.com>
Discussion:
https://www.postgresql.org/message-id/flat/e72eaa49-354d-4c2e-8eb9-255197f55...@enterprisedb.com
---
contrib/btree_gist/btree_gist.c | 2 +-
src/backend/nodes/Makefile | 1 +
src/backend/nodes/gen_node_support.pl | 1 +
src/include/access/cmptype.h | 44 +++++++++++++++++++++++++++
src/include/nodes/meson.build | 1 +
src/include/nodes/primnodes.h | 28 +----------------
6 files changed, 49 insertions(+), 28 deletions(-)
create mode 100644 src/include/access/cmptype.h
diff --git a/contrib/btree_gist/btree_gist.c b/contrib/btree_gist/btree_gist.c
index fc6c7091795..7fcb0cd6d03 100644
--- a/contrib/btree_gist/btree_gist.c
+++ b/contrib/btree_gist/btree_gist.c
@@ -3,8 +3,8 @@
*/
#include "postgres.h"
+#include "access/cmptype.h"
#include "access/stratnum.h"
-#include "nodes/primnodes.h"
#include "utils/builtins.h"
PG_MODULE_MAGIC;
diff --git a/src/backend/nodes/Makefile b/src/backend/nodes/Makefile
index 66bbad8e6e0..77ddb9ca53f 100644
--- a/src/backend/nodes/Makefile
+++ b/src/backend/nodes/Makefile
@@ -46,6 +46,7 @@ node_headers = \
nodes/plannodes.h \
nodes/execnodes.h \
access/amapi.h \
+ access/cmptype.h \
access/sdir.h \
access/tableam.h \
access/tsmapi.h \
diff --git a/src/backend/nodes/gen_node_support.pl
b/src/backend/nodes/gen_node_support.pl
index 7c012c27f88..1a657f7e0ae 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -58,6 +58,7 @@ sub elem
nodes/plannodes.h
nodes/execnodes.h
access/amapi.h
+ access/cmptype.h
access/sdir.h
access/tableam.h
access/tsmapi.h
diff --git a/src/include/access/cmptype.h b/src/include/access/cmptype.h
new file mode 100644
index 00000000000..339099564e9
--- /dev/null
+++ b/src/include/access/cmptype.h
@@ -0,0 +1,44 @@
+/*-------------------------------------------------------------------------
+ *
+ * cmptype.h
+ * POSTGRES compare type definitions.
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/cmptype.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef CMPTYPE_H
+#define CMPTYPE_H
+
+/*
+ * CompareType - fundamental semantics of certain operators
+ *
+ * These enum symbols represent the fundamental semantics of certain operators
+ * that the system needs to have some hardcoded knowledge about. (For
+ * example, RowCompareExpr needs to know which operators can be determined to
+ * act like =, <>, <, etc.) Index access methods map (some of) strategy
+ * numbers to these values so that the system can know about the meaning of
+ * (some of) the operators without needing hardcoded knowledge of index AM's
+ * strategy numbering.
+ *
+ * XXX Currently, this mapping is not fully developed and most values are
+ * chosen to match btree strategy numbers, which is not going to work very
+ * well for other access methods.
+ */
+typedef enum CompareType
+{
+ COMPARE_INVALID = 0,
+ COMPARE_LT = 1, /* BTLessStrategyNumber */
+ COMPARE_LE = 2, /* BTLessEqualStrategyNumber */
+ COMPARE_EQ = 3, /* BTEqualStrategyNumber */
+ COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber
*/
+ COMPARE_GT = 5, /* BTGreaterStrategyNumber */
+ COMPARE_NE = 6, /* no such btree strategy */
+ COMPARE_OVERLAP,
+ COMPARE_CONTAINED_BY,
+} CompareType;
+
+#endif /* CMPTYPE_H */
diff --git a/src/include/nodes/meson.build b/src/include/nodes/meson.build
index f3dd5461fef..d1ca24dd32f 100644
--- a/src/include/nodes/meson.build
+++ b/src/include/nodes/meson.build
@@ -8,6 +8,7 @@ node_support_input_i = [
'nodes/plannodes.h',
'nodes/execnodes.h',
'access/amapi.h',
+ 'access/cmptype.h',
'access/sdir.h',
'access/tableam.h',
'access/tsmapi.h',
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 59e7bb26bbd..839e71d52f4 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -18,6 +18,7 @@
#define PRIMNODES_H
#include "access/attnum.h"
+#include "access/cmptype.h"
#include "nodes/bitmapset.h"
#include "nodes/pg_list.h"
@@ -1451,33 +1452,6 @@ typedef struct RowExpr
ParseLoc location; /* token location, or -1 if
unknown */
} RowExpr;
-/*
- * CompareType - fundamental semantics of certain operators
- *
- * These enum symbols represent the fundamental semantics of certain operators
- * that the system needs to have some hardcoded knowledge about. (For
- * example, RowCompareExpr needs to know which operators can be determined to
- * act like =, <>, <, etc.) Index access methods map (some of) strategy
- * numbers to these values so that the system can know about the meaning of
- * (some of) the operators without needing hardcoded knowledge of index AM's
- * strategy numbering.
- *
- * XXX Currently, this mapping is not fully developed and most values are
- * chosen to match btree strategy numbers, which is not going to work very
- * well for other access methods.
- */
-typedef enum CompareType
-{
- COMPARE_LT = 1, /* BTLessStrategyNumber */
- COMPARE_LE = 2, /* BTLessEqualStrategyNumber */
- COMPARE_EQ = 3, /* BTEqualStrategyNumber */
- COMPARE_GE = 4, /* BTGreaterEqualStrategyNumber
*/
- COMPARE_GT = 5, /* BTGreaterStrategyNumber */
- COMPARE_NE = 6, /* no such btree strategy */
- COMPARE_OVERLAP,
- COMPARE_CONTAINED_BY,
-} CompareType;
-
/*
* RowCompareExpr - row-wise comparison, such as (a, b) <= (1, 2)
*
--
2.48.1
From 84db7de83afbf35b95687baa2893b42183fa872e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 16:12:38 +0100
Subject: [PATCH v19.2 5/6] Convert strategies to and from compare types
For each Index AM, provide a mapping between operator strategies and
the system-wide generic concept of a comparison type. For example,
for btree, BTLessStrategyNumber maps to and from COMPARE_LT. Numerous
places in the planner and executor think directly in terms of btree
strategy numbers (and a few in terms of hash strategy numbers.) These
should be converted over subsequent commits to think in terms of
CompareType instead.
Author: Mark Dilger <mark.dil...@enterprisedb.com>
Discussion:
https://www.postgresql.org/message-id/flat/e72eaa49-354d-4c2e-8eb9-255197f55...@enterprisedb.com
---
contrib/bloom/blutils.c | 2 ++
doc/src/sgml/indexam.sgml | 26 +++++++++++++++
src/backend/access/brin/brin.c | 2 ++
src/backend/access/gist/gist.c | 2 ++
src/backend/access/hash/hash.c | 19 +++++++++++
src/backend/access/index/amapi.c | 50 ++++++++++++++++++++++++++++
src/backend/access/nbtree/nbtree.c | 43 ++++++++++++++++++++++++
src/backend/access/spgist/spgutils.c | 2 ++
src/include/access/amapi.h | 14 ++++++++
src/include/access/hash.h | 3 ++
src/include/access/nbtree.h | 3 ++
src/tools/pgindent/typedefs.list | 2 ++
12 files changed, 168 insertions(+)
diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index 3796bea786b..04b61042a57 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -150,6 +150,8 @@ blhandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
+ amroutine->amtranslatestrategy = NULL;
+ amroutine->amtranslatecmptype = NULL;
PG_RETURN_POINTER(amroutine);
}
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index dc7d14b60dd..d17fcbd5cec 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -164,6 +164,10 @@ <title>Basic API Structure for Indexes</title>
amestimateparallelscan_function amestimateparallelscan; /* can be NULL
*/
aminitparallelscan_function aminitparallelscan; /* can be NULL */
amparallelrescan_function amparallelrescan; /* can be NULL */
+
+ /* interface functions to support planning */
+ amtranslate_strategy_function amtranslatestrategy; /* can be NULL */
+ amtranslate_cmptype_function amtranslatecmptype; /* can be NULL */
} IndexAmRoutine;
</programlisting>
</para>
@@ -876,6 +880,28 @@ <title>Index Access Method Functions</title>
the beginning.
</para>
+ <para>
+<programlisting>
+CompareType
+amtranslatestrategy (StrategyNumber strategy, Oid opfamily, Oid opcintype);
+
+StrategyNumber
+amtranslatecmptype (CompareType cmptype, Oid opfamily, Oid opcintype);
+</programlisting>
+ These functions, if implemented, will be called by the planer and executor
+ to convert between fixed <type>CompareType</type> values and the specific
+ strategy numbers used by the access method. These functions can be
+ implemented by access methods that implement functionality similar to the
+ built-in btree or hash access methods, and by implementing these
+ translations, the system can learn about the semantics of the access
+ method's operations and can use them in place of btree or hash indexes in
+ various places. If the functionality of the access method is not similar
+ to those built-in access methods, these functions do not need to be
+ implemented. If the functions are not implemented, the access method will
+ be ignored for certain planner and executor decisions, but is otherwise
+ fully functional.
+ </para>
+
</sect1>
<sect1 id="index-scanning">
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 4289142e20b..ccf824bbdb2 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -298,6 +298,8 @@ brinhandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
+ amroutine->amtranslatestrategy = NULL;
+ amroutine->amtranslatecmptype = NULL;
PG_RETURN_POINTER(amroutine);
}
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index b6bc75b44e3..70f8086db7b 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -107,6 +107,8 @@ gisthandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
+ amroutine->amtranslatestrategy = NULL;
+ amroutine->amtranslatecmptype = NULL;
PG_RETURN_POINTER(amroutine);
}
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index f950b9925f5..504991215ea 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -21,6 +21,7 @@
#include "access/hash.h"
#include "access/hash_xlog.h"
#include "access/relscan.h"
+#include "access/stratnum.h"
#include "access/tableam.h"
#include "access/xloginsert.h"
#include "commands/progress.h"
@@ -105,6 +106,8 @@ hashhandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
+ amroutine->amtranslatestrategy = hashtranslatestrategy;
+ amroutine->amtranslatecmptype = hashtranslatecmptype;
PG_RETURN_POINTER(amroutine);
}
@@ -922,3 +925,19 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer
bucket_buf,
else
LockBuffer(bucket_buf, BUFFER_LOCK_UNLOCK);
}
+
+CompareType
+hashtranslatestrategy(StrategyNumber strategy, Oid opfamily, Oid opcintype)
+{
+ if (strategy == HTEqualStrategyNumber)
+ return COMPARE_EQ;
+ return COMPARE_INVALID;
+}
+
+StrategyNumber
+hashtranslatecmptype(CompareType cmptype, Oid opfamily, Oid opcintype)
+{
+ if (cmptype == COMPARE_EQ)
+ return HTEqualStrategyNumber;
+ return InvalidStrategy;
+}
diff --git a/src/backend/access/index/amapi.c b/src/backend/access/index/amapi.c
index 3522bcaa401..5f53f49ec32 100644
--- a/src/backend/access/index/amapi.c
+++ b/src/backend/access/index/amapi.c
@@ -107,6 +107,56 @@ GetIndexAmRoutineByAmId(Oid amoid, bool noerror)
}
+/*
+ * IndexAmTranslateStrategy - given an access method and strategy, get the
+ * corresponding compare type.
+ *
+ * If missing_ok is false, throw an error if no compare type is found. If
+ * true, just return COMPARE_INVALID.
+ */
+CompareType
+IndexAmTranslateStrategy(StrategyNumber strategy, Oid amoid, Oid opfamily, Oid
opcintype, bool missing_ok)
+{
+ CompareType result;
+ IndexAmRoutine *amroutine;
+
+ amroutine = GetIndexAmRoutineByAmId(amoid, false);
+ if (amroutine->amtranslatestrategy)
+ result = amroutine->amtranslatestrategy(strategy, opfamily,
opcintype);
+ else
+ result = COMPARE_INVALID;
+
+ if (!missing_ok && result == COMPARE_INVALID)
+ elog(ERROR, "could not translate strategy number %d for index
AM %u", strategy, amoid);
+
+ return result;
+}
+
+/*
+ * IndexAmTranslateCompareType - given an access method and compare type, get
+ * the corresponding strategy number.
+ *
+ * If missing_ok is false, throw an error if no strategy is found correlating
+ * to the given cmptype. If true, just return InvalidStrategy.
+ */
+StrategyNumber
+IndexAmTranslateCompareType(CompareType cmptype, Oid amoid, Oid opfamily, Oid
opcintype, bool missing_ok)
+{
+ StrategyNumber result;
+ IndexAmRoutine *amroutine;
+
+ amroutine = GetIndexAmRoutineByAmId(amoid, false);
+ if (amroutine->amtranslatecmptype)
+ result = amroutine->amtranslatecmptype(cmptype, opfamily,
opcintype);
+ else
+ result = InvalidStrategy;
+
+ if (!missing_ok && result == InvalidStrategy)
+ elog(ERROR, "could not translate compare type %u for index AM
%u", cmptype, amoid);
+
+ return result;
+}
+
/*
* Ask appropriate access method to validate the specified opclass.
*/
diff --git a/src/backend/access/nbtree/nbtree.c
b/src/backend/access/nbtree/nbtree.c
index 3d617f168f5..971405e89af 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -20,6 +20,7 @@
#include "access/nbtree.h"
#include "access/relscan.h"
+#include "access/stratnum.h"
#include "commands/progress.h"
#include "commands/vacuum.h"
#include "nodes/execnodes.h"
@@ -148,6 +149,8 @@ bthandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = btestimateparallelscan;
amroutine->aminitparallelscan = btinitparallelscan;
amroutine->amparallelrescan = btparallelrescan;
+ amroutine->amtranslatestrategy = bttranslatestrategy;
+ amroutine->amtranslatecmptype = bttranslatecmptype;
PG_RETURN_POINTER(amroutine);
}
@@ -1508,3 +1511,43 @@ btgettreeheight(Relation rel)
{
return _bt_getrootheight(rel);
}
+
+CompareType
+bttranslatestrategy(StrategyNumber strategy, Oid opfamily, Oid opcintype)
+{
+ switch (strategy)
+ {
+ case BTLessStrategyNumber:
+ return COMPARE_LT;
+ case BTLessEqualStrategyNumber:
+ return COMPARE_LE;
+ case BTEqualStrategyNumber:
+ return COMPARE_EQ;
+ case BTGreaterEqualStrategyNumber:
+ return COMPARE_GE;
+ case BTGreaterStrategyNumber:
+ return COMPARE_GT;
+ default:
+ return COMPARE_INVALID;
+ }
+}
+
+StrategyNumber
+bttranslatecmptype(CompareType cmptype, Oid opfamily, Oid opcintype)
+{
+ switch (cmptype)
+ {
+ case COMPARE_LT:
+ return BTLessStrategyNumber;
+ case COMPARE_LE:
+ return BTLessEqualStrategyNumber;
+ case COMPARE_EQ:
+ return BTEqualStrategyNumber;
+ case COMPARE_GE:
+ return BTGreaterEqualStrategyNumber;
+ case COMPARE_GT:
+ return BTGreaterStrategyNumber;
+ default:
+ return InvalidStrategy;
+ }
+}
diff --git a/src/backend/access/spgist/spgutils.c
b/src/backend/access/spgist/spgutils.c
index 6e968048917..367c36ef9af 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -92,6 +92,8 @@ spghandler(PG_FUNCTION_ARGS)
amroutine->amestimateparallelscan = NULL;
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
+ amroutine->amtranslatestrategy = NULL;
+ amroutine->amtranslatecmptype = NULL;
PG_RETURN_POINTER(amroutine);
}
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index fb94b3d1acf..6723de75a4d 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -12,7 +12,9 @@
#ifndef AMAPI_H
#define AMAPI_H
+#include "access/cmptype.h"
#include "access/genam.h"
+#include "access/stratnum.h"
/*
* We don't wish to include planner header files here, since most of an index
@@ -99,6 +101,12 @@ typedef struct OpFamilyMember
* Callback function signatures --- see indexam.sgml for more info.
*/
+/* translate AM-specific strategies to general operator types */
+typedef CompareType (*amtranslate_strategy_function) (StrategyNumber strategy,
Oid opfamily, Oid opcintype);
+
+/* translate general operator types to AM-specific strategies */
+typedef StrategyNumber (*amtranslate_cmptype_function) (CompareType cmptype,
Oid opfamily, Oid opcintype);
+
/* build new index */
typedef IndexBuildResult *(*ambuild_function) (Relation heapRelation,
Relation indexRelation,
@@ -301,11 +309,17 @@ typedef struct IndexAmRoutine
amestimateparallelscan_function amestimateparallelscan; /* can be NULL
*/
aminitparallelscan_function aminitparallelscan; /* can be NULL */
amparallelrescan_function amparallelrescan; /* can be NULL */
+
+ /* interface functions to support planning */
+ amtranslate_strategy_function amtranslatestrategy; /* can be NULL
*/
+ amtranslate_cmptype_function amtranslatecmptype; /* can be NULL
*/
} IndexAmRoutine;
/* Functions in access/index/amapi.c */
extern IndexAmRoutine *GetIndexAmRoutine(Oid amhandler);
extern IndexAmRoutine *GetIndexAmRoutineByAmId(Oid amoid, bool noerror);
+extern CompareType IndexAmTranslateStrategy(StrategyNumber strategy, Oid
amoid, Oid opfamily, Oid opcintype, bool missing_ok);
+extern StrategyNumber IndexAmTranslateCompareType(CompareType cmptype, Oid
amoid, Oid opfamily, Oid opcintype, bool missing_ok);
#endif /* AMAPI_H */
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 0f09f35a9fd..e91f2b00ad9 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -387,6 +387,9 @@ extern void hashadjustmembers(Oid opfamilyoid,
List *operators,
List *functions);
+extern CompareType hashtranslatestrategy(StrategyNumber strategy, Oid
opfamily, Oid opcintype);
+extern StrategyNumber hashtranslatecmptype(CompareType cmptype, Oid opfamily,
Oid opcintype);
+
/* private routines */
/* hashinsert.c */
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 6a501537e1e..000c7289b80 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -1183,6 +1183,9 @@ extern IndexBulkDeleteResult
*btvacuumcleanup(IndexVacuumInfo *info,
extern bool btcanreturn(Relation index, int attno);
extern int btgettreeheight(Relation rel);
+extern CompareType bttranslatestrategy(StrategyNumber strategy, Oid opfamily,
Oid opcintype);
+extern StrategyNumber bttranslatecmptype(CompareType cmptype, Oid opfamily,
Oid opcintype);
+
/*
* prototypes for internal functions in nbtree.c
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a2644a2e653..9a3bee93dec 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3318,6 +3318,8 @@ amparallelrescan_function
amproperty_function
amrescan_function
amrestrpos_function
+amtranslate_strategy_function amtranslatestrategy;
+amtranslate_cmptype_function amtranslatecmptype;
amvacuumcleanup_function
amvalidate_function
array_iter
--
2.48.1
From 23c0334e05f0bd4baf5b606264f9ff532adf116b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 24 Jan 2025 16:55:47 +0100
Subject: [PATCH v19.2 6/6] Integrate GistTranslateCompareType() into
IndexAmTranslateCompareType()
This turns GistTranslateCompareType() into a callback function of the
gist index AM instead of a standalone function. The existing callers
are changed to use IndexAmTranslateCompareType(). This then also
makes that code not or less hardcoded toward gist.
---
src/backend/access/gist/gist.c | 2 +-
src/backend/access/gist/gistutil.c | 8 +----
src/backend/commands/indexcmds.c | 22 ++++----------
src/backend/commands/tablecmds.c | 28 ++++--------------
src/backend/executor/execReplication.c | 34 +---------------------
src/backend/replication/logical/relation.c | 8 ++++-
src/include/access/gist.h | 2 +-
src/include/executor/executor.h | 1 -
8 files changed, 23 insertions(+), 82 deletions(-)
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 70f8086db7b..4d858b65e1e 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -108,7 +108,7 @@ gisthandler(PG_FUNCTION_ARGS)
amroutine->aminitparallelscan = NULL;
amroutine->amparallelrescan = NULL;
amroutine->amtranslatestrategy = NULL;
- amroutine->amtranslatecmptype = NULL;
+ amroutine->amtranslatecmptype = gisttranslatecmptype;
PG_RETURN_POINTER(amroutine);
}
diff --git a/src/backend/access/gist/gistutil.c
b/src/backend/access/gist/gistutil.c
index 4d3b6dfa32b..dbc4ac639a2 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -1095,17 +1095,11 @@ gist_stratnum_common(PG_FUNCTION_ARGS)
* Returns InvalidStrategy if the function is not defined.
*/
StrategyNumber
-GistTranslateCompareType(Oid opclass, CompareType cmptype)
+gisttranslatecmptype(CompareType cmptype, Oid opfamily, Oid opcintype)
{
- Oid opfamily;
- Oid opcintype;
Oid funcid;
Datum result;
- /* Look up the opclass family and input datatype. */
- if (!get_opclass_opfamily_and_input_type(opclass, &opfamily,
&opcintype))
- return InvalidStrategy;
-
/* Check whether the function is provided. */
funcid = get_opfamily_proc(opfamily, opcintype, opcintype,
GIST_STRATNUM_PROC);
if (!OidIsValid(funcid))
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f788b8f29b8..637ff52b505 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2422,6 +2422,7 @@ void
GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
Oid *opid, StrategyNumber
*strat)
{
+ Oid amid = GIST_AM_OID; /* For now we only need
GiST support. */
Oid opfamily;
Oid opcintype;
@@ -2432,28 +2433,17 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype,
CompareType cmptype,
if (get_opclass_opfamily_and_input_type(opclass, &opfamily, &opcintype))
{
/*
- * Ask the opclass to translate to its internal stratnum
- *
- * For now we only need GiST support, but this could support
other
- * indexams if we wanted.
+ * Ask the index AM to translate to its internal stratnum
*/
- *strat = GistTranslateCompareType(opclass, cmptype);
+ *strat = IndexAmTranslateCompareType(cmptype, amid, opfamily,
opcintype, true);
if (*strat == InvalidStrategy)
- {
- HeapTuple tuple;
-
- tuple = SearchSysCache1(CLAOID,
ObjectIdGetDatum(opclass));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for operator
class %u", opclass);
-
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
cmptype = COMPARE_EQ ? errmsg("could
not identify an equality operator for type %s", format_type_be(opcintype)) :
cmptype == COMPARE_OVERLAP ?
errmsg("could not identify an overlaps operator for type %s",
format_type_be(opcintype)) :
cmptype == COMPARE_CONTAINED_BY ?
errmsg("could not identify a contained-by operator for type %s",
format_type_be(opcintype)) : 0,
- errdetail("Could not translate compare
type %d for operator class \"%s\" for access method \"%s\".",
- cmptype,
NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
- }
+ errdetail("Could not translate compare
type %d for operator family \"%s\", input type %s, access method \"%s\".",
+ cmptype,
get_opfamily_name(opfamily, false), format_type_be(opcintype),
get_am_name(amid)));
/*
* We parameterize rhstype so foreign keys can ask for a <@
operator
@@ -2472,7 +2462,7 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype,
CompareType cmptype,
cmptype == COMPARE_OVERLAP ? errmsg("could not
identify an overlaps operator for type %s", format_type_be(opcintype)) :
cmptype == COMPARE_CONTAINED_BY ? errmsg("could
not identify a contained-by operator for type %s", format_type_be(opcintype)) :
0,
errdetail("There is no suitable operator in
operator family \"%s\" for access method \"%s\".",
- get_opfamily_name(opfamily,
false), "gist"));
+ get_opfamily_name(opfamily,
false), get_am_name(amid)));
}
/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d51298284f6..9e027f1b328 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10002,37 +10002,21 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
CompareType cmptype;
bool for_overlaps = with_period && i ==
numpks - 1;
- /*
- * GiST indexes are required to support temporal
foreign keys
- * because they combine equals and overlaps.
- */
- if (amid != GIST_AM_OID)
- elog(ERROR, "only GiST indexes are supported
for temporal foreign keys");
-
cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
/*
- * An opclass can use whatever strategy numbers it
wants, so we
- * ask the opclass what number it actually uses instead
of our RT*
- * constants.
+ * An index AM can use whatever strategy numbers it
wants, so we
+ * ask it what number it actually uses.
*/
- eqstrategy = GistTranslateCompareType(opclasses[i],
cmptype);
+ eqstrategy = IndexAmTranslateCompareType(cmptype, amid,
opfamily, opcintype, true);
if (eqstrategy == InvalidStrategy)
- {
- HeapTuple tuple;
-
- tuple = SearchSysCache1(CLAOID,
ObjectIdGetDatum(opclasses[i]));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for
operator class %u", opclasses[i]);
-
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
for_overlaps
? errmsg("could not identify an
overlaps operator for foreign key")
: errmsg("could not identify an
equality operator for foreign key"),
- errdetail("Could not translate
compare type %d for operator class \"%s\" for access method \"%s\".",
- cmptype,
NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
- }
+ errdetail("Could not translate
compare type %d for operator family \"%s\", input type %s, access method
\"%s\".",
+ cmptype,
get_opfamily_name(opfamily, false), format_type_be(opcintype),
get_am_name(amid)));
}
else
{
@@ -10041,7 +10025,7 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
* other index AMs support unique indexes. If we ever
did have
* other types of unique indexes, we'd need a way to
determine
* which operator strategy number is equality. (We
could use
- * something like GistTranslateCompareType.)
+ * IndexAmTranslateCompareType.)
*/
if (amid != BTREE_AM_OID)
elog(ERROR, "only b-tree indexes are supported
for foreign keys");
diff --git a/src/backend/executor/execReplication.c
b/src/backend/executor/execReplication.c
index 2dac4bd363b..5f7613cc831 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -38,35 +38,6 @@
static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
TypeCacheEntry **eq);
-/*
- * Returns the fixed strategy number, if any, of the equality operator for the
- * given operator class, otherwise, InvalidStrategy.
- */
-StrategyNumber
-get_equal_strategy_number(Oid opclass)
-{
- Oid am = get_opclass_method(opclass);
- int ret;
-
- switch (am)
- {
- case BTREE_AM_OID:
- ret = BTEqualStrategyNumber;
- break;
- case HASH_AM_OID:
- ret = HTEqualStrategyNumber;
- break;
- case GIST_AM_OID:
- ret = GistTranslateCompareType(opclass, COMPARE_EQ);
- break;
- default:
- ret = InvalidStrategy;
- break;
- }
-
- return ret;
-}
-
/*
* Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that
* is setup to match 'rel' (*NOT* idxrel!).
@@ -120,10 +91,7 @@ build_replindex_scan_key(ScanKey skey, Relation rel,
Relation idxrel,
*/
optype = get_opclass_input_type(opclass->values[index_attoff]);
opfamily = get_opclass_family(opclass->values[index_attoff]);
- eq_strategy =
get_equal_strategy_number(opclass->values[index_attoff]);
- if (!eq_strategy)
- elog(ERROR, "missing equal strategy for opclass %u",
opclass->values[index_attoff]);
-
+ eq_strategy = IndexAmTranslateCompareType(COMPARE_EQ,
idxrel->rd_rel->relam, opfamily, optype, false);
operator = get_opfamily_member(opfamily, optype,
optype,
eq_strategy);
diff --git a/src/backend/replication/logical/relation.c
b/src/backend/replication/logical/relation.c
index 67aab02ff76..e9ad90d64a5 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -27,6 +27,7 @@
#include "replication/logicalrelation.h"
#include "replication/worker_internal.h"
#include "utils/inval.h"
+#include "utils/lsyscache.h"
#include "utils/syscache.h"
@@ -835,7 +836,12 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel,
AttrMap *attrmap)
/* Ensure that the index has a valid equal strategy for each key column
*/
for (int i = 0; i < idxrel->rd_index->indnkeyatts; i++)
{
- if (get_equal_strategy_number(indclass->values[i]) ==
InvalidStrategy)
+ Oid opfamily;
+ Oid opcintype;
+
+ if (!get_opclass_opfamily_and_input_type(indclass->values[i],
&opfamily, &opcintype))
+ return false;
+ if (IndexAmTranslateCompareType(COMPARE_EQ,
idxrel->rd_rel->relam, opfamily, opcintype, true) == InvalidStrategy)
return false;
}
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index eff019f7515..db62a9ac0b4 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -248,6 +248,6 @@ typedef struct
do { (e).key = (k); (e).rel = (r); (e).page = (pg); \
(e).offset = (o); (e).leafkey = (l); } while (0)
-extern StrategyNumber GistTranslateCompareType(Oid opclass, CompareType
cmptype);
+extern StrategyNumber gisttranslatecmptype(CompareType cmptype, Oid opfamily,
Oid opcintype);
#endif /* GIST_H */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index c7db6defd3e..45b80e6b98e 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -664,7 +664,6 @@ extern void check_exclusion_constraint(Relation heap,
Relation index,
/*
* prototypes from functions in execReplication.c
*/
-extern StrategyNumber get_equal_strategy_number(Oid opclass);
extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
LockTupleMode lockmode,
TupleTableSlot *searchslot,
--
2.48.1
