You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "shyjsarah (via GitHub)" <gi...@apache.org> on 2026/07/27 11:50:20 UTC
[PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
shyjsarah opened a new pull request, #623:
URL: https://github.com/apache/paimon-rust/pull/623
### Purpose
Linked issue: close #622
`SQLContext` applies session-scoped Paimon dynamic options to ordinary table scans, but the automatically registered `vector_search` function loaded its table directly from the raw catalog. This caused options such as `blob-as-descriptor` to be ignored when vector-search results were materialized.
### Brief change log
- pass the shared `DynamicOptions` instance when `SQLContext` registers `vector_search`
- merge a snapshot of the current options into the loaded table before creating the vector-search provider
- preserve the existing standalone `register_vector_search` and `VectorSearchFunction::new` APIs by using empty options there
- add a regression test proving that an option set after catalog registration reaches the vector-search table
### Tests
- `cargo fmt --all -- --check`
- `cargo check -p paimon-datafusion --all-features`
- `cargo clippy -p paimon-datafusion --all-features --lib --tests --no-deps -- -D warnings`
- `cargo test -p paimon-datafusion test_vector_search_applies_session_dynamic_options --lib`
- `cargo test -p paimon-datafusion --test read_tables vector_search_tests::test_vector_search_java_vindex_table`
The full `paimon-datafusion --lib` run completed 311 tests successfully; 7 existing warehouse-fixture tests failed because the local `PAIMON_TEST_WAREHOUSE` did not contain `partitioned_log_table` / `multi_partitioned_log_table`.
### API and Format
No public API or storage-format changes.
### Documentation
No documentation changes are required; this makes `vector_search` consistent with the documented session dynamic-option behavior.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #623:
URL: https://github.com/apache/paimon-rust/pull/623#discussion_r3666730954
##########
crates/integrations/datafusion/src/vector_search.rs:
##########
@@ -426,6 +461,21 @@ impl ExecutionPlan for VectorSearchExec {
}
}
+/// Vector search currently resolves candidates from the latest snapshot, so forwarding a
+/// time-travel selector would search one snapshot and materialize rows from another.
+fn vector_search_dynamic_options(dynamic_options: &DynamicOptions) -> HashMap<String, String> {
+ let mut options = dynamic_options.read().unwrap().clone();
+ for key in [
+ SCAN_VERSION_OPTION,
Review Comment:
Why need to remote these options? Can we just keep it as is.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
Posted by "shyjsarah (via GitHub)" <gi...@apache.org>.
shyjsarah commented on code in PR #623:
URL: https://github.com/apache/paimon-rust/pull/623#discussion_r3656960936
##########
crates/integrations/datafusion/src/sql_context.rs:
##########
@@ -198,7 +198,12 @@ impl SQLContext {
Some(session_state),
)),
);
- register_table_functions(&self.ctx, &catalog, default_db.unwrap_or("default"));
+ register_table_functions(
+ &self.ctx,
+ &catalog,
+ default_db.unwrap_or("default"),
+ self.dynamic_options.clone(),
Review Comment:
Good catch. The DE vector-search path reads the latest index snapshot, so forwarding a time-travel selector could indeed search the latest snapshot and materialize rows from an older one. I updated the patch to filter `scan.version`, `scan.timestamp-millis`, `scan.snapshot-id`, and `scan.tag-name` while still forwarding supported options such as `blob-as-descriptor`. The regression test now covers both behaviors.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
Posted by "shyjsarah (via GitHub)" <gi...@apache.org>.
shyjsarah commented on code in PR #623:
URL: https://github.com/apache/paimon-rust/pull/623#discussion_r3671325976
##########
crates/integrations/datafusion/src/vector_search.rs:
##########
@@ -426,6 +461,21 @@ impl ExecutionPlan for VectorSearchExec {
}
}
+/// Vector search currently resolves candidates from the latest snapshot, so forwarding a
+/// time-travel selector would search one snapshot and materialize rows from another.
+fn vector_search_dynamic_options(dynamic_options: &DynamicOptions) -> HashMap<String, String> {
+ let mut options = dynamic_options.read().unwrap().clone();
+ for key in [
+ SCAN_VERSION_OPTION,
Review Comment:
Good point. We can keep these options as-is. I updated vector search to honor the resolved time-travel snapshot instead of always using the latest snapshot, and added regression tests covering `scan.version` together with `blob-as-descriptor`.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
Posted by "XiaoHongbo-Hope (via GitHub)" <gi...@apache.org>.
XiaoHongbo-Hope commented on code in PR #623:
URL: https://github.com/apache/paimon-rust/pull/623#discussion_r3656914533
##########
crates/integrations/datafusion/src/sql_context.rs:
##########
@@ -198,7 +198,12 @@ impl SQLContext {
Some(session_state),
)),
);
- register_table_functions(&self.ctx, &catalog, default_db.unwrap_or("default"));
+ register_table_functions(
+ &self.ctx,
+ &catalog,
+ default_db.unwrap_or("default"),
+ self.dynamic_options.clone(),
Review Comment:
This also forwards time-travel options such as scan.version, but vector search still uses the latest snapshot.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(datafusion): apply dynamic options to vector search [paimon-rust]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #623:
URL: https://github.com/apache/paimon-rust/pull/623
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org