JingsongLi commented on code in PR #726:
URL: https://github.com/apache/paimon-rust/pull/726#discussion_r3800982908
##########
crates/paimon/src/vindex/mod.rs:
##########
@@ -249,17 +310,33 @@ fn is_supported_user_option_key(key: &str, index_type:
&str, field_name: &str) -
fn is_allowed_native_key(key: &str, index_type: &str) -> bool {
match key {
- "dimension" | "nlist" | "metric" => true,
- "pq.m" | "use-opq" => index_type == IVF_PQ_IDENTIFIER,
- _ => false,
+ "dimension" | "metric" => true,
+ "nlist" => index_type != DISKANN_IDENTIFIER,
+ "use-opq" => index_type == IVF_PQ_IDENTIFIER,
+ "rq.bits" => index_type == IVF_RQ_IDENTIFIER,
+ "pq.m" if index_type == IVF_PQ_IDENTIFIER => true,
+ _ => {
+ index_type == DISKANN_IDENTIFIER
+ && DISKANN_OPTION_KEYS
+ .iter()
+ .any(|(native_key, _)| *native_key == key)
+ }
}
}
fn is_allowed_paimon_suffix(suffix: &str, index_type: &str) -> bool {
match suffix {
- "dimension" | "nlist" | "distance.metric" | "train.sample-ratio" =>
true,
- "pq.m" | "pq.use-opq" => index_type == IVF_PQ_IDENTIFIER,
- _ => false,
+ "dimension" | "distance.metric" => true,
+ "nlist" | "train.sample-ratio" => index_type != DISKANN_IDENTIFIER,
Review Comment:
[P2] Accept DiskANN training sampling in procedure options
The Rust documentation says `train.sample-ratio` applies to all vindex
types, and the resolver plus build path already consume it without excluding
DiskANN. Grouping it with `nlist` here makes `options =>
diskann.train.sample-ratio=0.5` fail as unknown, while the equivalent table
property is honored. Please allow this suffix for DiskANN and add a
procedure-option regression test.
##########
crates/paimon/src/vindex/reader.rs:
##########
@@ -30,6 +31,8 @@ use std::time::{Duration, Instant};
const DEFAULT_NPROBE: usize = 16;
const NPROBE_PARAMETER: &str = "ivf.nprobe";
+const L_SEARCH_PARAMETER: &str = "diskann.l-search";
Review Comment:
[P2] Use the canonical DiskANN search option name
Apache Paimon Java and the public option contract use `diskann.l_search`,
with an underscore. With that canonical key, this lookup misses and silently
falls back to automatic search width, so the requested recall/latency tuning is
ignored. Please use `diskann.l_search` as the canonical name; accepting
`diskann.l-search` as a compatibility alias is fine.
##########
crates/paimon/src/vindex/mod.rs:
##########
@@ -159,6 +192,34 @@ impl VindexVectorIndexOptions {
),
);
}
+ if index_type == IVF_RQ_IDENTIFIER {
+ native_options.insert(
+ "rq.bits".to_string(),
Review Comment:
[P2] Let the IVF-RQ capacity budget determine `rq.bits`
This unconditional default prevents `paimon-vindex-core` from inferring the
residual width from `max-bytes-per-vector`. The new validation and forwarding
logic accepts that capacity option only for DiskANN, so
`ivf-rq.max-bytes-per-vector` is rejected in procedure options and silently
ignored as a table property, even though Java 0.3 forwards it and core 0.3 uses
it to enforce the storage bound. Please forward the applicable capacity options
for IVF-RQ and insert `rq.bits` only when it is explicitly configured.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
