JingsongLi commented on code in PR #10701:
URL: https://github.com/apache/paimon/pull/10701#discussion_r4332918357
##########
paimon-python/pypaimon/multimodal/lerobot/reader.py:
##########
@@ -30,15 +30,34 @@ class _PaimonTableFrameReader:
def __init__(self, frames_table, *, columns):
self._table, self.snapshot_id, splits = _indexed_read_table(
frames_table, columns)
+ options = self._table.options.options
+ if not any(options.contains(option) for option in (
+ CoreOptions.READ_NATIVE_ENABLED,
CoreOptions.SCAN_NATIVE_PLAN_ENABLED)):
+ from pypaimon.execution.native.runtime import
native_runtime_available
+ if native_runtime_available():
+ self._table = self._table.copy_without_time_travel({
+ CoreOptions.READ_NATIVE_ENABLED.key(): "true"})
snapshot = self._table.snapshot_manager().get_snapshot_by_id(
self.snapshot_id)
+ self._table = self._table._copy_with_snapshot(snapshot)
self.num_rows = snapshot.next_row_id
self.file_io = self._table.file_io
self._locator = _FrameLocator(self._table, snapshot, splits)
def read_indices(self, indices, columns):
- splits, needs_filter = self._locator.locate(indices)
builder = self._table.new_read_builder().with_projection(columns)
+ if (self._table.options.native_plan_enabled()
+ or self._table.options.native_read_enabled()):
+ from pypaimon.execution.native.runtime import
native_runtime_available
+ if native_runtime_available():
+ # Keep lookup predicates in the native plan and residual read.
+ builder.with_filter(_index_predicate(self._table, indices))
+ scan = builder.new_scan()
+ if scan._native_plan_supported():
+ plan = scan.plan()
+ return builder.new_read().to_arrow(plan.splits())
Review Comment:
[P2] Reuse the cached frame locator when native planning fails
`_native_plan_supported()` returning true does not guarantee that
`scan.plan()` used native planning: `TableScan.plan()` catches native planning
failures and falls back to a new Python scanner. The unconditional return here
therefore bypasses `self._locator` and discards the cached frame-index lookup
path for every batch.
A concrete trigger is an existing `viewfs://` frames table with the Rust
extension installed and neither native option explicitly set. Python supports
that storage scheme, the capability check does not reject it, and Rust rejects
it during table construction. The existing
`test_plan_falls_back_when_native_plan_raises` also models this
unsupported-scheme failure. Each subsequent training batch then reloads
manifests and reopens the BTree instead of reusing the fixed-snapshot locator.
Local reproduction on `d5a7141d9a`:
1. Create an indexed data-evolution frames table and a reader with
`scan.native-plan.enabled=true`, `read.native.enabled=false` to isolate
planning.
2. Make `pypaimon.execution.native.read.plan.native_plan` raise
`RuntimeError("native planner unavailable")`.
3. Read three successive batches and count
`DataEvolutionGlobalIndexScanner.create`.
Observed: **3 scanner creations and 0 calls to the cached locator**. Taking
the existing locator fallback creates **1 scanner for the same 3 batches**.
Returned rows are correct in both cases. This reproduces the runtime-failure
path locally; remote `viewfs` reads were not exercised end to end.
Please distinguish actual native planning success from its Python fallback,
and continue through `_locator.locate()` on failure while preserving residual
filtering. Extending `test_native_plan_failure_keeps_predicate` to multiple
batches with a scanner-reuse assertion would cover this case; the current reuse
test only covers a false capability check.
--
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]
