OIiveirra commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4367402592
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/LocalTableValuedFunction.java:
##########
@@ -135,6 +143,38 @@ private void getFileListFromBackend() throws
AnalysisException {
}
}
+ /** Apply LOCAL admission to sinks that select the built-in local JNI
writer as well. */
+ public static void checkSinkEnabled(String tvfName, Map<String, String>
properties) {
+ String writerClass = properties.getOrDefault("writer_class", "");
+ // JNI consumes a C string, and ScannerLoader accepts slash-separated
class names.
+ int nul = writerClass.indexOf('\0');
+ if (nul >= 0) {
+ writerClass = writerClass.substring(0, nul);
+ }
+ if ("local".equalsIgnoreCase(tvfName)
+ ||
"org.apache.doris.writer.LocalFileJniWriter".equals(writerClass.replace('/',
'.'))) {
Review Comment:
The remaining consumer-identity variants are addressed in
d1b3962540b16751ca205419f022d8cbfcb082f7. One normalized admission module
distinguishes native output paths/properties from Java cleanup. It covers
default authority aliases, explicit resources, VDFS/runtime delegation, raw
convert_path ordering before Hadoop parsing, non-prefix NameNode ambiguity, and
release relative/trimmed Java cleanup paths. The actual cleanup path is checked
before filesystem creation.
The current revision passed 86 FE and 22 BE tests. Real bundled Hadoop and
storage-normalizer oracles validate path/property identity; cleanup tests
assert zero filesystem/executor interactions and perform no real deletion.
Explicit remote providers and valid native relative-path controls remain
covered.
##########
be/src/format_v2/table/lance_reader.cpp:
##########
@@ -35,11 +39,79 @@
#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 {
+std::string expand_home_path(const std::string& path) {
+ if (path != "~" && !path.starts_with("~/")) {
+ return path;
+ }
+ // Match the SDK's std::env::home_dir: HOME first, then the current user's
passwd entry.
+ std::string home;
+ if (const char* value = std::getenv("HOME"); value != nullptr && value[0]
!= '\0') {
+ home = value;
+ } else {
+ std::vector<char> buffer(1024);
+ struct passwd entry {};
+ struct passwd* result = nullptr;
+ int error;
+ while ((error = getpwuid_r(getuid(), &entry, buffer.data(),
buffer.size(), &result)) ==
+ ERANGE) {
+ buffer.resize(buffer.size() * 2);
+ }
+ if (error == 0 && result != nullptr && entry.pw_dir != nullptr) {
+ home = entry.pw_dir;
+ }
+ }
+ if (home.empty()) {
+ return path;
+ }
+ return path == "~" ? home : (io::Path(home) / path.substr(2)).string();
+}
+
+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) /
expand_home_path(requested_uri))
+ .lexically_normal()
+ .string();
+ }
+ return io::global_local_filesystem()->canonicalize_local_file(
Review Comment:
The manifest-reference gap remaining after the earlier descendant walk is
addressed in d1b3962540b16751ca205419f022d8cbfcb082f7. Physical-tree checks
remain, and SDK metadata now validates every registered manifest base before
TVF schema/delegated reads. LOCAL external bases must remain inside the secure
root; remote TVFs reject local bases. Validation includes
dataset-root/direct-directory bases and cached/session reuse, without changing
unmarked fixed-version catalog policy.
Official SDK-generated shallow clones reject out-of-root source bases and
return actual rows 1–4 when wholly in root. All 86 FE and 22 BE tests passed;
isolated S3 negative/positive tests execute without skips. Matching Lance
header/archive requirements and mixed-version limits are documented in the PR.
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java:
##########
@@ -63,6 +76,11 @@ public S3TableValuedFunction(Map<String, String> properties,
List<TBrokerFileSta
} catch (UserException e) {
throw new RuntimeException(e);
}
+ // Validate the final URI, including when BrokerProperties passes it
through unchanged.
+ // A local dataset must not bypass the LOCAL switch, privileges or BE
secure directory.
+ if (isLanceFormat() && !isRemoteLanceUri(filePath)) {
Review Comment:
Fixed in d1b3962540b16751ca205419f022d8cbfcb082f7. Java TVF metadata loading
inspects manifest bases immediately after opening the primary snapshot and
before BASIC metadata use. BE TVFs use the new metadata-only Lance C API to
inspect SDK-decoded primary and every registered base before
schema/scanner/index/row-id work: remote TVFs reject local bases; LOCAL bases
must canonicalize inside the secure root and their reachable trees are checked.
Both dataset-root and direct-data-directory bases are covered. Policy is part
of the dataset key and is enforced on cached/reused handles; unmarked
fixed-version catalogs retain their configured policy.
All 86 selected FE and 22 selected BE tests passed with zero
failures/errors/skips. Official SDK-generated fixtures prove outside-root
rejection and actual in-root rows 1–4. Isolated S3 tests execute Java
rejection, BE remote/local-base rejection and actual remote rows 1–4. The C
ABI/SDK tests and matching header/archive checks passed. Complete enforcement
requires upgraded serving FE/BE versions; the PR describes the optional TVF
marker and mixed-version boundary.
--
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]
