On 03.02.25 12:08, Peter Eisentraut wrote:
Yeah, actually this turned out to be unnecessary, because you can easily
obtain the AM OID from the passed-in opclass ID. So I have fixed it
that way. I have committed this whole patch set now with the mentioned
adjustments. I'll post a rebased version of the main patch set soon.
I picked off two more patches from the full patch series that can be
dealt with easily. These are
- Support non-btree indexes for foreign keys
- Allow non-btree speculative insertion indexes
(v19-0012 and v19-0013)
The version of these patches that were posted in the v19 bundle replaced
the hardcoded comparison with BTREE_AM_OID by a check against
(indirectly) amcanunique. But I think that is not necessary, because at
that point we already know that we are dealing with a unique index. The
reason for the check against BTREE_AM_OID was that we didn't have a way
to get the right strategy number for other index methods. But now we
have, so we can use the strategy translation functions and we don't need
to check for btree-ness anymore.
See attached patches.
From aa813425fbef10801bdd9582da09fc3eeaec9bb4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 4 Feb 2025 11:33:23 +0100
Subject: [PATCH v19.3 1/2] Support non-btree indexes for foreign keys
Previously, only btrees were supported as the referenced unique index
for foreign keys because there was no way to get the equality strategy
number for other index methods. We have this now (commit
c09e5a6a016), so we can support this. In fact, this is now just a
special case of the existing generalized "period" foreign key
support, since that already knows how to lookup equality strategy
numbers.
Note that this does not change the requirement that the referenced
index needs to be unique, and at the moment, only btree supports that,
so this does not change anything in practice, but it would allow
another index method that has amcanunique to be supported.
---
src/backend/commands/tablecmds.c | 57 +++++++++++++-------------------
1 file changed, 23 insertions(+), 34 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 18f64db6e39..5ea0b77400b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9981,6 +9981,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo
*tab, Relation rel,
Oid amid;
Oid opfamily;
Oid opcintype;
+ bool for_overlaps;
+ CompareType cmptype;
Oid pfeqop;
Oid ppeqop;
Oid ffeqop;
@@ -9997,40 +9999,27 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
opcintype = cla_tup->opcintype;
ReleaseSysCache(cla_ht);
- if (with_period)
- {
- CompareType cmptype;
- bool for_overlaps = with_period && i ==
numpks - 1;
-
- cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
-
- /*
- * An index AM can use whatever strategy numbers it
wants, so we
- * ask it what number it actually uses.
- */
- eqstrategy = IndexAmTranslateCompareType(cmptype, amid,
opfamily, opcintype, true);
- if (eqstrategy == InvalidStrategy)
- 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 family \"%s\", input type %s, access method
\"%s\".",
- cmptype,
get_opfamily_name(opfamily, false), format_type_be(opcintype),
get_am_name(amid)));
- }
- else
- {
- /*
- * Check it's a btree; currently this can never fail
since no
- * 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
- * IndexAmTranslateCompareType.)
- */
- if (amid != BTREE_AM_OID)
- elog(ERROR, "only b-tree indexes are supported
for foreign keys");
- eqstrategy = BTEqualStrategyNumber;
- }
+ /*
+ * Get strategy number from index AM.
+ *
+ * For a normal foreign-key constraint, this should not fail,
since we
+ * already checked that the index is unique and should
therefore have
+ * appropriate equal operators. For a period foreign key, this
could
+ * fail if we selected a non-matching exclusion constraint
earlier.
+ * (XXX Maybe we should do these lookups earlier so we don't
end up
+ * doing that.)
+ */
+ for_overlaps = with_period && i == numpks - 1;
+ cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
+ eqstrategy = IndexAmTranslateCompareType(cmptype, amid,
opfamily, opcintype, true);
+ if (eqstrategy == InvalidStrategy)
+ 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 family \"%s\", input type %s, access method \"%s\".",
+ cmptype,
get_opfamily_name(opfamily, false), format_type_be(opcintype),
get_am_name(amid)));
/*
* There had better be a primary equality operator for the
index.
--
2.48.1
From 61352aee28236c64ae1225caf0ef10bce9d58c12 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 4 Feb 2025 13:54:03 +0100
Subject: [PATCH v19.3 2/2] Allow non-btree speculative insertion indexes
Previously, only btrees were supported as the arbiter index for
speculative insertion because there was no way to get the equality
strategy number for other index methods. We have this now (commit
c09e5a6a016), so we can support this.
At the moment, only btree supports unique indexes, so this does not
change anything in practice, but it would allow another index method
that has amcanunique to be supported.
---
src/backend/catalog/index.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7377912b41e..cdabf780244 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2677,9 +2677,6 @@ BuildSpeculativeIndexInfo(Relation index, IndexInfo *ii)
*/
Assert(ii->ii_Unique);
- if (index->rd_rel->relam != BTREE_AM_OID)
- elog(ERROR, "unexpected non-btree speculative unique index");
-
ii->ii_UniqueOps = (Oid *) palloc(sizeof(Oid) * indnkeyatts);
ii->ii_UniqueProcs = (Oid *) palloc(sizeof(Oid) * indnkeyatts);
ii->ii_UniqueStrats = (uint16 *) palloc(sizeof(uint16) * indnkeyatts);
@@ -2691,7 +2688,12 @@ BuildSpeculativeIndexInfo(Relation index, IndexInfo *ii)
/* We need the func OIDs and strategy numbers too */
for (i = 0; i < indnkeyatts; i++)
{
- ii->ii_UniqueStrats[i] = BTEqualStrategyNumber;
+ ii->ii_UniqueStrats[i] =
+ IndexAmTranslateCompareType(COMPARE_EQ,
+
index->rd_rel->relam,
+
index->rd_opfamily[i],
+
index->rd_opcintype[i],
+
false);
ii->ii_UniqueOps[i] =
get_opfamily_member(index->rd_opfamily[i],
index->rd_opcintype[i],
--
2.48.1
