From f0252a04b3c5fc46bf89e1f793e325cbbfd092d5 Mon Sep 17 00:00:00 2001 From: Andrew Kane Date: Wed, 21 Oct 2026 00:11:26 -0700 Subject: [PATCH] Improved amvalidate for indexes --- CHANGELOG.md | 1 + src/hnsw.c | 141 +++++++++++++++++++++++++++++++- src/ivfflat.c | 147 +++++++++++++++++++++++++++++++++- test/expected/hnsw_vector.out | 6 ++ test/sql/hnsw_vector.sql | 4 + 5 files changed, 297 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f58998..fbff6db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## 0.9.0 (unreleased) +- Improved `amvalidate` for indexes - Dropped support for Postgres 13 and 14 ## 0.8.7 (2026-10-01) diff --git a/src/hnsw.c b/src/hnsw.c index 55c7519..c4dae47 100644 --- a/src/hnsw.c +++ b/src/hnsw.c @@ -5,8 +5,13 @@ #include #include "access/amapi.h" +#include "access/amvalidate.h" #include "access/genam.h" +#include "access/htup_details.h" #include "access/reloptions.h" +#include "catalog/pg_amop.h" +#include "catalog/pg_amproc.h" +#include "catalog/pg_opclass.h" #include "commands/progress.h" #include "commands/vacuum.h" #include "fmgr.h" @@ -14,13 +19,21 @@ #include "miscadmin.h" #include "nodes/pg_list.h" #include "storage/lwlock.h" +#include "utils/catcache.h" #include "utils/float.h" #include "utils/guc.h" +#include "utils/lsyscache.h" +#include "utils/regproc.h" #include "utils/relcache.h" #include "utils/selfuncs.h" #include "utils/spccache.h" +#include "utils/syscache.h" #include "vector.h" +#if PG_VERSION_NUM < 180000 +#include "catalog/pg_opfamily.h" +#endif + static const struct config_enum_entry hnsw_iterative_scan_options[] = { {"off", HNSW_ITERATIVE_SCAN_OFF, false}, {"relaxed_order", HNSW_ITERATIVE_SCAN_RELAXED, false}, @@ -251,7 +264,133 @@ hnswoptions(Datum reloptions, bool validate) static bool hnswvalidate(Oid opclassoid) { - return true; + bool result = true; + HeapTuple classtup; + Form_pg_opclass classform; + Oid opcintype; + Oid opfamilyoid; +#if PG_VERSION_NUM < 180000 + HeapTuple familytup; + Form_pg_opfamily familyform; +#endif + char *opfamilyname; + CatCList *proclist; + CatCList *oprlist; + + /* Fetch opclass information */ + classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid)); + if (!HeapTupleIsValid(classtup)) + elog(ERROR, "cache lookup failed for operator class %u", opclassoid); + classform = (Form_pg_opclass) GETSTRUCT(classtup); + opcintype = classform->opcintype; + + /* Fetch opfamily information */ + opfamilyoid = classform->opcfamily; +#if PG_VERSION_NUM >= 180000 + opfamilyname = get_opfamily_name(opfamilyoid, false); +#else + 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); +#endif + + /* Fetch all operators and support functions of the opfamily */ + oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid)); + proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid)); + + /* Check individual support functions */ + for (int i = 0; i < proclist->n_members; i++) + { + HeapTuple proctup = &proclist->members[i]->tuple; + Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup); + bool ok; + + /* Check procedure numbers and function signatures */ + switch (procform->amprocnum) + { + case HNSW_DISTANCE_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 2, 2, opcintype, opcintype); + break; + case HNSW_NORM_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 1, 1, opcintype); + break; + case HNSW_TYPE_INFO_PROC: + ok = check_amproc_signature(procform->amproc, INTERNALOID, true, 1, 1, INTERNALOID); + break; + default: + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains function %s with invalid support number %d", + opfamilyname, "hnsw", + format_procedure(procform->amproc), + procform->amprocnum))); + result = false; + continue; + } + + if (!ok) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains function %s with wrong signature for support number %d", + opfamilyname, "hnsw", + format_procedure(procform->amproc), + procform->amprocnum))); + result = false; + } + } + + /* Check individual operators */ + for (int i = 0; i < oprlist->n_members; i++) + { + HeapTuple oprtup = &oprlist->members[i]->tuple; + Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); + + /* Check strategy number */ + if (oprform->amopstrategy != 1) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains operator %s with invalid strategy number %d", + opfamilyname, "hnsw", + format_operator(oprform->amopopr), + oprform->amopstrategy))); + result = false; + } + + /* Check sort family */ + if (oprform->amoppurpose != AMOP_ORDER || !opfamily_can_sort_type(oprform->amopsortfamily, FLOAT8OID)) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains invalid ORDER BY specification for operator %s", + opfamilyname, "hnsw", + format_operator(oprform->amopopr)))); + result = false; + } + + /* Check operator signature */ + if (!check_amop_signature(oprform->amopopr, FLOAT8OID, opcintype, opcintype)) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains operator %s with wrong signature", + opfamilyname, "hnsw", + format_operator(oprform->amopopr)))); + result = false; + } + } + + ReleaseCatCacheList(proclist); + ReleaseCatCacheList(oprlist); +#if PG_VERSION_NUM < 180000 + ReleaseSysCache(familytup); +#endif + ReleaseSysCache(classtup); + + return result; } /* diff --git a/src/ivfflat.c b/src/ivfflat.c index e5285a0..a4a9399 100644 --- a/src/ivfflat.c +++ b/src/ivfflat.c @@ -3,20 +3,33 @@ #include #include "access/amapi.h" +#include "access/amvalidate.h" #include "access/genam.h" +#include "access/htup_details.h" #include "access/reloptions.h" +#include "catalog/pg_amop.h" +#include "catalog/pg_amproc.h" +#include "catalog/pg_opclass.h" #include "commands/progress.h" #include "commands/vacuum.h" #include "fmgr.h" #include "ivfflat.h" #include "nodes/pg_list.h" +#include "utils/catcache.h" #include "utils/float.h" #include "utils/guc.h" +#include "utils/lsyscache.h" +#include "utils/regproc.h" #include "utils/relcache.h" #include "utils/selfuncs.h" #include "utils/spccache.h" +#include "utils/syscache.h" #include "vector.h" +#if PG_VERSION_NUM < 180000 +#include "catalog/pg_opfamily.h" +#endif + int ivfflat_probes; int ivfflat_iterative_scan; int ivfflat_max_probes; @@ -168,7 +181,139 @@ ivfflatoptions(Datum reloptions, bool validate) static bool ivfflatvalidate(Oid opclassoid) { - return true; + bool result = true; + HeapTuple classtup; + Form_pg_opclass classform; + Oid opcintype; + Oid opfamilyoid; +#if PG_VERSION_NUM < 180000 + HeapTuple familytup; + Form_pg_opfamily familyform; +#endif + char *opfamilyname; + CatCList *proclist; + CatCList *oprlist; + + /* Fetch opclass information */ + classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid)); + if (!HeapTupleIsValid(classtup)) + elog(ERROR, "cache lookup failed for operator class %u", opclassoid); + classform = (Form_pg_opclass) GETSTRUCT(classtup); + opcintype = classform->opcintype; + + /* Fetch opfamily information */ + opfamilyoid = classform->opcfamily; +#if PG_VERSION_NUM >= 180000 + opfamilyname = get_opfamily_name(opfamilyoid, false); +#else + 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); +#endif + + /* Fetch all operators and support functions of the opfamily */ + oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid)); + proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid)); + + /* Check individual support functions */ + for (int i = 0; i < proclist->n_members; i++) + { + HeapTuple proctup = &proclist->members[i]->tuple; + Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup); + bool ok; + + /* Check procedure numbers and function signatures */ + switch (procform->amprocnum) + { + case IVFFLAT_DISTANCE_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 2, 2, opcintype, opcintype); + break; + case IVFFLAT_NORM_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 1, 1, opcintype); + break; + case IVFFLAT_KMEANS_DISTANCE_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 2, 2, opcintype, opcintype); + break; + case IVFFLAT_KMEANS_NORM_PROC: + ok = check_amproc_signature(procform->amproc, FLOAT8OID, true, 1, 1, opcintype); + break; + case IVFFLAT_TYPE_INFO_PROC: + ok = check_amproc_signature(procform->amproc, INTERNALOID, true, 1, 1, INTERNALOID); + break; + default: + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains function %s with invalid support number %d", + opfamilyname, "ivfflat", + format_procedure(procform->amproc), + procform->amprocnum))); + result = false; + continue; + } + + if (!ok) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains function %s with wrong signature for support number %d", + opfamilyname, "ivfflat", + format_procedure(procform->amproc), + procform->amprocnum))); + result = false; + } + } + + /* Check individual operators */ + for (int i = 0; i < oprlist->n_members; i++) + { + HeapTuple oprtup = &oprlist->members[i]->tuple; + Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup); + + /* Check strategy number */ + if (oprform->amopstrategy != 1) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains operator %s with invalid strategy number %d", + opfamilyname, "ivfflat", + format_operator(oprform->amopopr), + oprform->amopstrategy))); + result = false; + } + + /* Check sort family */ + if (oprform->amoppurpose != AMOP_ORDER || !opfamily_can_sort_type(oprform->amopsortfamily, FLOAT8OID)) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains invalid ORDER BY specification for operator %s", + opfamilyname, "ivfflat", + format_operator(oprform->amopopr)))); + result = false; + } + + /* Check operator signature */ + if (!check_amop_signature(oprform->amopopr, FLOAT8OID, opcintype, opcintype)) + { + ereport(INFO, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("operator family \"%s\" of access method %s contains operator %s with wrong signature", + opfamilyname, "ivfflat", + format_operator(oprform->amopopr)))); + result = false; + } + } + + ReleaseCatCacheList(proclist); + ReleaseCatCacheList(oprlist); +#if PG_VERSION_NUM < 180000 + ReleaseSysCache(familytup); +#endif + ReleaseSysCache(classtup); + + return result; } /* diff --git a/test/expected/hnsw_vector.out b/test/expected/hnsw_vector.out index e129982..0237681 100644 --- a/test/expected/hnsw_vector.out +++ b/test/expected/hnsw_vector.out @@ -216,3 +216,9 @@ CREATE TABLE t (val vector(2001)); CREATE INDEX ON t USING hnsw (val vector_l2_ops); ERROR: column cannot have more than 2000 dimensions for hnsw index DROP TABLE t; +-- opclass validation +SELECT opcname FROM pg_opclass WHERE NOT amvalidate(oid); + opcname +--------- +(0 rows) + diff --git a/test/sql/hnsw_vector.sql b/test/sql/hnsw_vector.sql index 848ebc9..7ccfb34 100644 --- a/test/sql/hnsw_vector.sql +++ b/test/sql/hnsw_vector.sql @@ -122,3 +122,7 @@ DROP TABLE t; CREATE TABLE t (val vector(2001)); CREATE INDEX ON t USING hnsw (val vector_l2_ops); DROP TABLE t; + +-- opclass validation + +SELECT opcname FROM pg_opclass WHERE NOT amvalidate(oid);