JunRuiLee commented on code in PR #441:
URL: https://github.com/apache/paimon-rust/pull/441#discussion_r3520945242
##########
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:
Fixed in 490f61b. `validate_scan_options` now accepts explicit
`scan.mode=from-snapshot` when `scan.snapshot-id` / `scan.tag-name` /
`scan.version` is present, and `scan.mode=from-timestamp` with
`scan.timestamp-millis` — Java's `CoreOptions.setDefaultValues()` writes these
modes next to their selectors, so configs from the Java toolchain carry both
keys. Two details beyond the suggestion: an explicit mode *without* its
selector fails as invalid input (mirroring Java `SchemaValidation`) rather than
silently reading latest, and truly unimplemented modes (`incremental`,
`compacted-full`, `latest`, ...) still raise `Unsupported` →
`NotImplementedError`. Covered by new tests in `core_options.rs` and
`test_read.py`.
##########
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:
Fixed in 490f61b — the `plan()` doc now describes each selector separately:
`scan.version` keeps the ambiguous tag → snapshot-id resolution, while
`scan.snapshot-id` and `scan.tag-name` are documented as strict. Also scoped
the `TimeTravelUtil` reference to `scan.version` only, since the strict
selectors don't follow it.
--
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]
