Re: Bug: When user-defined AM is used, the index path cannot be selected correctly - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Bug: When user-defined AM is used, the index path cannot be selected correctly
Date
Msg-id 635919.1662064398@sss.pgh.pa.us
Whole thread Raw
In response to Re: Bug: When user-defined AM is used, the index path cannot be selected correctly  (Quan Zongliang <quanzongliang@yeah.net>)
Responses Re: Bug: When user-defined AM is used, the index path cannot be selected correctly
List pgsql-hackers
Quan Zongliang <quanzongliang@yeah.net> writes:
> New patch attached.
> It seems that partitions do not use AM other than btree and hash.
> Rewrite only indxpath.c and check if it is a custom AM.

This seems drastically underdocumented, and the test you're using
for extension opclasses is wrong.  What we need to know before
applying match_boolean_index_clause is that a clause using
BooleanEqualOperator will be valid for the index.  That's got next
door to nothing to do with whether the opclass is default for
the index AM.  A non-default opclass might support that operator,
and conversely a default one might not (although I concede it's
not that easy to imagine what other set of operators-on-boolean
an extension opclass might be interested in).

I think we need something more like the attached.

            regards, tom lane

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 045ff2e487..63a8eef45c 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -153,6 +153,7 @@ static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
                                              RestrictInfo *rinfo,
                                              int indexcol,
                                              IndexOptInfo *index);
+static bool IsBooleanOpfamily(Oid opfamily);
 static IndexClause *match_boolean_index_clause(PlannerInfo *root,
                                                RestrictInfo *rinfo,
                                                int indexcol, IndexOptInfo *index);
@@ -2342,6 +2343,23 @@ match_clause_to_indexcol(PlannerInfo *root,
     return NULL;
 }

+/*
+ * IsBooleanOpfamily
+ *      Detect whether an opfamily supports boolean equality as an operator.
+ *
+ * If the opfamily OID is in the range of built-in objects, we can rely
+ * on hard-wired knowledge of which built-in opfamilies support this.
+ * For extension opfamilies, there's no choice but to do a catcache lookup.
+ */
+static bool
+IsBooleanOpfamily(Oid opfamily)
+{
+    if (opfamily < FirstNormalObjectId)
+        return IsBuiltinBooleanOpfamily(opfamily);
+    else
+        return op_in_opfamily(BooleanEqualOperator, opfamily);
+}
+
 /*
  * match_boolean_index_clause
  *      Recognize restriction clauses that can be matched to a boolean index.
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 1fa7fc99b5..b9c1ce0a64 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -1191,8 +1191,13 @@ partkey_is_bool_constant_for_query(RelOptInfo *partrel, int partkeycol)
     PartitionScheme partscheme = partrel->part_scheme;
     ListCell   *lc;

-    /* If the partkey isn't boolean, we can't possibly get a match */
-    if (!IsBooleanOpfamily(partscheme->partopfamily[partkeycol]))
+    /*
+     * If the partkey isn't boolean, we can't possibly get a match.
+     *
+     * Partitioning currently can only use built-in AMs, so checking for
+     * built-in boolean opclasses is good enough.
+     */
+    if (!IsBuiltinBooleanOpfamily(partscheme->partopfamily[partkeycol]))
         return false;

     /* Check each restriction clause for the partitioned rel */
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index bf9fe5b7aa..b5403149d7 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -3596,7 +3596,11 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,

     *outconst = NULL;

-    if (!IsBooleanOpfamily(partopfamily))
+    /*
+     * Partitioning currently can only use built-in AMs, so checking for
+     * built-in boolean opclasses is good enough.
+     */
+    if (!IsBuiltinBooleanOpfamily(partopfamily))
         return PARTCLAUSE_UNSUPPORTED;

     if (IsA(clause, BooleanTest))
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 8dc9ce01bb..b56d714322 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -55,7 +55,8 @@ DECLARE_UNIQUE_INDEX_PKEY(pg_opfamily_oid_index, 2755, OpfamilyOidIndexId, on pg

 #ifdef EXPOSE_TO_CLIENT_CODE

-#define IsBooleanOpfamily(opfamily) \
+/* This does not account for non-core opclasses that might accept boolean */
+#define IsBuiltinBooleanOpfamily(opfamily) \
     ((opfamily) == BOOL_BTREE_FAM_OID || (opfamily) == BOOL_HASH_FAM_OID)

 #endif                            /* EXPOSE_TO_CLIENT_CODE */

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: introduce bufmgr hooks
Next
From: Jeff Davis
Date:
Subject: Re: pg_auth_members.grantor is bunk