Horton
Menu

Re: Index AM API cleanup

Mark Dilger <mark.dilger@enterprisedb.com>

View thread
From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-03-12T16:08:00Z
Lists: pgsql-hackers 
On Wed, Mar 12, 2025 at 7:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Peter Eisentraut <peter@eisentraut.org> writes:
> > 0002: Add get_opfamily_member_for_cmptype().  This was called
> > get_opmethod_member() in your patch set, but I think that name wasn't
> > quite right.  I also removed the opmethod argument, which was rarely
> > used and is somewhat redundant.
>
> Hm, that will throw an error if IndexAmTranslateCompareType fails.
> Shouldn't it be made to return InvalidOid instead?
>

There are two failure modes.  In one mode, the AM has a concept of
equality, but there is no operator for the given type.  In the other mode,
the AM simply has no concept of equality.

In DefineIndex, the call:

                        if (stmt->unique && !stmt->iswithoutoverlaps)
                        {
                            idx_eqop =
get_opfamily_member_for_cmptype(idx_opfamily,

 idx_opcintype,

 idx_opcintype,

 COMPARE_EQ);
                            if (!idx_eqop)
                                ereport(ERROR,
                                        errcode(ERRCODE_UNDEFINED_OBJECT),
                                        errmsg("could not identify an
equality operator for type %s", format_type_be(idx_opcintype)),
                                        errdetail("There is no suitable
operator in operator family \"%s\" for access method \"%s\".",

get_opfamily_name(idx_opfamily, false),
get_am_name(get_opfamily_method(idx_opfamily))));
                        }

probably should error about COMPARE_EQ not having a strategy in the AM,
rather than complaining later that an equality operator cannot be found for
the type.  So that one seems fine as it is now.

The other caller, refresh_by_match_merge, is similar:

                op = get_opfamily_member_for_cmptype(opfamily, opcintype,
opcintype, COMPARE_EQ);
                if (!OidIsValid(op))
                    elog(ERROR, "missing equality operator for (%u,%u) in
opfamily %u",
                         opcintype, opcintype, opfamily);

seems fine.  There are no other callers as yet.

You have made me a bit paranoid that, in future, we might add additional
callers who are not anticipating the error.  Should we solve that with a
more complete code comment, or do you think refactoring is in order?

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Relax ordering-related hardcoded btree requirements in planning

  2. Support non-btree indexes in get_actual_variable_range()

  3. Convert PathKey to use CompareType

  4. Generalize index support in network support function

  5. Update a code comment

  6. Allow non-btree unique indexes for matviews

  7. Allow non-btree unique indexes for partition keys

  8. Add some opfamily support functions to lsyscache.c

  9. Simplify and generalize PrepareSortSupportFromIndexRel()

  10. Generalize hash and ordering support in amapi

  11. Drop opcintype from index AM strategy translation API

  12. Allow non-btree speculative insertion indexes

  13. Support non-btree indexes for foreign keys

  14. Integrate GistTranslateCompareType() into IndexAmTranslateCompareType()

  15. Convert strategies to and from compare types

  16. Move CompareType to separate header file

  17. Add get_opfamily_name() function

  18. Rename GistTranslateStratnum() to GistTranslateCompareType()

  19. Change gist stratnum function to use CompareType

  20. Rename RowCompareType to CompareType

  21. Improve slightly misleading internal error message

  22. Track scan reversals in MergeJoin

  23. Track sort direction in SortGroupClause

  24. Add stratnum GiST support function

  25. Add amgettreeheight index AM API routine