OIiveirra commented on code in PR #69526:
URL: https://github.com/apache/doris/pull/69526#discussion_r4366828133
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTVFCommand.java:
##########
@@ -185,6 +188,10 @@ private void deleteExistingFilesInFE(String tvfName,
Map<String, String> props)
fsCopyProps.remove("compress_type");
StorageAdapter storageAdapter = StorageAdapter.of(fsCopyProps);
+ if (storageAdapter.getType() == StorageTypeId.HDFS) {
Review Comment:
Fixed in 071d4b6ec7c8c9a561fe752f7e42390bf38d1aa7. FE cleanup now checks the
actual selected LOCAL SPI adapter before creating any filesystem, regardless of
the nominal sink name. The new fs.local.support test uses a mocked
FileSystemFactory and verifies rejection before any factory interaction; it
performs no real deletion. Startup opt-in retains the existing implementation.
All 30 selected FE tests and targeted Checkstyle passed on the final source.
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/LocalTableValuedFunction.java:
##########
@@ -137,6 +140,129 @@ private void getFileListFromBackend() throws
AnalysisException {
}
}
+ /** Apply LOCAL admission to the effective sink before planning or
filesystem side effects. */
+ public static void checkSinkEnabled(String tvfName, Map<String, String>
properties) {
+ if ("local".equalsIgnoreCase(tvfName) ||
isLocalWriter(properties.getOrDefault("writer_class", ""))) {
+ checkEnabled();
+ }
+ if ("hdfs".equalsIgnoreCase(tvfName)) {
+ if (!properties.containsKey("writer_class")) {
+ checkHdfsSinkEnabled(properties);
+ }
+ if
(Boolean.parseBoolean(properties.getOrDefault("delete_existing_files",
"false"))) {
+
checkHadoopFileSystemEnabled(properties.getOrDefault("file_path", ""),
properties);
+ }
+ }
+ }
+
+ private static boolean isLocalWriter(String writer) {
+ // BE splits the complete identifier first, then passes each half
through JNI C strings.
+ int separator = writer.indexOf(':');
+ return separator > 0 &&
"java-writer".equals(nativeStringPrefix(writer.substring(0, separator)))
+ &&
"local-file".equals(nativeStringPrefix(writer.substring(separator + 1)));
+ }
+
+ /** Check the NameNode and provider used by the native FILE_HDFS writer. */
+ public static void checkHdfsSinkEnabled(Map<String, String> properties) {
+ String nameNode =
nativeStringPrefix(properties.getOrDefault("fs.defaultFS", "")).trim();
+ if ("default".equals(nameNode)) {
+ // libhdfs delegates this selector to the BE's defaults, which may
select local storage.
+ checkEnabled();
+ }
+ // libhdfs prefixes bare NameNodes with hdfs:// rather than using the
output path's scheme.
+ checkHadoopSchemeEnabled(nameNode.contains("://") ?
uriScheme(nameNode) : "hdfs", properties);
+ }
+
+ /** Check Hadoop's path/default-filesystem selection before FE-side
cleanup. */
+ public static void checkHadoopFileSystemEnabled(String path, Map<String,
String> properties) {
+ String scheme = uriScheme(path);
+ if (!scheme.isEmpty()) {
+ checkHadoopSchemeEnabled(scheme, properties);
+ return;
+ }
+ String defaultFs = hadoopSelectorProperty("fs.defaultFS", properties);
+ String legacyDefaultFs = hadoopSelectorProperty("fs.default.name",
properties);
+ // Configuration.set synchronizes aliases; either value can win when
the map is applied.
+ // Do not assume that the canonical key overrides a local legacy
selector.
+ if (defaultFs != null) {
+ checkHadoopDefaultEnabled(defaultFs, properties);
+ }
+ if (legacyDefaultFs != null) {
+ checkHadoopDefaultEnabled(legacyDefaultFs, properties);
+ }
+ if (defaultFs == null && legacyDefaultFs == null) {
+ checkHadoopSchemeEnabled("file", properties);
+ }
+ }
+
+ private static void checkHadoopDefaultEnabled(String value, Map<String,
String> properties) {
+ value = hadoopSelectorValue(value).trim();
+ // Hadoop retains the old "local" default-FS spelling.
+ String scheme = "local".equals(value) ? "file" : uriScheme(value);
+ checkHadoopSchemeEnabled(scheme.isEmpty() ? "hdfs" : scheme,
properties);
+ }
+
+ private static void checkHadoopSchemeEnabled(String scheme, Map<String,
String> properties) {
+ if ("file".equalsIgnoreCase(scheme)) {
+ checkEnabled();
+ }
+ String implementationKey = "fs." + scheme + ".impl";
Review Comment:
The delegated ViewFS variant is fixed in
071d4b6ec7c8c9a561fe752f7e42390bf38d1aa7. Native admission uses the actual
generated file's parent; FE cleanup checks its actual parent directory. The raw
and normalized checks follow the selected mount table and path-component link,
including trailing-source slashes, authority/default table selection, fallback
and multiple targets. Nested targets are checked before filesystem creation.
Unused local mounts do not block an unambiguous remote route.
Runtime-overloaded, regex, cyclic, inherited or ambiguous nested delegation
requires the existing LOCAL startup opt-in instead of assuming the inline
configuration proves remote storage. Enabled requests retain their existing
Hadoop behavior.
All 30 selected FE tests passed with fresh XML and unchanged sources,
including INSERT/EXPLAIN, direct binding, resource-derived mounts, off/on and
remote controls, and a global-LOAD command entry with cleanup enabled.
Factory/executor mocks verify no side effect before rejection. No real
directory deletion was executed; targeted Checkstyle 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]
