From c66a6de0bd7275263e3a417d767182a2afcd34fb Mon Sep 17 00:00:00 2001 From: Andrew Kane Date: Tue, 13 Feb 2024 13:07:30 -0800 Subject: [PATCH] Tried switching column order [skip ci] --- sql/vector--0.6.1--0.7.0.sql | 4 ++++ sql/vector.sql | 4 ++++ src/hnswbuild.c | 18 +++++++++--------- src/hnswinsert.c | 12 ++++++------ src/hnswscan.c | 4 ++-- src/hnswutils.c | 10 +++++----- src/hnswvacuum.c | 4 ++-- test/t/020_hnsw_filtering.pl | 14 +++++++------- 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/sql/vector--0.6.1--0.7.0.sql b/sql/vector--0.6.1--0.7.0.sql index 2b3fa8e..9186e54 100644 --- a/sql/vector--0.6.1--0.7.0.sql +++ b/sql/vector--0.6.1--0.7.0.sql @@ -4,3 +4,7 @@ CREATE OPERATOR CLASS vector_integer_ops DEFAULT FOR TYPE integer USING hnsw AS OPERATOR 2 = (integer, integer); + +CREATE OPERATOR CLASS vector_bigint_ops + DEFAULT FOR TYPE bigint USING hnsw AS + OPERATOR 2 = (bigint, bigint); diff --git a/sql/vector.sql b/sql/vector.sql index bc5ede2..4e3d733 100644 --- a/sql/vector.sql +++ b/sql/vector.sql @@ -296,3 +296,7 @@ CREATE OPERATOR CLASS vector_cosine_ops CREATE OPERATOR CLASS vector_integer_ops DEFAULT FOR TYPE integer USING hnsw AS OPERATOR 2 = (integer, integer); + +CREATE OPERATOR CLASS vector_bigint_ops + DEFAULT FOR TYPE bigint USING hnsw AS + OPERATOR 2 = (bigint, bigint); diff --git a/src/hnswbuild.c b/src/hnswbuild.c index f7b0aaa..78b98c9 100644 --- a/src/hnswbuild.c +++ b/src/hnswbuild.c @@ -481,7 +481,7 @@ InsertTuple(Relation index, Datum *values, bool *isnull, ItemPointer heaptid, Hn bool unused; /* Detoast once for all calls */ - Datum value = PointerGetDatum(PG_DETOAST_DATUM(values[0])); + Datum value = PointerGetDatum(PG_DETOAST_DATUM(values[IndexRelationGetNumberOfKeyAttributes(index) - 1])); /* Normalize if needed */ if (buildstate->normprocinfo != NULL) @@ -551,7 +551,7 @@ InsertTuple(Relation index, Datum *values, bool *isnull, ItemPointer heaptid, Hn /* Copy the index tuple */ memcpy(itupPtr, itup, itupSize); HnswPtrStore(base, element->itup, itupPtr); - valuePtr = DatumGetPointer(index_getattr(itupPtr, 1, tupdesc, &unused)); + valuePtr = DatumGetPointer(index_getattr(itupPtr, IndexRelationGetNumberOfKeyAttributes(index), tupdesc, &unused)); HnswPtrStore(base, element->value, valuePtr); /* Create a lock for the element */ @@ -582,7 +582,7 @@ BuildCallback(Relation index, CALLBACK_ITEM_POINTER, Datum *values, #endif /* Skip nulls */ - if (isnull[0]) + if (isnull[IndexRelationGetNumberOfKeyAttributes(index) - 1]) return; /* Use memory context */ @@ -674,16 +674,16 @@ InitBuildState(HnswBuildState * buildstate, Relation heap, Relation index, Index buildstate->m = HnswGetM(index); buildstate->efConstruction = HnswGetEfConstruction(index); - buildstate->dimensions = TupleDescAttr(index->rd_att, 0)->atttypmod; + buildstate->dimensions = TupleDescAttr(index->rd_att, IndexRelationGetNumberOfKeyAttributes(index) - 1)->atttypmod; /* For now */ if (IndexRelationGetNumberOfKeyAttributes(index) > 2) elog(ERROR, "index cannot have more than two columns"); - if (!OidIsValid(index_getprocid(index, 1, HNSW_DISTANCE_PROC))) - elog(ERROR, "first column must be a vector"); + if (!OidIsValid(index_getprocid(index, IndexRelationGetNumberOfKeyAttributes(index), HNSW_DISTANCE_PROC))) + elog(ERROR, "last column must be a vector"); - for (int i = 1; i < IndexRelationGetNumberOfKeyAttributes(index); i++) + for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(index) - 1; i++) { if (OidIsValid(index_getprocid(index, i + 1, HNSW_DISTANCE_PROC))) elog(ERROR, "column %d cannot be a vector", i + 1); @@ -703,9 +703,9 @@ InitBuildState(HnswBuildState * buildstate, Relation heap, Relation index, Index buildstate->indtuples = 0; /* Get support functions */ - buildstate->procinfo = index_getprocinfo(index, 1, HNSW_DISTANCE_PROC); + buildstate->procinfo = index_getprocinfo(index, IndexRelationGetNumberOfKeyAttributes(index), HNSW_DISTANCE_PROC); buildstate->normprocinfo = HnswOptionalProcInfo(index, HNSW_NORM_PROC); - buildstate->collation = index->rd_indcollation[0]; + buildstate->collation = index->rd_indcollation[IndexRelationGetNumberOfKeyAttributes(index) - 1]; InitGraph(&buildstate->graphData, NULL, maintenance_work_mem * 1024L); buildstate->graph = &buildstate->graphData; diff --git a/src/hnswinsert.c b/src/hnswinsert.c index 5024c68..3648f7f 100644 --- a/src/hnswinsert.c +++ b/src/hnswinsert.c @@ -561,8 +561,8 @@ HnswInsertTupleOnDisk(Relation index, Datum value, Datum *values, bool *isnull, HnswElement element; int m; int efConstruction = HnswGetEfConstruction(index); - FmgrInfo *procinfo = index_getprocinfo(index, 1, HNSW_DISTANCE_PROC); - Oid collation = index->rd_indcollation[0]; + FmgrInfo *procinfo = index_getprocinfo(index, IndexRelationGetNumberOfKeyAttributes(index), HNSW_DISTANCE_PROC); + Oid collation = index->rd_indcollation[IndexRelationGetNumberOfKeyAttributes(index) - 1]; LOCKMODE lockmode = ShareLock; char *base = NULL; TupleDesc tupdesc = HnswTupleDesc(index); @@ -584,7 +584,7 @@ HnswInsertTupleOnDisk(Relation index, Datum value, Datum *values, bool *isnull, element = HnswInitElement(base, heap_tid, m, HnswGetMl(m), HnswGetMaxLevel(m), NULL); itup = HnswFormIndexTuple(index, tupdesc, value, values, isnull); HnswPtrStore(base, element->itup, itup); - valuePtr = DatumGetPointer(index_getattr(itup, 1, tupdesc, &unused)); + valuePtr = DatumGetPointer(index_getattr(itup, IndexRelationGetNumberOfKeyAttributes(index), tupdesc, &unused)); HnswPtrStore(base, element->value, valuePtr); /* Prevent concurrent inserts when likely updating entry point */ @@ -621,10 +621,10 @@ HnswInsertTuple(Relation index, Datum *values, bool *isnull, ItemPointer heap_ti { Datum value; FmgrInfo *normprocinfo; - Oid collation = index->rd_indcollation[0]; + Oid collation = index->rd_indcollation[IndexRelationGetNumberOfKeyAttributes(index) - 1]; /* Detoast once for all calls */ - value = PointerGetDatum(PG_DETOAST_DATUM(values[0])); + value = PointerGetDatum(PG_DETOAST_DATUM(values[IndexRelationGetNumberOfKeyAttributes(index) - 1])); /* Normalize if needed */ normprocinfo = HnswOptionalProcInfo(index, HNSW_NORM_PROC); @@ -653,7 +653,7 @@ hnswinsert(Relation index, Datum *values, bool *isnull, ItemPointer heap_tid, MemoryContext insertCtx; /* Skip nulls */ - if (isnull[0]) + if (isnull[IndexRelationGetNumberOfKeyAttributes(index) - 1]) return false; /* Create memory context */ diff --git a/src/hnswscan.c b/src/hnswscan.c index 8df6dfe..6bd6f28 100644 --- a/src/hnswscan.c +++ b/src/hnswscan.c @@ -109,9 +109,9 @@ hnswbeginscan(Relation index, int nkeys, int norderbys) ALLOCSET_DEFAULT_SIZES); /* Set support functions */ - so->procinfo = index_getprocinfo(index, 1, HNSW_DISTANCE_PROC); + so->procinfo = index_getprocinfo(index, IndexRelationGetNumberOfKeyAttributes(index), HNSW_DISTANCE_PROC); so->normprocinfo = HnswOptionalProcInfo(index, HNSW_NORM_PROC); - so->collation = index->rd_indcollation[0]; + so->collation = index->rd_indcollation[IndexRelationGetNumberOfKeyAttributes(index) - 1]; scan->opaque = so; diff --git a/src/hnswutils.c b/src/hnswutils.c index 99dc242..21fc394 100644 --- a/src/hnswutils.c +++ b/src/hnswutils.c @@ -148,10 +148,10 @@ HnswGetEfConstruction(Relation index) FmgrInfo * HnswOptionalProcInfo(Relation index, uint16 procnum) { - if (!OidIsValid(index_getprocid(index, 1, procnum))) + if (!OidIsValid(index_getprocid(index, IndexRelationGetNumberOfKeyAttributes(index), procnum))) return NULL; - return index_getprocinfo(index, 1, procnum); + return index_getprocinfo(index, IndexRelationGetNumberOfKeyAttributes(index), procnum); } /* @@ -324,7 +324,7 @@ HnswFormIndexTuple(Relation index, TupleDesc tupdesc, Datum value, Datum *values Datum *newValues = palloc(size); memcpy(newValues, values, size); - newValues[0] = value; + newValues[IndexRelationGetNumberOfKeyAttributes(index) - 1] = value; return index_form_tuple(tupdesc, newValues, isnull); } @@ -642,7 +642,7 @@ HnswLoadElementFromTuple(HnswElement element, HnswElementTuple etup, bool loadHe TupleDesc tupdesc = RelationGetDescr(index); bool unused; IndexTuple itup = CopyIndexTuple((IndexTuple) &etup->data); - Datum value = index_getattr(itup, 1, tupdesc, &unused); + Datum value = index_getattr(itup, IndexRelationGetNumberOfKeyAttributes(index), tupdesc, &unused); HnswPtrStore(base, element->itup, itup); HnswPtrStore(base, element->value, DatumGetPointer(value)); @@ -722,7 +722,7 @@ HnswLoadElement(HnswElement element, float *distance, Datum *q, Relation index, TupleDesc tupdesc = RelationGetDescr(index); bool unused; - value = index_getattr(itup, 1, tupdesc, &unused); + value = index_getattr(itup, IndexRelationGetNumberOfKeyAttributes(index), tupdesc, &unused); } else value = PointerGetDatum(&etup->data); diff --git a/src/hnswvacuum.c b/src/hnswvacuum.c index 68afec8..178f58c 100644 --- a/src/hnswvacuum.c +++ b/src/hnswvacuum.c @@ -585,8 +585,8 @@ InitVacuumState(HnswVacuumState * vacuumstate, IndexVacuumInfo *info, IndexBulkD vacuumstate->callback_state = callback_state; vacuumstate->efConstruction = HnswGetEfConstruction(index); vacuumstate->bas = GetAccessStrategy(BAS_BULKREAD); - vacuumstate->procinfo = index_getprocinfo(index, 1, HNSW_DISTANCE_PROC); - vacuumstate->collation = index->rd_indcollation[0]; + vacuumstate->procinfo = index_getprocinfo(index, IndexRelationGetNumberOfKeyAttributes(index), HNSW_DISTANCE_PROC); + vacuumstate->collation = index->rd_indcollation[IndexRelationGetNumberOfKeyAttributes(index) - 1]; vacuumstate->ntup = palloc0(HNSW_TUPLE_ALLOC_SIZE); vacuumstate->tmpCtx = AllocSetContextCreate(CurrentMemoryContext, "Hnsw vacuum temporary context", diff --git a/test/t/020_hnsw_filtering.pl b/test/t/020_hnsw_filtering.pl index f85f9a3..77ae391 100644 --- a/test/t/020_hnsw_filtering.pl +++ b/test/t/020_hnsw_filtering.pl @@ -58,11 +58,11 @@ $node->start; # Create table $node->safe_psql("postgres", "CREATE EXTENSION vector;"); -$node->safe_psql("postgres", "CREATE TABLE tst (i int4, v vector($dim), c int4);"); +$node->safe_psql("postgres", "CREATE TABLE tst (i int4, v vector($dim), c int8);"); $node->safe_psql("postgres", "INSERT INTO tst SELECT i, ARRAY[$array_sql], i % $nc FROM generate_series(1, 10000) i;" ); -$node->safe_psql("postgres", "CREATE INDEX ON tst USING hnsw (v vector_l2_ops, c);"); +$node->safe_psql("postgres", "CREATE INDEX ON tst USING hnsw (c, v vector_l2_ops);"); $node->safe_psql("postgres", "INSERT INTO tst SELECT i, ARRAY[$array_sql], i % $nc FROM generate_series(1, 10000) i;" ); @@ -99,15 +99,15 @@ $node->safe_psql("postgres", "VACUUM tst;"); # Test columns my ($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (c);"); -like($stderr, qr/first column must be a vector/); +like($stderr, qr/last column must be a vector/); -($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (c, v vector_l2_ops);"); -like($stderr, qr/first column must be a vector/); +($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (c, c);"); +like($stderr, qr/last column must be a vector/); -($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (v vector_l2_ops, c, c);"); +($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (c, c, v vector_l2_ops);"); like($stderr, qr/index cannot have more than two columns/); ($ret, $stdout, $stderr) = $node->psql("postgres", "CREATE INDEX ON tst USING hnsw (v vector_l2_ops, v vector_l2_ops);"); -like($stderr, qr/column 2 cannot be a vector/); +like($stderr, qr/column 1 cannot be a vector/); done_testing();