QuakeWang commented on code in PR #441:
URL: https://github.com/apache/paimon-rust/pull/441#discussion_r3517833077
##########
crates/paimon/src/spec/core_options.rs:
##########
@@ -176,16 +183,64 @@ pub struct CoreOptions<'a> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum TimeTravelSelector<'a> {
TimestampMillis(i64),
- /// Raw version string from `VERSION AS OF`. Resolved at scan time:
- /// tag name (if tag exists) → snapshot id (if parseable as i64) → error.
- Version(&'a str),
+ /// `scan.version` (SQL `VERSION AS OF`): ambiguous by design. Resolved at
+ /// scan time as tag name (if a tag exists) → snapshot id (if parseable) →
+ /// error. `option_name` is kept for error attribution.
+ Version {
+ value: &'a str,
+ option_name: &'static str,
+ },
+ /// `scan.snapshot-id`: an explicit snapshot id. Resolved strictly by
+ /// parsing `value` as an id — never falls back to a tag lookup.
+ SnapshotId {
+ value: &'a str,
+ option_name: &'static str,
+ },
+ /// `scan.tag-name`: an explicit tag name. Resolved strictly by tag lookup
—
+ /// never falls back to a snapshot id.
+ TagName {
+ value: &'a str,
+ option_name: &'static str,
+ },
}
impl<'a> CoreOptions<'a> {
pub fn new(options: &'a HashMap<String, String>) -> Self {
Self { options }
}
+ /// Reject scan options whose semantics the Rust core does not yet
implement.
+ ///
+ /// These are not malformed input — they are unimplemented scan modes — so
+ /// they surface as `Error::Unsupported` (mapped to `NotImplementedError`
at
+ /// the Python boundary). `scan.mode` is allowed only when absent or
+ /// `"default"`, since the core has no `scan.mode` consumer yet.
+ pub fn validate_scan_options(&self) -> crate::Result<()> {
+ for key in [
+ INCREMENTAL_BETWEEN_OPTION,
+ INCREMENTAL_BETWEEN_TIMESTAMP_OPTION,
+ INCREMENTAL_BETWEEN_SCAN_MODE_OPTION,
+ SCAN_WATERMARK_OPTION,
+ ] {
+ if self.options.contains_key(key) {
+ return Err(crate::Error::Unsupported {
+ message: format!("Scan option '{key}' is not supported by
the Rust reader yet"),
+ });
+ }
+ }
+ if let Some(mode) = self.options.get(SCAN_MODE_OPTION) {
Review Comment:
This rejects documented Paimon option combinations such as
`scan.mode=from-snapshot` with `scan.snapshot-id` / `scan.tag-name`, and
`scan.mode=from-timestamp` with `scan.timestamp-millis`.
Those modes are standard ways to express the selectors this PR now supports.
Could we allow the explicit modes when they match an implemented selector, and
only reject truly unsupported modes like `incremental` / `compacted-full`?
##########
crates/paimon/src/table/table_scan.rs:
##########
@@ -364,8 +364,10 @@ impl<'a> TableScan<'a> {
/// Plan the full scan: resolve snapshot (via options or latest), then
read manifests and build DataSplits.
///
/// Time travel is resolved from table options:
- /// - only one of `scan.version`, `scan.timestamp-millis` may be set
- /// - `scan.version` → tag name (if exists) → snapshot id (if parseable) →
error
+ /// - only one of `scan.version`, `scan.timestamp-millis`,
+ /// `scan.snapshot-id`, `scan.tag-name` may be set
+ /// - `scan.version` / `scan.snapshot-id` / `scan.tag-name` → tag name (if
Review Comment:
The comment is now stale: `scan.snapshot-id` and `scan.tag-name` are strict
selectors in the implementation. Only `scan.version` does tag-first then
snapshot-id fallback.
--
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]
