Skip to content

Commit

Permalink
fix: a set of incorrect behaviors (#430)
Browse files Browse the repository at this point in the history
* fix: set amoptionalkey to false

Signed-off-by: usamoi <usamoi@outlook.com>

* fix: set returned value of aminsert to false

Signed-off-by: usamoi <usamoi@outlook.com>

* fix: forbid scanning with a non-MVCC-compliant snapshot

Signed-off-by: usamoi <usamoi@outlook.com>

* fix: pg_vector_index_stat

Signed-off-by: usamoi <usamoi@outlook.com>

* fix: printing info instead of throwing errors in amvalidate

Signed-off-by: usamoi <usamoi@outlook.com>

* fix: revert set amoptionalkey to false

Signed-off-by: usamoi <usamoi@outlook.com>

---------

Signed-off-by: usamoi <usamoi@outlook.com>
  • Loading branch information
usamoi authored Mar 17, 2024
1 parent e4e8a5e commit 59afde9
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 180 deletions.
4 changes: 2 additions & 2 deletions crates/base/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ pub struct VectorOptions {
#[validate(range(min = 1, max = 1_048_575))]
#[serde(rename = "dimensions")]
pub dims: u32,
#[serde(rename = "distance")]
pub d: DistanceKind,
#[serde(rename = "vector")]
pub v: VectorKind,
#[serde(rename = "distance")]
pub d: DistanceKind,
}

impl VectorOptions {
Expand Down
6 changes: 5 additions & 1 deletion src/datatype/memory_bvecf32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ impl IntoDatum for BVecf32Output {
}

fn type_oid() -> Oid {
pgrx::wrappers::regtypein("vectors.bvector")
let namespace = pgrx::pg_catalog::PgNamespace::search_namespacename(c"vectors").unwrap();
let namespace = namespace.get().expect("pgvecto.rs is not installed.");
let t = pgrx::pg_catalog::PgType::search_typenamensp(c"bvector", namespace.oid()).unwrap();
let t = t.get().expect("pg_catalog is broken.");
t.oid()
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/datatype/memory_svecf32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ impl IntoDatum for SVecf32Output {
}

fn type_oid() -> Oid {
pgrx::wrappers::regtypein("vectors.svector")
let namespace = pgrx::pg_catalog::PgNamespace::search_namespacename(c"vectors").unwrap();
let namespace = namespace.get().expect("pgvecto.rs is not installed.");
let t = pgrx::pg_catalog::PgType::search_typenamensp(c"svector", namespace.oid()).unwrap();
let t = t.get().expect("pg_catalog is broken.");
t.oid()
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/datatype/memory_vecf16.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ impl IntoDatum for Vecf16Output {
}

fn type_oid() -> Oid {
pgrx::wrappers::regtypein("vectors.vecf16")
let namespace = pgrx::pg_catalog::PgNamespace::search_namespacename(c"vectors").unwrap();
let namespace = namespace.get().expect("pgvecto.rs is not installed.");
let t = pgrx::pg_catalog::PgType::search_typenamensp(c"vecf16", namespace.oid()).unwrap();
let t = t.get().expect("pg_catalog is broken.");
t.oid()
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/datatype/memory_vecf32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ impl IntoDatum for Vecf32Output {
}

fn type_oid() -> Oid {
pgrx::wrappers::regtypein("vectors.vector")
let namespace = pgrx::pg_catalog::PgNamespace::search_namespacename(c"vectors").unwrap();
let namespace = namespace.get().expect("pgvecto.rs is not installed.");
let t = pgrx::pg_catalog::PgType::search_typenamensp(c"vector", namespace.oid()).unwrap();
let t = t.get().expect("pg_catalog is broken.");
t.oid()
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/datatype/memory_veci8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ impl IntoDatum for Veci8Output {
}

fn type_oid() -> Oid {
pgrx::wrappers::regtypein("vectors.veci8")
let namespace = pgrx::pg_catalog::PgNamespace::search_namespacename(c"vectors").unwrap();
let namespace = namespace.get().expect("pgvecto.rs is not installed.");
let t = pgrx::pg_catalog::PgType::search_typenamensp(c"veci8", namespace.oid()).unwrap();
let t = t.get().expect("pg_catalog is broken.");
t.oid()
}
}

Expand Down
9 changes: 0 additions & 9 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,6 @@ pgvecto.rs: Dimensions type modifier of a vector column is needed for building t
}
}

pub fn bad_opclass() -> ! {
error!(
"\
pgvecto.rs: Indexes can only be built on built-in distance functions.
ADVICE: If you want pgvecto.rs to support more distance functions, \
visit `https://github.com/tensorchord/pgvecto.rs/issues` and contribute your ideas."
);
}

pub fn bad_service_not_exist() -> ! {
error!(
"\
Expand Down
32 changes: 15 additions & 17 deletions src/index/am.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,16 @@ const AM_HANDLER: pgrx::pg_sys::IndexAmRoutine = {

am_routine.type_ = pgrx::pg_sys::NodeTag::T_IndexAmRoutine;

am_routine.amstrategies = 1;
am_routine.amsupport = 0;

am_routine.amcanorder = false;
am_routine.amcanorderbyop = true;
am_routine.amcanbackward = false;
am_routine.amcanunique = false;
am_routine.amcanmulticol = false;

// Index access methods that set `amoptionalkey` to `false`
// must index all tuples, even if the first column is `NULL`.
// However, PostgreSQL does not generate a path if there is no
// index clauses, even if there is a `ORDER BY` clause.
// So we have to set it to `true` and set costs of every path
// for vector index scans without `ORDER BY` clauses a large number
// and throw errors if someone really wants such a path.
am_routine.amoptionalkey = true;
am_routine.amsearcharray = false;
am_routine.amsearchnulls = false;
am_routine.amstorage = false;
am_routine.amclusterable = false;
am_routine.ampredlocks = false;
am_routine.amcaninclude = false;
am_routine.amkeytype = pgrx::pg_sys::InvalidOid;

am_routine.amvalidate = Some(amvalidate);
am_routine.amoptions = Some(amoptions);
Expand All @@ -82,8 +76,12 @@ const AM_HANDLER: pgrx::pg_sys::IndexAmRoutine = {

#[pgrx::pg_guard]
pub unsafe extern "C" fn amvalidate(opclass_oid: pgrx::pg_sys::Oid) -> bool {
am_setup::convert_opclass_to_distance(opclass_oid);
true
if am_setup::convert_opclass_to_vd(opclass_oid).is_some() {
pgrx::info!("Vector indexes can only be built on built-in operator classes.");
true
} else {
false
}
}

#[pgrx::pg_guard]
Expand Down Expand Up @@ -210,7 +208,7 @@ pub unsafe extern "C" fn aminsert(
if let Some(v) = vector {
am_update::update_insert(id, v, *heap_tid);
}
true
false
}

#[pgrx::pg_guard]
Expand Down
8 changes: 8 additions & 0 deletions src/index/am_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ pub unsafe fn next_scan(scan: pgrx::pg_sys::IndexScanDesc) -> bool {
let scanner = &mut *((*scan).opaque as *mut Scanner);
if let Scanner::Initial { vector } = scanner {
if let Some(vector) = vector.as_ref() {
// https://www.postgresql.org/docs/current/index-locking.html
// If heap entries referenced physical pointers are deleted before
// they are consumed by PostgreSQL, PostgreSQL will received wrong
// physical pointers: no rows or irreverent rows are referenced.
if (*(*scan).xs_snapshot).snapshot_type != pgrx::pg_sys::SnapshotType_SNAPSHOT_MVCC {
pgrx::error!("scanning with a non-MVCC-compliant snapshot is not supported");
}

let oid = (*(*scan).indexRelation).rd_id;
let id = get_handle(oid);

Expand Down
Loading

0 comments on commit 59afde9

Please sign in to comment.