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

I wrote:
> I think we need something more like the attached.

Meh -- serves me right for not doing check-world before sending.
The patch causes some plans to change in the btree_gin and btree_gist
modules; which is good, because that shows that the patch is actually
doing what it's supposed to.  The fact that your patch didn't make
the cfbot unhappy implies that it wasn't triggering for those modules.
I think the reason is that you did

+        ((amid) >= FirstNormalObjectId && \
+                OidIsValid(GetDefaultOpClass(BOOLOID, (amid)))) \

so that the FirstNormalObjectId cutoff was applied to the AM's OID,
not to the opfamily OID, causing it to do the wrong thing for
extension opclasses over built-in AMs.

The good news is this means we don't need to worry about making
a test case ...

            regards, tom lane

diff --git a/contrib/btree_gin/expected/bool.out b/contrib/btree_gin/expected/bool.out
index efb3e1e327..207a3f2328 100644
--- a/contrib/btree_gin/expected/bool.out
+++ b/contrib/btree_gin/expected/bool.out
@@ -87,13 +87,15 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i<=true ORDER BY i;
 (6 rows)

 EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i=true ORDER BY i;
-         QUERY PLAN
------------------------------
+                QUERY PLAN
+-------------------------------------------
  Sort
    Sort Key: i
-   ->  Seq Scan on test_bool
+   ->  Bitmap Heap Scan on test_bool
          Filter: i
-(4 rows)
+         ->  Bitmap Index Scan on idx_bool
+               Index Cond: (i = true)
+(6 rows)

 EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i>=true ORDER BY i;
                 QUERY PLAN
diff --git a/contrib/btree_gist/expected/bool.out b/contrib/btree_gist/expected/bool.out
index c67a9685d5..29390f079c 100644
--- a/contrib/btree_gist/expected/bool.out
+++ b/contrib/btree_gist/expected/bool.out
@@ -71,7 +71,7 @@ SELECT * FROM booltmp WHERE a;
                 QUERY PLAN
 ------------------------------------------
  Index Only Scan using boolidx on booltmp
-   Filter: a
+   Index Cond: (a = true)
 (2 rows)

 SELECT * FROM booltmp WHERE a;
@@ -85,7 +85,7 @@ SELECT * FROM booltmp WHERE NOT a;
                 QUERY PLAN
 ------------------------------------------
  Index Only Scan using boolidx on booltmp
-   Filter: (NOT a)
+   Index Cond: (a = false)
 (2 rows)

 SELECT * FROM booltmp WHERE NOT a;
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 */