leaves12138 commented on code in PR #10455:
URL: https://github.com/apache/paimon/pull/10455#discussion_r4226418242
##########
paimon-python/pypaimon/write/native_blob_rows.py:
##########
@@ -42,15 +45,61 @@ class NativeBlobRows:
the core cannot reuse an unrelated Python stream.
"""
- def __init__(self, file_io):
+ def __init__(self, fallback=None):
self.readers = {}
- self.fallback = UriReaderFactory.from_file_io(file_io)
+ self.fallback = fallback
+
+ def _supports_uri(self, uri):
+ # Ordinary descriptors retain the core FileIO/HTTP path. A configured
+ # application factory owns other URIs and its errors must propagate.
+ return uri in self.readers or self.fallback is not None
Review Comment:
[P2] Preserve the fallback factory URI-ownership selector
When with_blob_uri_reader_factory(application_factory) sets fallback, this
returns true for every URI and never calls application_factory._supports_uri.
It therefore overrides the scoped-factory contract added in the companion Rust
PR. For example, an application factory that owns only custom:// URIs and
returns false for file:// gets its create method called for an ordinary
readable file BlobDescriptor; mixed native/custom writes then fail instead of
using native FileIO. I reproduced this for both row/Arrow inputs and
batch/stream writers (application selector calls = 0, create calls = 1),
whereas the direct Rust binding selects once and never calls create for that
same file. Selection exceptions are also bypassed. Keep synthetic URIs owned
locally; for other URIs delegate to the fallback selector when present,
preserving true as the default for legacy factories and propagating selection
errors. Add a regression with a scoped application factory plus an ordinary
file 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]