From b961ff645193cae389ba71a525f41e31dc9f9a6b Mon Sep 17 00:00:00 2001 From: jackylee-ch Date: Mon, 21 Sep 2026 10:00:55 +0800 Subject: [PATCH] fix(lumina): store the canonical native metric name in index metadata lumina.distance.metric accepts the enum spellings L2/COSINE/INNER_PRODUCT as well as the native ones, but the raw string was copied into the native option map that becomes the committed index metadata. The read side is exact-match only, so LuminaIndexMeta::metric() then failed with "Unknown lumina metric name: L2" on every query against such an index. Persist metric.lumina_name() instead, mirroring Java LuminaVectorIndexOptions (apache/paimon#8676). strip_lumina_options needs the same treatment. Java crosses its native boundary with a typed MetricType, so the string is inert there, but Rust hands the whole option map to the native library and ensure_loaded overlays the stripped table options on top of the index metadata -- the configured spelling would otherwise come straight back. An unrecognized value is left untouched so its error surface does not move, and the readers stay exact-match: from_lumina_name is the wire decoder and widening it would accept metadata Java rejects. --- crates/paimon/src/lumina/mod.rs | 84 ++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/crates/paimon/src/lumina/mod.rs b/crates/paimon/src/lumina/mod.rs index 56796e2c6..2302fd9f2 100644 --- a/crates/paimon/src/lumina/mod.rs +++ b/crates/paimon/src/lumina/mod.rs @@ -135,7 +135,8 @@ impl LuminaVectorIndexOptions { .cloned() .unwrap_or_else(|| "diskann".to_string()); - let lumina_options = build_lumina_options(paimon_options, dimension)?; + let mut lumina_options = build_lumina_options(paimon_options, dimension)?; + canonicalize_metric(&mut lumina_options); Ok(Self { dimension, @@ -150,6 +151,26 @@ impl LuminaVectorIndexOptions { } } +/// Rewrite `distance.metric` to the spelling `from_lumina_name` accepts. +/// +/// The option parser also takes the enum names, via `from_string` above, but +/// `LuminaIndexMeta::metric` is exact-match, so the configured spelling must not +/// reach the committed metadata. An unrecognized value is left alone, keeping its +/// existing error surface. +fn canonicalize_metric(native_options: &mut HashMap) { + let Some(configured) = native_options.get(KEY_DISTANCE_METRIC) else { + return; + }; + let parsed = LuminaVectorMetric::from_lumina_name(configured) + .or_else(|_| LuminaVectorMetric::from_string(configured)); + if let Ok(metric) = parsed { + native_options.insert( + KEY_DISTANCE_METRIC.to_string(), + metric.lumina_name().to_string(), + ); + } +} + fn validate_encoding_metric(encoding: &str, metric: LuminaVectorMetric) -> crate::Result<()> { if encoding.eq_ignore_ascii_case("pq") && metric == LuminaVectorMetric::Cosine { return Err(crate::Error::DataInvalid { @@ -226,6 +247,9 @@ pub fn strip_lumina_options(paimon_options: &HashMap) -> HashMap result.insert(native_key.to_string(), value.to_string()); } } + // `LuminaVectorReader::ensure_loaded` overlays this map on top of the index + // metadata, so a table option spelled `L2` would put the enum name back. + canonicalize_metric(&mut result); result } @@ -494,4 +518,62 @@ mod tests { assert_eq!(lumina_opts.get("encoding.pq.m").unwrap(), "64"); assert_eq!(lumina_opts.get("search.parallel_number").unwrap(), "5"); } + + #[test] + fn test_enum_form_metric_is_canonicalized_in_native_options() { + // The native-spelling rows pin that canonicalization is a no-op for them. + let cases = [ + ("L2", "l2"), + ("COSINE", "cosine"), + ("INNER_PRODUCT", "inner_product"), + ("l2", "l2"), + ("cosine", "cosine"), + ("inner_product", "inner_product"), + ]; + for (configured, native) in cases { + let paimon_options = HashMap::from([ + ("lumina.index.dimension".to_string(), "4".to_string()), + // `cosine` is rejected with the default `pq` encoding. + ("lumina.encoding.type".to_string(), "rawf32".to_string()), + ("lumina.distance.metric".to_string(), configured.to_string()), + ]); + let options = LuminaVectorIndexOptions::new(&paimon_options).unwrap(); + let native_options = options.to_lumina_options(); + assert_eq!( + native_options.get(KEY_DISTANCE_METRIC).map(String::as_str), + Some(native), + "configured: {configured}" + ); + let metric = LuminaIndexMeta::new(native_options) + .metric() + .unwrap_or_else(|e| panic!("configured {configured}: {e}")); + assert_eq!(metric.lumina_name(), native, "configured: {configured}"); + } + } + + #[test] + fn test_stripped_search_options_use_the_canonical_metric_name() { + let stripped = strip_lumina_options(&HashMap::from([( + "lumina.distance.metric".to_string(), + "L2".to_string(), + )])); + assert_eq!( + stripped.get(KEY_DISTANCE_METRIC).map(String::as_str), + Some("l2") + ); + } + + /// An unknown metric keeps its existing error surface rather than being + /// defaulted here. Passes without the fix too. + #[test] + fn test_stripped_search_options_pass_an_unknown_metric_through() { + let stripped = strip_lumina_options(&HashMap::from([( + "lumina.distance.metric".to_string(), + "hamming".to_string(), + )])); + assert_eq!( + stripped.get(KEY_DISTANCE_METRIC).map(String::as_str), + Some("hamming") + ); + } }