OIiveirra commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4366381467
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java:
##########
@@ -662,6 +663,11 @@ private Plan
bindTVFTableSink(MatchingContext<UnboundTVFTableSink<Plan>> ctx) {
"INSERT INTO TVF only supports local/s3/hdfs, but got: " +
tvfName);
}
+ // INSERT/EXPLAIN INSERT bind a sink without constructing the LOCAL
read TVF.
+ if (tvfName.equals("local")) {
Review Comment:
Fixed in d1ec854a5d1a1b6751583f0f900b805b9a702fee, and mirrored in the
existing master and 4.1 PRs where the same write path exists.
Shared LOCAL admission now checks the effective built-in LocalFileJniWriter
before required-property validation and at final TVF sink binding. It
recognizes the loader's dot/slash class names and JNI C-string NUL truncation.
The startup opt-in and other JNI/native remote writers retain their existing
behavior.
All 39 selected FE tests passed, including disabled INSERT/EXPLAIN under
S3/HDFS names, direct binding with complete S3 properties, opt-in and
native-remote controls, and the actual command entry with ADMIN=false /
LOAD=true. No executor interaction occurs in the disabled-mode execution test.
##########
be/src/format_v2/table/lance_reader.cpp:
##########
@@ -35,11 +35,50 @@
#include "format_v2/lance/lance_reader_helper.h"
#include "format_v2/lance/lance_runtime_filter_helper.h"
#include "format_v2/lance/lance_session_manager.h"
+#include "io/fs/local_file_system.h"
#include "runtime/file_scan_profile.h"
#include "runtime/runtime_state.h"
namespace doris::format::lance {
+namespace {
+Status resolve_dataset_uri(const TFileRangeDesc& range, const
TFileScanRangeParams& scan_params,
+ std::string* uri) {
+ const auto& requested_uri =
range.table_format_params.lance_params.dataset_uri;
+ // Modern plans tag each range; older plans put the type in the scan
parameters.
+ // Untagged catalog scans retain their original storage-provider selection.
+ const bool is_local = range.__isset.file_type
+ ? range.file_type == TFileType::FILE_LOCAL
+ : scan_params.__isset.file_type &&
+ scan_params.file_type ==
TFileType::FILE_LOCAL;
+ // LOCAL TVF uses version zero for both schema discovery and whole-dataset
scans.
+ // Catalog scans pin a positive snapshot version, including file://
catalogs whose
+ // operator-configured warehouse is independent of the LOCAL TVF secure
directory.
+ if (is_local && range.table_format_params.lance_params.version == 0) {
+ // LOCAL Lance bypasses glob because it opens a dataset directory.
Apply the same
+ // canonical secure-root check before either schema discovery or a
cached dataset open.
+ std::string local_input = requested_uri;
+ const auto colon = requested_uri.find(':');
+ const auto slash = requested_uri.find('/');
+ const bool bare_path =
+ colon == std::string::npos || (slash != std::string::npos &&
slash < colon);
+ if (bare_path && !requested_uri.empty() && requested_uri.find('\0') ==
std::string::npos &&
+ !config::user_files_secure_path.empty()) {
+ // Lance's PathAbs fallback resolves bare dot segments before
following symlinks.
+ // Anchor relative TVF paths to the canonical secure root, not the
process cwd.
+ std::string secure_root;
+ RETURN_IF_ERROR(io::global_local_filesystem()->canonicalize(
+ config::user_files_secure_path, &secure_root));
+ local_input = (io::Path(secure_root) /
requested_uri).lexically_normal().string();
Review Comment:
Fixed in d1ec854a5d1a1b6751583f0f900b805b9a702fee. Only LOCAL/version-zero
bare paths expand SDK-supported `~` and `~/` before lexical normalization and
canonical secure-root containment. HOME is used first with a thread-safe passwd
fallback, matching the SDK; named-user spellings and file-URI literal tildes
are preserved.
All 15 selected BE tests passed. Distinct real Lance fixtures verify schema,
canonical dataset key and actual rows for the home target versus the
literal-tilde target, plus an expanded target outside the secure root. The
tests use a unique fixture under the existing home and never override HOME.
Remote ranges and fixed-version catalogs retain their routing.
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -455,28 +455,158 @@ const std::shared_ptr<LocalFileSystem>&
global_local_filesystem() {
Status LocalFileSystem::canonicalize_local_file(const std::string& dir,
const std::string& file_path,
std::string* full_path) {
- const std::string absolute_path = dir + "/" + file_path;
+ // An empty root would turn a relative request into an unrestricted
absolute path.
+ if (dir.empty()) {
+ return Status::InvalidArgument("user_files_secure_path must not be
empty");
+ }
+ // Reject embedded NULs before passing paths to filesystem APIs that use C
strings.
+ if (file_path.empty() || file_path.find('\0') != std::string::npos) {
+ return Status::InvalidArgument("local file path must not be empty or
contain NUL");
+ }
+ // Match URL preprocessing only for actual file URIs; bare paths keep
literal whitespace.
+ const auto is_c0_or_space = [](unsigned char c) { return c <= 0x20; };
+ size_t begin = 0;
+ size_t end = file_path.size();
+ while (begin < end && is_c0_or_space(file_path[begin])) {
+ ++begin;
+ }
+ while (end > begin && is_c0_or_space(file_path[end - 1])) {
+ --end;
+ }
+ std::string uri_input;
+ for (size_t i = begin; i < end; ++i) {
+ if (file_path[i] != '\t' && file_path[i] != '\r' && file_path[i] !=
'\n') {
+ uri_input += file_path[i];
+ }
+ }
+ const bool is_file_uri = uri_input.size() >= 5 &&
+ (uri_input[0] == 'f' || uri_input[0] == 'F') &&
+ (uri_input[1] == 'i' || uri_input[1] == 'I') &&
+ (uri_input[2] == 'l' || uri_input[2] == 'L') &&
+ (uri_input[3] == 'e' || uri_input[3] == 'E') &&
uri_input[4] == ':';
+ std::string local_input = is_file_uri ? std::move(uri_input) : file_path;
+ if (is_file_uri) {
+ local_input.replace(0, 4, "file");
+ // File URLs treat raw backslashes as separators, including before the
authority.
+ // Do this before conversion; percent-encoded backslashes must remain
literal.
+ for (size_t i = 5; i < local_input.size(); ++i) {
+ if (local_input[i] == '\\') {
+ local_input[i] = '/';
+ }
+ }
+ }
+ Path local_path;
+ // Convert file: URIs to filesystem paths and reject remote schemes before
any reader opens them.
+ RETURN_IF_ERROR(convert_to_abs_path(local_input, local_path));
Review Comment:
Fixed in d1ec854a5d1a1b6751583f0f900b805b9a702fee. A raw drive in file-URL
authority position is preserved as the first path component, so
`file://C:/data.lance` resolves to `/C:/data.lance` before containment.
File-URL raw-drive parent handling follows the pinned SDK; bare Unix paths keep
their existing semantics. The legacy convert_to_abs_path API is unchanged.
The pure parser test covers 19 forms without creating root-level
directories; 18 file-URL expectations were independently checked against pinned
url 2.5.8 on Unix. Real nested-drive fixtures additionally assert schema and
scanned row identity. All 15 selected filesystem/Lance tests passed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
