From 912e7ba08d45d13026dcb7ff8aa47aa001727431 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 12:56:22 +0800 Subject: [PATCH 01/13] [feat](connector) give a connector plugin its own conf file Adding a deployment-level setting for one connector meant editing two files in the engine: an @ConfField in fe-common's Config, and a line in fe-core's DefaultConnectorContext.buildEnvironment that forwards it. A plugin cannot read Config itself -- it loads child-first, so its own bundled copy shadows the engine's and every field reads back as a code default -- so the engine had to carry each key by name, and fe-core ended up knowing the key names of connectors it is otherwise entirely agnostic about. Give a connector a configuration file of its own instead. The engine reads /.conf, where is the plugin's ConnectorProvider.name(), and serves the parsed map back through ConnectorContext.getConnectorConfig(). The file is parsed generically, so no key name of any connector reaches fe-core, and a new connector needs no engine change at all. ConnectorConf.get layers that map over getEnvironment(): plugin conf first, then the fe.conf key the setting used to live under, then a default. That is what lets the settings that already ship move to the new channel without their @ConfFields going away, so an existing deployment keeps working after an upgrade with nothing to edit. A setting introduced from now on passes null for the legacy key and has no fe.conf half. Blank is "not set" at every step. An operator who writes 'key=' means they have not configured it, and reading it as a set empty string would let one stray line mask the fe.conf value actually in effect -- with nothing in either file to show which won. ConnectorConfFile still keeps blank-valued keys in the map it returns, so the map reflects the file as written and the decision stays in one place. The engine side of the split is deliberate: the file is located, parsed and defaulted once here rather than once per plugin, and getConnectorConfig() is a narrower thing to hand a plugin than the plugin directory path would be. This adds a method to ConnectorContext, so the recorded plugin API surface baseline is refreshed in the same commit. connector.plugin.api.version stays at 1.0: the method is a default, nothing outside fe-connector-spi implements ConnectorContext, and the only decorators of it extend the parent-first ForwardingConnectorContext -- which is loaded from the FE's own classpath, so they inherit the new forward without being rebuilt. A connector plugin built before this change therefore loads and behaves exactly as it did; it simply never reads the new map. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../doris/connector/spi/ConnectorConf.java | 77 ++++++++++ .../connector/spi/ConnectorConfFile.java | 95 ++++++++++++ .../doris/connector/spi/ConnectorContext.java | 27 ++++ .../connector/spi/ConnectorProvider.java | 15 ++ .../spi/ForwardingConnectorContext.java | 5 + .../connector/spi/ConnectorConfFileTest.java | 112 ++++++++++++++ .../connector/spi/ConnectorConfTest.java | 140 ++++++++++++++++++ .../resources/connector-plugin-surface.txt | 1 + 8 files changed, 472 insertions(+) create mode 100644 fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConf.java create mode 100644 fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConfFile.java create mode 100644 fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfFileTest.java create mode 100644 fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfTest.java diff --git a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConf.java b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConf.java new file mode 100644 index 00000000000000..4533f98ade3e29 --- /dev/null +++ b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConf.java @@ -0,0 +1,77 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.spi; + +/** + * Reads one deployment-level setting for a connector, from the plugin's own {@code .conf} first + * and from fe.conf second. + * + *

Two channels exist because the plugin conf file is the newer one. Settings that predate it are + * declared as {@code @ConfField}s in fe.conf and forwarded through + * {@link ConnectorContext#getEnvironment()}; those {@code @ConfField}s are kept so that an existing + * deployment keeps working untouched after an upgrade. The precedence below is what lets both be true + * at once, and it lives here — in one place — so that the connectors reading these settings cannot + * drift apart on it. + * + *

A setting introduced from now on has no fe.conf half: pass {@code null} for + * {@code legacyEnvKey} and the engine needs no change to carry it. + */ +public final class ConnectorConf { + + private ConnectorConf() { + } + + /** + * Resolves {@code key} as: the plugin's {@code .conf}, then fe.conf, then {@code defaultValue}. + * + *

A value that is absent or blank is "not set" at each step. Blank counts as unset + * because an operator who writes {@code key=} in a conf file means "I have not configured this", + * not "configure this to the empty string" — and reading it the other way would let an empty line + * mask the fe.conf value that is actually in effect. + * + *

Values are returned as written (the conf file's were trimmed when the file was parsed; fe.conf's + * are passed through untouched, so a connector sees exactly the string it saw before this channel + * existed). + * + * @param context the connector's context + * @param key the key in {@code .conf}. Not prefixed with the connector's name — + * the file name already namespaces it + * @param legacyEnvKey the same setting's fe.conf name as forwarded through + * {@link ConnectorContext#getEnvironment()}, or null for a setting that never + * had one + * @param defaultValue returned when neither channel has the setting; may be null + */ + public static String get(ConnectorContext context, String key, String legacyEnvKey, + String defaultValue) { + String fromConf = context.getConnectorConfig().get(key); + if (isSet(fromConf)) { + return fromConf; + } + if (legacyEnvKey != null) { + String fromEnv = context.getEnvironment().get(legacyEnvKey); + if (isSet(fromEnv)) { + return fromEnv; + } + } + return defaultValue; + } + + private static boolean isSet(String value) { + return value != null && !value.trim().isEmpty(); + } +} diff --git a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConfFile.java b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConfFile.java new file mode 100644 index 00000000000000..9df36e7b44d3ec --- /dev/null +++ b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorConfFile.java @@ -0,0 +1,95 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.spi; + +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Properties; + +/** + * Reads a connector plugin's own configuration file, {@code /.conf}. + * + *

Engine side only. A connector never calls this: the engine loads the file once at plugin + * load and serves the parsed map through {@link ConnectorContext#getConnectorConfig()}, which is what + * connectors read (via {@link ConnectorConf}). Keeping the parsing here means one implementation of + * "find it, parse it, decide what an absent file means" instead of one per plugin. + * + *

No path validation is needed or wanted here. The file name is {@code name + ".conf"} where + * {@code name} is a plugin name that {@code PluginNames.validate} has already confined to + * {@code [a-zA-Z0-9._-]}. No separator can appear in it, so no traversal is expressible. A redundant + * check added later would only suggest the constraint lives here, when it lives there. + */ +public final class ConnectorConfFile { + + /** The suffix that makes a plugin directory entry a configuration file. */ + public static final String SUFFIX = ".conf"; + + private ConnectorConfFile() { + } + + /** The configuration file name for a plugin called {@code name}. For logs and error messages. */ + public static String fileName(String name) { + return name + SUFFIX; + } + + /** + * Parses {@code /.conf} in {@link Properties} text format, the same shape as fe.conf. + * + *

An absent file is not an error and yields an empty map: a connector whose settings all + * have defaults (or a fe.conf fallback) never needs the file, and requiring one would mean every + * deployment carries a file full of commented-out lines. + * + *

Values are trimmed. A key present with an empty value is kept rather than dropped, so the + * map reflects the file as written; {@link ConnectorConf#get} is where "written but blank" is decided + * to mean "not set". + * + * @return an immutable map, never null + * @throws IOException if the file exists but cannot be read or parsed + */ + public static Map load(Path pluginDir, String name) throws IOException { + Objects.requireNonNull(pluginDir, "pluginDir"); + Objects.requireNonNull(name, "name"); + Path file = pluginDir.resolve(fileName(name)); + if (!Files.isRegularFile(file)) { + return Collections.emptyMap(); + } + Properties properties = new Properties(); + try (InputStream in = Files.newInputStream(file); + Reader reader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + properties.load(reader); + } catch (IllegalArgumentException e) { + // Properties.load throws this (unchecked) on a malformed \\uXXXX escape. Rethrown as + // IOException so the caller's single "the file is unusable" branch covers every way it can be. + throw new IOException("malformed content in " + file, e); + } + Map parsed = new LinkedHashMap<>(); + for (String key : properties.stringPropertyNames()) { + parsed.put(key, properties.getProperty(key).trim()); + } + return Collections.unmodifiableMap(parsed); + } +} diff --git a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorContext.java b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorContext.java index eb20d9a44e6d5a..590801e10aee38 100644 --- a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorContext.java +++ b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorContext.java @@ -55,6 +55,33 @@ default Map getEnvironment() { return Collections.emptyMap(); } + /** + * The contents of {@code .conf} in this connector's own plugin directory, keys and values + * verbatim, immutable. {@code } is this connector's {@link ConnectorProvider#name()}. + * + *

This is a connector's deployment-level configuration channel: one per FE process, + * maintained by an administrator in the plugin directory, and not settable by a user in + * {@code CREATE CATALOG}. A value that varies per catalog belongs in the property map handed to + * {@code ConnectorProvider.create}; a value that varies per query belongs in + * {@code ConnectorSession.getSessionProperties()}. + * + *

Unlike {@link #getEnvironment()}, adding a key here costs the engine nothing: the file is named + * after the plugin and parsed generically, so no key name of yours ever appears in {@code fe-core}. + * Read it through {@link ConnectorConf#get}, which layers this map over {@code getEnvironment()} for + * keys that predate this channel. + * + *

Never null. Returns an empty map when: the file does not exist; the file could not be read (the + * engine has already logged an ERROR); or the connector was not loaded from a plugin directory at all + * (a classpath built-in, or a provider registered by a test). + * + *

Engine side: {@code ConnectorPluginManager.loadPlugins} reads the file once, right after the + * provider is admitted, and {@code ConnectorPluginManager.createConnector} attaches it to the context + * it hands {@code ConnectorProvider.create}. Editing the file needs an FE restart, same as fe.conf. + */ + default Map getConnectorConfig() { + return Collections.emptyMap(); + } + /** * Returns the HTTP security hook for SSRF protection. * Connectors making outbound HTTP requests should call this hook diff --git a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorProvider.java b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorProvider.java index 33368ff2fff9a5..80f8cf815d7077 100644 --- a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorProvider.java +++ b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorProvider.java @@ -177,6 +177,21 @@ default String displayEngineName() { return getType(); } + /** + * This plugin's identity to the engine. Defaults to {@link #getType()}; two loaded plugins may not + * share one ({@code ConnectorPluginManager.loadPlugins} skips the second). + * + *

It is also the name of this connector's own configuration file: the engine reads + * {@code /.conf} and serves it back through + * {@link ConnectorContext#getConnectorConfig()}. So a connector that ships a conf template must name + * it {@code .conf.template}, and changing this method renames that file — the plugin + * directory name has no say in it, and need not match ({@code hive/hms.conf} is a shipped example). + * Guard it with a test that the template resource exists under {@code name() + ".conf.template"}. + * + *

The engine builds a file name out of this string, which is safe because + * {@code PluginNames.validate} has already confined it to {@code [a-zA-Z0-9._-]} — no separator can + * reach a path. Do not re-validate it. + */ @Override default String name() { return getType(); diff --git a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ForwardingConnectorContext.java b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ForwardingConnectorContext.java index a74bf71a41f22c..b263fc16d82f3b 100644 --- a/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ForwardingConnectorContext.java +++ b/fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ForwardingConnectorContext.java @@ -83,6 +83,11 @@ public Map getEnvironment() { return delegate.getEnvironment(); } + @Override + public Map getConnectorConfig() { + return delegate.getConnectorConfig(); + } + @Override public ConnectorHttpSecurityHook getHttpSecurityHook() { return delegate.getHttpSecurityHook(); diff --git a/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfFileTest.java b/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfFileTest.java new file mode 100644 index 00000000000000..dd2c68a24ff27c --- /dev/null +++ b/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfFileTest.java @@ -0,0 +1,112 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.spi; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +public class ConnectorConfFileTest { + + @TempDir + private Path pluginDir; + + private Map write(String content) throws IOException { + Files.write(pluginDir.resolve("demo.conf"), content.getBytes(StandardCharsets.UTF_8)); + return ConnectorConfFile.load(pluginDir, "demo"); + } + + @Test + public void fileName_isNamePlusSuffix() { + // The plugin's name IS the file name; anything else and the engine looks for a file no plugin ships. + Assertions.assertEquals("hms.conf", ConnectorConfFile.fileName("hms")); + Assertions.assertEquals("trino-connector.conf", ConnectorConfFile.fileName("trino-connector")); + } + + @Test + public void missingFile_isEmptyMapNotAnError() throws IOException { + // Deliberate: a connector whose settings all have defaults or a fe.conf fallback ships no conf at + // all. Making this an error would force every deployment to carry a file of commented-out lines. + Assertions.assertTrue(ConnectorConfFile.load(pluginDir, "demo").isEmpty()); + } + + @Test + public void directoryNamedLikeTheConfFile_isEmptyMapNotAnError() throws IOException { + // isRegularFile, not exists: a directory that happens to be called demo.conf must not blow up + // plugin loading -- it is not a conf file, so the connector simply has none. + Files.createDirectory(pluginDir.resolve("demo.conf")); + Assertions.assertTrue(ConnectorConfFile.load(pluginDir, "demo").isEmpty()); + } + + @Test + public void parsesCommentsBlankLinesAndTrimsValues() throws IOException { + Map conf = write("# a comment\n" + + "\n" + + "drivers_dir = /opt/drivers \n" + + "! another comment style\n" + + "timeout=30\n"); + Assertions.assertEquals(2, conf.size(), conf.toString()); + // Trimmed, because an operator lining up '=' signs is not configuring a path with spaces in it. + Assertions.assertEquals("/opt/drivers", conf.get("drivers_dir")); + Assertions.assertEquals("30", conf.get("timeout")); + } + + @Test + public void blankValueIsKeptSoTheMapReflectsTheFile() throws IOException { + // The loader reports the file as written; deciding that "written but blank" means "not set" is + // ConnectorConf.get's job. Dropping the key here would erase that distinction before anyone + // could act on it -- and would make a future information_schema view lie about the file. + Map conf = write("drivers_dir=\nother= \n"); + Assertions.assertTrue(conf.containsKey("drivers_dir")); + Assertions.assertEquals("", conf.get("drivers_dir")); + Assertions.assertEquals("", conf.get("other")); + } + + @Test + public void readsUtf8NotIso8859() throws IOException { + // Properties.load(InputStream) would decode as ISO-8859-1 and mangle this; the reader overload + // with an explicit UTF-8 charset is what keeps a non-ASCII path usable. + Map conf = write("warehouse=/数据/仓库\n"); + Assertions.assertEquals("/数据/仓库", conf.get("warehouse")); + } + + @Test + public void malformedContent_throwsIoExceptionNamingTheFile() throws IOException { + // Properties.load throws an unchecked IllegalArgumentException on a bad \\uXXXX escape. It is + // rethrown as IOException so the engine's single "this file is unusable" catch covers every way + // the file can be bad -- an unchecked escape would instead abort plugin loading entirely. + Files.write(pluginDir.resolve("demo.conf"), "k=\\uZZZZ\n".getBytes(StandardCharsets.UTF_8)); + IOException e = Assertions.assertThrows(IOException.class, + () -> ConnectorConfFile.load(pluginDir, "demo")); + Assertions.assertTrue(e.getMessage().contains("demo.conf"), e.getMessage()); + } + + @Test + public void returnedMapIsImmutable() throws IOException { + // It is handed to plugin code through getConnectorConfig(); a plugin must not be able to edit + // what another catalog of the same type will read next. + Map conf = write("k=v\n"); + Assertions.assertThrows(UnsupportedOperationException.class, () -> conf.put("k2", "v2")); + } +} diff --git a/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfTest.java b/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfTest.java new file mode 100644 index 00000000000000..5bfe9791e0fb72 --- /dev/null +++ b/fe/fe-connector/fe-connector-spi/src/test/java/org/apache/doris/connector/spi/ConnectorConfTest.java @@ -0,0 +1,140 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.spi; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.Map; + +public class ConnectorConfTest { + + private static ConnectorContext context(Map conf, Map env) { + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + + @Override + public Map getConnectorConfig() { + return conf; + } + + @Override + public Map getEnvironment() { + return env; + } + }; + } + + @Test + public void pluginConfWinsOverFeConf() { + // The whole point of the channel: an administrator who sets the key in the plugin's conf gets + // that value, whatever fe.conf still says. Reverse this precedence and migrating a deployment + // to the new channel silently does nothing. + ConnectorContext ctx = context(Collections.singletonMap("drivers_dir", "/from/plugin/conf"), + Collections.singletonMap("jdbc_drivers_dir", "/from/fe/conf")); + Assertions.assertEquals("/from/plugin/conf", + ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", "/default")); + } + + @Test + public void fallsBackToFeConfWhenThePluginConfHasNoSuchKey() { + // This is what makes the migration non-breaking: an untouched deployment ships no plugin conf, + // so every one of these settings must still resolve to the fe.conf value it resolved to before. + ConnectorContext ctx = context(Collections.emptyMap(), + Collections.singletonMap("jdbc_drivers_dir", "/from/fe/conf")); + Assertions.assertEquals("/from/fe/conf", + ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", "/default")); + } + + @Test + public void blankInThePluginConfFallsBackRatherThanMaskingFeConf() { + // 'drivers_dir=' in a conf file reads as "I have not configured this", not "configure it to the + // empty string". Treating it as a set value would let one stray line hide the value actually in + // effect, and the operator would have no way to tell from either file which one won. + ConnectorContext ctx = context(Collections.singletonMap("drivers_dir", " "), + Collections.singletonMap("jdbc_drivers_dir", "/from/fe/conf")); + Assertions.assertEquals("/from/fe/conf", + ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", "/default")); + } + + @Test + public void blankInFeConfFallsBackToTheDefault() { + ConnectorContext ctx = context(Collections.emptyMap(), + Collections.singletonMap("jdbc_drivers_dir", "")); + Assertions.assertEquals("/default", + ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", "/default")); + } + + @Test + public void defaultWhenNeitherChannelHasIt() { + ConnectorContext ctx = context(Collections.emptyMap(), Collections.emptyMap()); + Assertions.assertEquals("/default", + ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", "/default")); + Assertions.assertNull(ConnectorConf.get(ctx, "drivers_dir", "jdbc_drivers_dir", null)); + } + + @Test + public void nullLegacyKeyNeverReadsTheEnvironment() { + // A setting introduced after this channel has no fe.conf half. It must not accidentally pick up + // an unrelated engine environment entry that happens to share its (unprefixed) name -- 'doris_home' + // and 'doris_version' live in that same map. + ConnectorContext ctx = context(Collections.emptyMap(), + Collections.singletonMap("drivers_dir", "/from/env")); + Assertions.assertEquals("/default", + ConnectorConf.get(ctx, "drivers_dir", null, "/default")); + } + + @Test + public void feConfValueIsReturnedVerbatim() { + // Byte-identical to what the connector read before this channel existed: the fallback path must + // not start trimming values that fe.conf delivered untrimmed, or a migrated connector's behavior + // would differ from the one it replaced in a way no test of the new channel would catch. + ConnectorContext ctx = context(Collections.emptyMap(), + Collections.singletonMap("hive_default_file_format", " orc ")); + Assertions.assertEquals(" orc ", + ConnectorConf.get(ctx, "default_file_format", "hive_default_file_format", "parquet")); + } + + @Test + public void worksOnAContextThatImplementsNeitherGetter() { + // Direct-construction unit tests and classpath built-ins get the interface defaults (two empty + // maps). Reading a setting must degrade to the default, not NPE. + ConnectorContext bare = new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + }; + Assertions.assertEquals("/default", + ConnectorConf.get(bare, "drivers_dir", "jdbc_drivers_dir", "/default")); + } +} diff --git a/fe/fe-connector/fe-connector-spi/src/test/resources/connector-plugin-surface.txt b/fe/fe-connector/fe-connector-spi/src/test/resources/connector-plugin-surface.txt index 10e17766d0918a..287fbe3cbf151c 100644 --- a/fe/fe-connector/fe-connector-spi/src/test/resources/connector-plugin-surface.txt +++ b/fe/fe-connector/fe-connector-spi/src/test/resources/connector-plugin-surface.txt @@ -23,6 +23,7 @@ org.apache.doris.connector.spi.ConnectorContext#createSiblingConnector(java.lang org.apache.doris.connector.spi.ConnectorContext#executeAuthenticated(java.util.concurrent.Callable):java.lang.Object org.apache.doris.connector.spi.ConnectorContext#getCatalogId():long org.apache.doris.connector.spi.ConnectorContext#getCatalogName():java.lang.String +org.apache.doris.connector.spi.ConnectorContext#getConnectorConfig():java.util.Map org.apache.doris.connector.spi.ConnectorContext#getEnvironment():java.util.Map org.apache.doris.connector.spi.ConnectorContext#getHttpSecurityHook():org.apache.doris.connector.api.ConnectorHttpSecurityHook org.apache.doris.connector.spi.ConnectorContext#getStorageContext():org.apache.doris.connector.spi.ConnectorStorageContext From 897c35788b8a54d72be44763aad426c5a570c249 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:15:05 +0800 Subject: [PATCH 02/13] [feat](connector) load a plugin's conf next to its jars The SPI half of this landed with getConnectorConfig() answering an empty map for everyone. This is the engine half: after a plugin directory is admitted, read /.conf and hand it to that plugin's connectors. The map is keyed by provider instance in an IdentityHashMap, so a lookup never calls equals/hashCode on plugin code -- the same rule DirectoryPluginRuntimeManager follows by snapshotting name() once at load and never re-entering the plugin on a query path. Attaching it has to happen in createConnector rather than in the context itself: fe-core builds a ConnectorContext for a catalog before it knows which plugin will claim the type, so the conf can only be layered on once the provider is picked. Keeping it out of DefaultConnectorContext is also what keeps the engine's context free of any connector's key names -- which is the point of the whole change. A sibling connector comes back through this same method, so it is handed its own plugin's conf rather than inheriting the gateway's: DefaultConnectorContext.createSiblingConnector passes the unwrapped engine context, which is then wrapped with the sibling provider's own map. The conf belongs to the plugin, not to the catalog. A conf file that cannot be read is logged and skipped, and the plugin is still registered. Refusing it would make the catalog type vanish, and the only thing a user would see is CREATE CATALOG answering "no provider supports type" -- which points nowhere near a bad file. Every setting reachable this way has a default or a fe.conf fallback, so proceeding is a real degradation path rather than a guess. The tests drive loadPlugins + createConnector on real plugin directories, because the two things worth proving -- that the file is found beside the jars, and that each provider gets its own -- exist only on that path. The probe plugins live outside org.apache.doris.connector. so their API version is read from the jar that defines them; they report back through a sink class that IS in that (parent-first) package, which is what makes the result readable across the classloader split. One test deploys a plugin into a directory NOT named after it, so nothing can pass by reading a file named after the directory instead of after the provider. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../connector/ConnectorConfigContext.java | 47 ++++ .../connector/ConnectorPluginManager.java | 67 ++++- .../apache/doris/connector/ConfProbeSink.java | 71 +++++ .../connector/ConnectorPluginConfTest.java | 253 ++++++++++++++++++ .../ConfProbeConnectorProviderA.java | 52 ++++ .../ConfProbeConnectorProviderB.java | 41 +++ 6 files changed, 530 insertions(+), 1 deletion(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorConfigContext.java create mode 100644 fe/fe-core/src/test/java/org/apache/doris/connector/ConfProbeSink.java create mode 100644 fe/fe-core/src/test/java/org/apache/doris/connector/ConnectorPluginConfTest.java create mode 100644 fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderA.java create mode 100644 fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderB.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorConfigContext.java b/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorConfigContext.java new file mode 100644 index 00000000000000..259cd9801fe9e4 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorConfigContext.java @@ -0,0 +1,47 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector; + +import org.apache.doris.connector.spi.ConnectorContext; +import org.apache.doris.connector.spi.ForwardingConnectorContext; + +import java.util.Map; +import java.util.Objects; + +/** + * A {@link ConnectorContext} carrying one plugin's own {@code .conf}. + * + *

{@link ConnectorPluginManager#createConnector} wraps the engine context in this just before calling + * {@code ConnectorProvider.create}, because that is the first moment the engine knows which plugin the + * catalog type resolved to. Everything else forwards, so this class holds no engine behavior of its own — + * see {@link ForwardingConnectorContext} for why that base class rather than hand-written pass-throughs. + */ +final class ConnectorConfigContext extends ForwardingConnectorContext { + + private final Map conf; + + ConnectorConfigContext(ConnectorContext delegate, Map conf) { + super(delegate); + this.conf = Objects.requireNonNull(conf, "conf"); + } + + @Override + public Map getConnectorConfig() { + return conf; + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorPluginManager.java b/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorPluginManager.java index cd74eeddfda21a..35e2e31cda5b0c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorPluginManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorPluginManager.java @@ -18,6 +18,7 @@ package org.apache.doris.connector; import org.apache.doris.connector.api.Connector; +import org.apache.doris.connector.spi.ConnectorConfFile; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.connector.spi.ConnectorProvider; import org.apache.doris.datasource.CatalogFactory; @@ -32,11 +33,13 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -101,6 +104,18 @@ public class ConnectorPluginManager { new DirectoryPluginRuntimeManager<>(); private final ClassLoadingPolicy classLoadingPolicy = new ClassLoadingPolicy(CONNECTOR_PARENT_FIRST_PREFIXES); + /** + * Each directory-loaded provider's own {@code .conf}, read once at load and served back through + * {@link ConnectorContext#getConnectorConfig()}. + * + *

An {@link IdentityHashMap} because the key is the provider instance and lookup must not call + * {@code equals}/{@code hashCode} on plugin code — the same rule + * {@link DirectoryPluginRuntimeManager} follows by snapshotting {@code name()} once at load and never + * re-entering the plugin on a query path. Written only during startup, read when a catalog is built, + * so a synchronized wrapper is enough. + */ + private final Map> connectorConfigs = + Collections.synchronizedMap(new IdentityHashMap<>()); /** Called at FE startup to load built-in providers from classpath. */ public void loadBuiltins() { @@ -236,6 +251,7 @@ public void loadPlugins(List pluginRoots) { // information_schema.extensions never lists a connector the routing table cannot reach. if (registerDiscovered(handle.getFactory(), false)) { PluginRegistry.getInstance().registerExternal(PLUGIN_FAMILY, handle); + loadConnectorConfig(handle); LOG.info("Loaded connector plugin: name={}, pluginDir={}, jarCount={}", handle.getPluginName(), handle.getPluginDir(), handle.getResolvedJars().size()); @@ -249,6 +265,34 @@ public void loadPlugins(List pluginRoots) { } } + /** + * Reads this plugin's own {@code .conf}, if it ships one, so that + * {@link ConnectorContext#getConnectorConfig()} can serve it later. + * + *

Nothing here can keep the plugin from being registered. A conf file that cannot be read is + * reported and the connector proceeds without it: refusing the plugin would make the catalog type + * vanish entirely, and the only thing a user would then see is {@code CREATE CATALOG} answering + * "no provider supports type", which points nowhere near a bad file. Every setting reachable this way + * either has a default or a fe.conf fallback, so proceeding is a real degradation path, not a guess. + */ + private void loadConnectorConfig(PluginHandle handle) { + Map conf = Collections.emptyMap(); + try { + conf = ConnectorConfFile.load(handle.getPluginDir(), handle.getPluginName()); + } catch (IOException e) { + LOG.error("Failed to read connector plugin conf {} in {}; the connector starts without it " + + "and falls back to fe.conf", ConnectorConfFile.fileName(handle.getPluginName()), + handle.getPluginDir(), e); + } + connectorConfigs.put(handle.getFactory(), conf); + if (!conf.isEmpty()) { + // Key names only. A value here is administrator-supplied and may name a credential path or + // an internal host; the point of the line is to let an operator confirm the file was found. + LOG.info("Connector plugin conf loaded: name={}, file={}, keys={}", handle.getPluginName(), + ConnectorConfFile.fileName(handle.getPluginName()), conf.keySet()); + } + } + private boolean hasProviderNamed(String name) { for (ConnectorProvider p : providers) { if (name.equals(p.name())) { @@ -304,7 +348,7 @@ private Connector createConnector(String catalogType, Map proper } LOG.info("Creating connector via provider '{}' for catalogType='{}'", provider.getType(), catalogType); - return provider.create(properties, context); + return provider.create(properties, withConnectorConfig(context, provider)); } } LOG.debug("No ConnectorProvider supports catalogType='{}' (standaloneOnly={}). Registered: {}", @@ -312,6 +356,27 @@ private Connector createConnector(String catalogType, Map proper return null; } + /** + * Attaches the chosen provider's plugin conf to the context handed to {@code create}. + * + *

It has to happen here rather than in the context itself: fe-core builds a + * {@link ConnectorContext} for a catalog before it knows which plugin will claim the type, so the + * conf can only be layered on once the provider is picked. Keeping it out of + * {@code DefaultConnectorContext} is also what keeps the engine's context free of any connector's + * key names. + * + *

A sibling connector ({@code ConnectorContext.createSiblingConnector}) comes back through this + * same method, so it is handed its own plugin's conf rather than inheriting the gateway's. + * That is the intent: the conf belongs to the plugin, not to the catalog. + * + *

An empty conf is not wrapped. The interface default already answers an empty map, and one fewer + * decorator is one fewer place a future {@link ConnectorContext} method can be lost in. + */ + private ConnectorContext withConnectorConfig(ConnectorContext base, ConnectorProvider provider) { + Map conf = connectorConfigs.get(provider); + return conf == null || conf.isEmpty() ? base : new ConnectorConfigContext(base, conf); + } + /** * Finds the provider that would back a catalog of this type, without creating a connector. For engine * decisions that must be answered for a catalog that may not be initialized yet — asking the connector diff --git a/fe/fe-core/src/test/java/org/apache/doris/connector/ConfProbeSink.java b/fe/fe-core/src/test/java/org/apache/doris/connector/ConfProbeSink.java new file mode 100644 index 00000000000000..37fb70e410ac65 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/connector/ConfProbeSink.java @@ -0,0 +1,71 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector; + +import org.apache.doris.connector.api.Connector; +import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorSession; + +import java.util.HashMap; +import java.util.Map; + +/** + * Where the temporary probe plugins of {@link ConnectorPluginConfTest} hand back what the engine gave them. + * + *

It sits in {@code org.apache.doris.connector} deliberately. That prefix is parent-first for the + * CONNECTOR family, so the plugin's child-first classloader delegates this class to the FE's own loader and + * both sides share one Class — and therefore one static map. A probe provider cannot report through its own + * types: those really are child-loaded (which is the point — the loader reads a plugin's declared API + * version from the jar that defines its factory class), so anything it returns is uncastable on this side. + * + *

The {@link Connector} it hands back is likewise defined here rather than in the plugin jar, so the jar + * needs to carry nothing but the provider itself. + */ +public final class ConfProbeSink { + + private static final Map> SEEN = new HashMap<>(); + + private ConfProbeSink() { + } + + /** Records the connector config a provider was handed, and gives it a connector to return. */ + public static Connector record(String type, Map connectorConfig) { + SEEN.put(type, new HashMap<>(connectorConfig)); + return new ProbeConnector(); + } + + /** What the provider of {@code type} was handed, or null if it was never asked to create anything. */ + public static Map seen(String type) { + return SEEN.get(type); + } + + public static void reset() { + SEEN.clear(); + } + + private static final class ProbeConnector implements Connector { + @Override + public ConnectorMetadata getMetadata(ConnectorSession session) { + return null; + } + + @Override + public void close() { + } + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/connector/ConnectorPluginConfTest.java b/fe/fe-core/src/test/java/org/apache/doris/connector/ConnectorPluginConfTest.java new file mode 100644 index 00000000000000..e1de13a64a9a7a --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/connector/ConnectorPluginConfTest.java @@ -0,0 +1,253 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector; + +import org.apache.doris.connector.api.Connector; +import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorSession; +import org.apache.doris.connector.spi.ConnectorContext; +import org.apache.doris.connector.spi.ConnectorProvider; +import org.apache.doris.connectorconf.testplugins.ConfProbeConnectorProviderA; +import org.apache.doris.connectorconf.testplugins.ConfProbeConnectorProviderB; +import org.apache.doris.extension.loader.ApiVersionGate; +import org.apache.doris.extension.loader.PluginRegistry; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Map; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; + +/** + * A connector plugin's own {@code .conf} really reaches the connector, on real plugin directories. + * + *

Everything here goes through {@code loadPlugins} + {@code createConnector} rather than the + * {@code registerDiscovered} seam the rest of {@link ConnectorPluginManagerTest} uses, because the two + * things worth proving — that the file is found next to the jars, and that each provider gets its own + * file — only exist on that path. + */ +public class ConnectorPluginConfTest { + + @TempDir + Path tempDir; + + private ConnectorPluginManager manager; + + @BeforeEach + @AfterEach + void reset() { + // loadPlugins writes inventory rows into a process-wide singleton; leaving them behind would make + // information_schema.extensions assertions in other tests depend on execution order. + PluginRegistry.getInstance().clearForTest(); + ConfProbeSink.reset(); + manager = new ConnectorPluginManager(); + } + + @Test + public void connectorReceivesTheConfShippedNextToItsJars() throws IOException { + Path root = pluginRoot(); + deployPlugin(root, ConfProbeConnectorProviderA.class, ConfProbeConnectorProviderA.TYPE, + "drivers_dir=/opt/drivers\ntimeout=30\n"); + + manager.loadPlugins(Collections.singletonList(root)); + manager.createConnector(ConfProbeConnectorProviderA.TYPE, Collections.emptyMap(), context()); + + Map seen = ConfProbeSink.seen(ConfProbeConnectorProviderA.TYPE); + Assertions.assertNotNull(seen, "the probe provider was never asked to create a connector"); + Assertions.assertEquals("/opt/drivers", seen.get("drivers_dir")); + Assertions.assertEquals("30", seen.get("timeout")); + } + + @Test + public void confIsNamedAfterTheProviderNotAfterTheDirectory() throws IOException { + // The plugin directory name is the deployer's choice; the conf file name is the plugin's own + // identity. hive/hms.conf ships exactly this way, so a file named after the directory must NOT be + // picked up -- otherwise renaming a plugin directory would silently change which file is read. + Path root = pluginRoot(); + Path dir = deployPlugin(root, ConfProbeConnectorProviderA.class, + ConfProbeConnectorProviderA.TYPE, null); + Files.write(dir.resolve("some_plugin_dir.conf"), + "drivers_dir=/wrong\n".getBytes(StandardCharsets.UTF_8)); + + manager.loadPlugins(Collections.singletonList(root)); + manager.createConnector(ConfProbeConnectorProviderA.TYPE, Collections.emptyMap(), context()); + + Assertions.assertEquals(Collections.emptyMap(), + ConfProbeSink.seen(ConfProbeConnectorProviderA.TYPE), + "only .conf may be read"); + } + + @Test + public void missingConfLeavesTheProviderRegisteredWithAnEmptyMap() throws IOException { + // Shipping no conf at all is the normal case for a connector whose settings have defaults or a + // fe.conf fallback. It must cost the plugin nothing. + Path root = pluginRoot(); + deployPlugin(root, ConfProbeConnectorProviderA.class, ConfProbeConnectorProviderA.TYPE, null); + + manager.loadPlugins(Collections.singletonList(root)); + + Assertions.assertTrue(manager.getRegisteredTypes().contains(ConfProbeConnectorProviderA.TYPE), + manager.getRegisteredTypes().toString()); + Assertions.assertNotNull( + manager.createConnector(ConfProbeConnectorProviderA.TYPE, Collections.emptyMap(), context())); + Assertions.assertEquals(Collections.emptyMap(), + ConfProbeSink.seen(ConfProbeConnectorProviderA.TYPE)); + } + + @Test + public void unreadableConfLeavesTheProviderRegisteredWithAnEmptyMap() throws IOException { + // A broken file must not make the catalog type disappear: CREATE CATALOG would then answer "no + // provider supports type", which points nowhere near the real cause. The connector proceeds + // without the file and falls back to fe.conf. + Path root = pluginRoot(); + deployPlugin(root, ConfProbeConnectorProviderA.class, ConfProbeConnectorProviderA.TYPE, + "k=\\uZZZZ\n"); + + manager.loadPlugins(Collections.singletonList(root)); + + Assertions.assertTrue(manager.getRegisteredTypes().contains(ConfProbeConnectorProviderA.TYPE), + "a bad conf file must not cost the deployment its catalog type"); + manager.createConnector(ConfProbeConnectorProviderA.TYPE, Collections.emptyMap(), context()); + Assertions.assertEquals(Collections.emptyMap(), + ConfProbeSink.seen(ConfProbeConnectorProviderA.TYPE)); + } + + @Test + public void onePluginsConfNeverReachesAnother() throws IOException { + // The map is keyed by provider instance, and this is the assertion that keeps it that way. It is + // also what makes a sibling connector correct: createSiblingConnector comes back through + // createConnector, so a gateway's sibling is handed its OWN plugin's conf, not the gateway's. + Path root = pluginRoot(); + deployPlugin(root, ConfProbeConnectorProviderA.class, ConfProbeConnectorProviderA.TYPE, + "shared_key=from_a\nonly_in_a=yes\n"); + deployPlugin(root, ConfProbeConnectorProviderB.class, ConfProbeConnectorProviderB.TYPE, + "shared_key=from_b\n"); + + manager.loadPlugins(Collections.singletonList(root)); + manager.createConnector(ConfProbeConnectorProviderA.TYPE, Collections.emptyMap(), context()); + manager.createConnector(ConfProbeConnectorProviderB.TYPE, Collections.emptyMap(), context()); + + Map seenA = ConfProbeSink.seen(ConfProbeConnectorProviderA.TYPE); + Map seenB = ConfProbeSink.seen(ConfProbeConnectorProviderB.TYPE); + Assertions.assertEquals("from_a", seenA.get("shared_key")); + Assertions.assertEquals("from_b", seenB.get("shared_key")); + Assertions.assertNull(seenB.get("only_in_a"), "B must not see a key only A's conf declares"); + } + + @Test + public void providerWithNoPluginDirectoryGetsAnEmptyMap() { + // Classpath built-ins and providers a test registers directly have no plugin directory, so there + // is no file to read. They must fall through to the interface default rather than to a null map. + manager.registerProvider(new ConnectorProvider() { + @Override + public String getType() { + return "no_plugin_dir"; + } + + @Override + public Connector create(Map properties, ConnectorContext context) { + Assertions.assertEquals(Collections.emptyMap(), context.getConnectorConfig()); + return new Connector() { + @Override + public ConnectorMetadata getMetadata(ConnectorSession session) { + return null; + } + + @Override + public void close() { + } + }; + } + }); + + Assertions.assertNotNull( + manager.createConnector("no_plugin_dir", Collections.emptyMap(), context())); + } + + private Path pluginRoot() throws IOException { + Path root = tempDir.resolve("connector-plugins"); + Files.createDirectories(root); + return root; + } + + private static ConnectorContext context() { + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + }; + } + + /** + * Lays out one plugin the way the assembly and build.sh really do: {@code /

/.jar} with + * the provider's class bytes, its ServiceLoader registration and the served API version in the + * MANIFEST — plus, when {@code confContent} is given, {@code .conf} beside the jar. + * + *

The directory is deliberately NOT named after the provider, so that nothing here can pass by + * accidentally reading a file named after the directory. + */ + private Path deployPlugin(Path root, Class providerClass, String providerName, String confContent) + throws IOException { + Path dir = root.resolve("some_plugin_dir_" + providerName); + Files.createDirectories(dir); + Path jarPath = dir.resolve("plugin.jar"); + + Manifest manifest = new Manifest(); + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + manifest.getMainAttributes().putValue("Doris-Connector-Plugin-Api-Version", + ApiVersionGate.forFamily("connector", ConnectorProvider.class).getExpectedVersion()); + String classEntry = providerClass.getName().replace('.', '/') + ".class"; + try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(jarPath), manifest)) { + jar.putNextEntry(new JarEntry(classEntry)); + try (InputStream classBytes = providerClass.getClassLoader().getResourceAsStream(classEntry)) { + Assertions.assertNotNull(classBytes, "class bytes not found: " + classEntry); + byte[] buffer = new byte[8192]; + int read; + while ((read = classBytes.read(buffer)) != -1) { + jar.write(buffer, 0, read); + } + } + jar.closeEntry(); + jar.putNextEntry(new JarEntry("META-INF/services/" + ConnectorProvider.class.getName())); + jar.write((providerClass.getName() + "\n").getBytes(StandardCharsets.UTF_8)); + jar.closeEntry(); + } + if (confContent != null) { + Files.write(dir.resolve(providerName + ".conf"), confContent.getBytes(StandardCharsets.UTF_8)); + } + return dir; + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderA.java b/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderA.java new file mode 100644 index 00000000000000..4e36a6adf7c9ce --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderA.java @@ -0,0 +1,52 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connectorconf.testplugins; + +import org.apache.doris.connector.ConfProbeSink; +import org.apache.doris.connector.api.Connector; +import org.apache.doris.connector.spi.ConnectorContext; +import org.apache.doris.connector.spi.ConnectorProvider; + +import java.util.Map; + +/** + * A connector provider whose class bytes are copied into a temporary plugin jar, so a test can load it the + * way FE loads a shipped connector and see what {@code getConnectorConfig()} delivered. + * + *

It lives outside {@code org.apache.doris.connector.} on purpose: that prefix is parent-first, and the + * loader reads a plugin's declared API version from the jar that defines its factory class — a + * parent-first provider would always look undeclared and be refused. It reports through + * {@link ConfProbeSink}, which is parent-first and therefore shared with the test. + * + *

{@link ConfProbeConnectorProviderB} is its twin, deployed as a second plugin, so that a test can prove + * one plugin's conf never reaches the other. + */ +public class ConfProbeConnectorProviderA implements ConnectorProvider { + + public static final String TYPE = "conf_probe_a"; + + @Override + public String getType() { + return TYPE; + } + + @Override + public Connector create(Map properties, ConnectorContext context) { + return ConfProbeSink.record(TYPE, context.getConnectorConfig()); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderB.java b/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderB.java new file mode 100644 index 00000000000000..c77d6b9a265496 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/connectorconf/testplugins/ConfProbeConnectorProviderB.java @@ -0,0 +1,41 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connectorconf.testplugins; + +import org.apache.doris.connector.ConfProbeSink; +import org.apache.doris.connector.api.Connector; +import org.apache.doris.connector.spi.ConnectorContext; +import org.apache.doris.connector.spi.ConnectorProvider; + +import java.util.Map; + +/** The second probe plugin; see {@link ConfProbeConnectorProviderA}. */ +public class ConfProbeConnectorProviderB implements ConnectorProvider { + + public static final String TYPE = "conf_probe_b"; + + @Override + public String getType() { + return TYPE; + } + + @Override + public Connector create(Map properties, ConnectorContext context) { + return ConfProbeSink.record(TYPE, context.getConnectorConfig()); + } +} From c2f930fe97631878133081f548fcdd5c907b9b5c Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:15:18 +0800 Subject: [PATCH 03/13] [feat](connector) ship a conf template with each plugin Seeds .conf from the .conf.template the plugin zip carries, with cp -n, right after the zip is unpacked. The live .conf is deliberately NOT in the zip. That is what makes the ordinary upgrade -- unzip a newer plugin build over the deployed directory -- refresh the jars and the template while leaving whatever the administrator configured untouched. Shipping the .conf itself would silently revert their settings on every upgrade. The loop globs *.conf.template and names no connector, so a new connector that ships a template needs no change here. It sits inside the deploy loop because conn_plugin_target is unset once the loop ends. cp -n exits 0 when it skips an existing file, so it is safe under this script's set -eo pipefail; the [ -e ] guard covers a plugin that ships no template, where the glob stays literal. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- build.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 36f72ef562228f..50539178292547 100755 --- a/build.sh +++ b/build.sh @@ -1082,8 +1082,17 @@ if [[ "${BUILD_FE}" -eq 1 ]]; then fi mkdir -p "${conn_plugin_target}" unzip -o "${conn_zip}" -d "${conn_plugin_target}/" + # A connector's own settings file. The zip carries only .conf.template; the live + # .conf is seeded from it here and never overwritten, so an upgrade that unzips a new + # plugin build over this directory refreshes the jars and the template but leaves whatever the + # administrator configured. Deliberately generic (globbed on *.conf.template, no connector + # named): a new connector ships a template and needs no change here. + for conn_conf_tpl in "${conn_plugin_target}"/*.conf.template; do + [ -e "${conn_conf_tpl}" ] || continue + cp -n "${conn_conf_tpl}" "${conn_conf_tpl%.template}" + done done - unset CONN_PLUGIN_DIR conn_module conn_plugin_target conn_module_dir conn_zip + unset CONN_PLUGIN_DIR conn_module conn_plugin_target conn_module_dir conn_zip conn_conf_tpl # RC-4: self-contain the paimon connector plugin for OSS. The connector sets # fs.oss.impl=com.aliyun.jindodata.oss.JindoOssFileSystem; that impl lives in the jindofs jars, From b52c87ffe6e6d2672611a396687dc71760b0e7d9 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:22:24 +0800 Subject: [PATCH 04/13] [feat](trino) read the plugin dir from the plugin conf Moves trino_connector_plugin_dir onto the connector's own settings file. plugin_dir in trino-connector.conf now wins; fe.conf's key stays as the fallback, so a deployment that changes nothing keeps resolving exactly as before and its @ConfField keeps working. resolvePluginDir takes the resolved directory rather than the engine environment map. Which file a deployment-level setting comes from is the connector's business, not this helper's, and threading the map through meant the fe.conf key name was baked into a function that has no other reason to know it. Note the conf file is trino-connector.conf, not trino.conf: the engine names it after ConnectorProvider.name(), which is this connector's type, while the plugin directory it sits in is plugins/connector/trino. The two are allowed to differ -- the directory name is the deployer's choice and cannot be what the engine keys on. A test asserts the shipped template's name still tracks name(), so renaming getType() cannot silently deploy a file nothing opens. The fail-loud on a missing value is kept, and its message now names both places the setting can come from, since after this either one could be the missing half. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../src/main/assembly/plugin-zip.xml | 11 +++ .../doris/connector/trino/TrinoBootstrap.java | 31 +++--- .../trino/TrinoConnectorProvider.java | 19 +++- .../connector/trino/TrinoDorisConnector.java | 10 +- .../resources/trino-connector.conf.template | 18 ++++ .../connector/trino/TrinoBootstrapTest.java | 97 ++++++++++++++----- 6 files changed, 140 insertions(+), 46 deletions(-) create mode 100644 fe/fe-connector/fe-connector-trino/src/main/resources/trino-connector.conf.template diff --git a/fe/fe-connector/fe-connector-trino/src/main/assembly/plugin-zip.xml b/fe/fe-connector/fe-connector-trino/src/main/assembly/plugin-zip.xml index be333bbb7e616f..88edc785e8cb8c 100644 --- a/fe/fe-connector/fe-connector-trino/src/main/assembly/plugin-zip.xml +++ b/fe/fe-connector/fe-connector-trino/src/main/assembly/plugin-zip.xml @@ -42,6 +42,17 @@ under the License. ${project.build.directory}/${project.build.finalName}.jar / + + + src/main/resources/trino-connector.conf.template + / + diff --git a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoBootstrap.java b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoBootstrap.java index 0329f8423199fa..42b0f171969409 100644 --- a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoBootstrap.java +++ b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoBootstrap.java @@ -321,17 +321,10 @@ private static void configureJulLogging() { /** * Resolves the Trino plugin directory. * - *

This plugin runs in an isolated classloader and cannot read FE {@code Config} - * (it would see its own bundled copy holding default values). The FE config - * {@code trino_connector_plugin_dir} is therefore passed in through the engine - * environment map (see {@code DefaultConnectorContext}), mirroring how the JDBC - * connector receives {@code jdbc_drivers_dir}. - * *

Resolution order: *

    *
  1. the per-catalog {@code trino.plugin.dir} property, when set;
    2. - *
  2. otherwise the FE config {@code trino_connector_plugin_dir} from the environment, - * used verbatim (it defaults to {@code DORIS_HOME/plugins/trino_plugins}).
    3. + *
  3. otherwise {@code configuredDir}, used verbatim.
    4. *
* *

Nothing else is consulted: the dir the config names is the dir the plugins are loaded @@ -339,25 +332,27 @@ private static void configureJulLogging() { * {@code DORIS_HOME/plugins/connectors} when the config was left at its default, which forced * this class to duplicate the default as a literal just to tell "user set it" from "untouched". * That compatibility path was dropped deliberately — a deployment whose plugins still sit in a - * legacy dir must move them or point {@code trino_connector_plugin_dir} at them. + * legacy dir must move them or point the config at them. * - * @param properties catalog properties (unstripped, may carry {@code trino.plugin.dir}) - * @param environment engine environment from {@code ConnectorContext.getEnvironment()} + * @param properties catalog properties (unstripped, may carry {@code trino.plugin.dir}) + * @param configuredDir the deployment-level setting, already resolved by the caller from + * {@code plugin_dir} in the plugin's own conf or {@code trino_connector_plugin_dir} + * in fe.conf; null or empty means the engine delivered neither */ - public static String resolvePluginDir(Map properties, Map environment) { + public static String resolvePluginDir(Map properties, String configuredDir) { String explicitDir = properties.get("trino.plugin.dir"); if (explicitDir != null && !explicitDir.isEmpty()) { return explicitDir; } - String configuredDir = environment.get("trino_connector_plugin_dir"); if (configuredDir == null || configuredDir.isEmpty()) { - // DefaultConnectorContext always passes the FE config, which always holds a value. Absent - // means the engine failed to deliver it; guessing a dir here would surface as "catalog - // creates fine but every query fails", so fail where the cause is still visible. + // fe.conf always holds a value for this, and the engine always forwards it, so absent means + // the engine failed to deliver it. Guessing a dir here would surface as "catalog creates fine + // but every query fails", so fail where the cause is still visible. throw new IllegalStateException( - "trino_connector_plugin_dir was not delivered through the engine environment; " - + "cannot resolve the Trino plugin dir"); + "neither '" + TrinoConnectorProvider.CONF_PLUGIN_DIR + "' in " + + TrinoConnectorProvider.TYPE + ".conf nor trino_connector_plugin_dir in " + + "fe.conf was delivered; cannot resolve the Trino plugin dir"); } return configuredDir; } diff --git a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoConnectorProvider.java b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoConnectorProvider.java index f685261c23b231..f7e207f0b2d296 100644 --- a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoConnectorProvider.java +++ b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoConnectorProvider.java @@ -31,9 +31,26 @@ public class TrinoConnectorProvider implements ConnectorProvider { static final String TRINO_CONNECTOR_NAME = "trino.connector.name"; + /** + * This connector's type, and therefore its {@code name()} — which is what the engine names its + * conf file after, so the plugin must ship {@code trino-connector.conf.template}. Note that this is + * NOT the plugin directory name ({@code plugins/connector/trino}); the directory is the deployer's + * choice, the conf file name is this string. + */ + public static final String TYPE = "trino-connector"; + + /** + * Directory holding the Trino plugins this connector loads, in {@code trino-connector.conf}. + * Falls back to fe.conf's {@code trino_connector_plugin_dir}, which is where it used to live. + */ + public static final String CONF_PLUGIN_DIR = "plugin_dir"; + + /** The fe.conf name of {@link #CONF_PLUGIN_DIR}, forwarded through the engine environment. */ + public static final String ENV_PLUGIN_DIR = "trino_connector_plugin_dir"; + @Override public String getType() { - return "trino-connector"; + return TYPE; } @Override diff --git a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoDorisConnector.java b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoDorisConnector.java index 64f31fd8f68196..bd877e04d4a200 100644 --- a/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoDorisConnector.java +++ b/fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoDorisConnector.java @@ -22,6 +22,7 @@ import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorValidationContext; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; +import org.apache.doris.connector.spi.ConnectorConf; import org.apache.doris.connector.spi.ConnectorContext; import com.google.common.collect.ImmutableMap; @@ -164,9 +165,12 @@ private void doInitialize() { } // 2. Initialize Trino plugin infrastructure (singleton). - // The plugin dir comes from the FE engine environment (fe-core reads fe.conf); - // this plugin's classloader cannot see FE Config directly. - String pluginDir = TrinoBootstrap.resolvePluginDir(properties, context.getEnvironment()); + // The plugin dir is a deployment-level setting: this plugin's classloader cannot see FE Config + // directly, so it arrives either in this plugin's own trino-connector.conf or, for a deployment + // that has not moved to that file, from fe.conf through the engine environment. + String pluginDir = TrinoBootstrap.resolvePluginDir(properties, + ConnectorConf.get(context, TrinoConnectorProvider.CONF_PLUGIN_DIR, + TrinoConnectorProvider.ENV_PLUGIN_DIR, null)); TrinoBootstrap bootstrap = TrinoBootstrap.getInstance(pluginDir); // 3. Create Trino Connector + Session for this catalog diff --git a/fe/fe-connector/fe-connector-trino/src/main/resources/trino-connector.conf.template b/fe/fe-connector/fe-connector-trino/src/main/resources/trino-connector.conf.template new file mode 100644 index 00000000000000..e1d660d21fecca --- /dev/null +++ b/fe/fe-connector/fe-connector-trino/src/main/resources/trino-connector.conf.template @@ -0,0 +1,18 @@ +# Trino connector plugin configuration. +# +# build.sh seeds trino-connector.conf from this file on first deploy and never overwrites it, so an +# upgrade that unzips a new plugin build over this directory keeps whatever you configured here. +# This file must exist on EVERY FE node -- it is not replicated through Doris metadata. +# Changes take effect after an FE restart. +# +# The file name is the connector's name (ConnectorProvider.name()), NOT the directory name: this +# plugin is deployed under plugins/connector/trino/ but its settings file is trino-connector.conf. +# +# Every setting below is optional. Left commented out, each falls back to the fe.conf key named +# after it, and then to the built-in default -- so an existing deployment needs no change here. + +# Directory holding the Trino plugins this connector loads (a directory of per-plugin subdirectories). +# Falls back to fe.conf's trino_connector_plugin_dir, whose default is +# /plugins/trino_plugins. A catalog may override it per catalog with the +# trino.plugin.dir property, which wins over both. +# plugin_dir=/opt/doris/plugins/trino_plugins diff --git a/fe/fe-connector/fe-connector-trino/src/test/java/org/apache/doris/connector/trino/TrinoBootstrapTest.java b/fe/fe-connector/fe-connector-trino/src/test/java/org/apache/doris/connector/trino/TrinoBootstrapTest.java index 842b7b1b8449e8..5c56aff30460ea 100644 --- a/fe/fe-connector/fe-connector-trino/src/test/java/org/apache/doris/connector/trino/TrinoBootstrapTest.java +++ b/fe/fe-connector/fe-connector-trino/src/test/java/org/apache/doris/connector/trino/TrinoBootstrapTest.java @@ -17,6 +17,9 @@ package org.apache.doris.connector.trino; +import org.apache.doris.connector.spi.ConnectorConf; +import org.apache.doris.connector.spi.ConnectorContext; + import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -49,30 +52,19 @@ private static void installPluginIn(Path dorisHome, String subdir) throws IOExce Files.createDirectories(dorisHome.resolve(subdir).resolve("trino-hive")); } - private static Map envAtDefault(Path dorisHome) { - return ImmutableMap.of( - "doris_home", dorisHome.toString(), - "trino_connector_plugin_dir", dorisHome + "/plugins/trino_plugins"); - } - @Test public void perCatalogPropertyTakesPrecedence() { - Map env = ImmutableMap.of( - "doris_home", "/opt/doris", - "trino_connector_plugin_dir", "/should/be/ignored"); String resolved = TrinoBootstrap.resolvePluginDir( - ImmutableMap.of("trino.plugin.dir", "/custom/catalog/dir"), env); + ImmutableMap.of("trino.plugin.dir", "/custom/catalog/dir"), "/should/be/ignored"); Assertions.assertEquals("/custom/catalog/dir", resolved); } @Test - public void feConfigFromEnvironmentIsHonored() { - // Exactly what the regression environment sets in fe.conf, delivered via the - // engine environment because the plugin classloader cannot read FE Config. - Map env = ImmutableMap.of( - "doris_home", "/opt/doris", - "trino_connector_plugin_dir", "/tmp/trino_connector/connectors"); - String resolved = TrinoBootstrap.resolvePluginDir(Collections.emptyMap(), env); + public void theDeploymentLevelSettingIsHonored() { + // Exactly what the regression environment configures, delivered by the engine because the + // plugin classloader cannot read FE Config. + String resolved = TrinoBootstrap.resolvePluginDir( + Collections.emptyMap(), "/tmp/trino_connector/connectors"); Assertions.assertEquals("/tmp/trino_connector/connectors", resolved); } @@ -86,17 +78,74 @@ public void legacyPluginDirsAreNotConsultedEvenWhenTheyHoldPlugins(@TempDir Path installPluginIn(dorisHome, "connectors"); installPluginIn(dorisHome, "plugins/connectors"); - String resolved = TrinoBootstrap.resolvePluginDir(Collections.emptyMap(), envAtDefault(dorisHome)); + String resolved = TrinoBootstrap.resolvePluginDir( + Collections.emptyMap(), dorisHome + "/plugins/trino_plugins"); Assertions.assertEquals(dorisHome + "/plugins/trino_plugins", resolved); } @Test - public void missingConfigInTheEnvironmentFailsLoud() { - // DefaultConnectorContext always passes the FE config, so an absent key means the engine - // failed to deliver it. Guessing a dir would surface far away as "catalog creates fine but - // every query fails"; throwing keeps the cause at the point of breakage. + public void missingSettingFailsLoudNamingBothPlacesItCanBeSet() { + // fe.conf always holds a value for this and the engine always forwards it, so absent means the + // engine failed to deliver it. Guessing a dir would surface far away as "catalog creates fine + // but every query fails"; throwing keeps the cause at the point of breakage. The message names + // both channels because after the migration either one could be the missing half. + IllegalStateException e = Assertions.assertThrows(IllegalStateException.class, + () -> TrinoBootstrap.resolvePluginDir(Collections.emptyMap(), null)); + Assertions.assertTrue(e.getMessage().contains("trino-connector.conf"), e.getMessage()); + Assertions.assertTrue(e.getMessage().contains("trino_connector_plugin_dir"), e.getMessage()); Assertions.assertThrows(IllegalStateException.class, - () -> TrinoBootstrap.resolvePluginDir( - Collections.emptyMap(), ImmutableMap.of("doris_home", "/opt/doris"))); + () -> TrinoBootstrap.resolvePluginDir(Collections.emptyMap(), "")); + } + + @Test + public void pluginConfBeatsFeConfAndBothBeatNothing() { + // The resolution the connector performs before calling resolvePluginDir. Asserted here rather + // than trusted, because getting it backwards would make an administrator's edit to + // trino-connector.conf silently do nothing while fe.conf still holds the old value. + Assertions.assertEquals("/from/plugin/conf", ConnectorConf.get( + context(ImmutableMap.of(TrinoConnectorProvider.CONF_PLUGIN_DIR, "/from/plugin/conf"), + ImmutableMap.of(TrinoConnectorProvider.ENV_PLUGIN_DIR, "/from/fe/conf")), + TrinoConnectorProvider.CONF_PLUGIN_DIR, TrinoConnectorProvider.ENV_PLUGIN_DIR, null)); + + Assertions.assertEquals("/from/fe/conf", ConnectorConf.get( + context(ImmutableMap.of(), + ImmutableMap.of(TrinoConnectorProvider.ENV_PLUGIN_DIR, "/from/fe/conf")), + TrinoConnectorProvider.CONF_PLUGIN_DIR, TrinoConnectorProvider.ENV_PLUGIN_DIR, null)); + + Assertions.assertNull(ConnectorConf.get(context(ImmutableMap.of(), ImmutableMap.of()), + TrinoConnectorProvider.CONF_PLUGIN_DIR, TrinoConnectorProvider.ENV_PLUGIN_DIR, null)); + } + + @Test + public void theConfTemplateIsNamedAfterTheProvider() { + // The engine reads .conf, so a template under any other name deploys a file nothing ever + // opens -- silently, with every setting in it ignored. Renaming getType() must break here. + String expected = new TrinoConnectorProvider().name() + ".conf.template"; + Assertions.assertNotNull(getClass().getClassLoader().getResource(expected), + "the plugin must ship " + expected + " on its classpath"); + } + + private static ConnectorContext context(Map conf, Map env) { + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + + @Override + public Map getConnectorConfig() { + return conf; + } + + @Override + public Map getEnvironment() { + return env; + } + }; } } From 97c8e211213f235566d908853cbea7f7dce5f346 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:22:36 +0800 Subject: [PATCH 05/13] [feat](hive) read the create-table defaults from the plugin conf Moves hive_default_file_format and enable_create_hive_bucket_table onto the connector's own settings file, as default_file_format and enable_create_bucket_table in hms.conf. The plugin conf wins; fe.conf stays as the fallback, so a deployment that changes nothing behaves exactly as before and both @ConfFields keep working. The keys lose their hive_ prefix because the file name already namespaces them -- hms.conf can only be read by this connector. Note the file is hms.conf, not hive.conf: the engine names it after ConnectorProvider.name(), which for this connector is "hms", while the plugin directory it sits in is plugins/connector/hive. The two are allowed to differ, and a test asserts the shipped template's name still tracks name() so renaming getType() cannot silently deploy a file nothing opens. doris_version stays in the engine environment. It is a build stamp rather than something an administrator configures, so it does not belong in a settings file at all. One behavior difference worth naming: a blank value now counts as unset. Before, a blank hive_default_file_format reached the metastore create as an empty format string; now it falls through to orc. That is the intent of a blank line in a conf file, and an empty file format was never a working configuration. The bucket-gate rejection message now names both the conf key and the fe.conf key, since after this either one could be the one that is off. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../src/main/assembly/plugin-zip.xml | 11 +++ .../connector/hive/HiveConnectorMetadata.java | 13 ++-- .../hive/HiveConnectorProperties.java | 7 ++ .../src/main/resources/hms.conf.template | 21 ++++++ .../connector/hive/FakeConnectorContext.java | 19 +++++- .../hive/HiveConnectorMetadataDdlTest.java | 67 +++++++++++++++++++ 6 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 fe/fe-connector/fe-connector-hive/src/main/resources/hms.conf.template diff --git a/fe/fe-connector/fe-connector-hive/src/main/assembly/plugin-zip.xml b/fe/fe-connector/fe-connector-hive/src/main/assembly/plugin-zip.xml index 2e141b72f15096..89d021ed83c3ba 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/assembly/plugin-zip.xml +++ b/fe/fe-connector/fe-connector-hive/src/main/assembly/plugin-zip.xml @@ -40,6 +40,17 @@ under the License. ${project.build.directory}/${project.build.finalName}.jar / + + + src/main/resources/hms.conf.template + / + diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java index 261eab5a67ca3d..603c2071e23355 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java @@ -67,6 +67,7 @@ import org.apache.doris.connector.hms.HmsPartitionInfo; import org.apache.doris.connector.hms.HmsTableInfo; import org.apache.doris.connector.hms.HmsTypeMapping; +import org.apache.doris.connector.spi.ConnectorConf; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.connector.spi.ConnectorStorageContext; import org.apache.doris.filesystem.FileSystem; @@ -1618,7 +1619,8 @@ public void createTable(ConnectorSession session, ConnectorCreateTableRequest re } Map env = context.getEnvironment(); String fileFormat = userProps.getOrDefault(HiveConnectorProperties.CREATE_FILE_FORMAT, - env.getOrDefault(HiveConnectorProperties.ENV_HIVE_DEFAULT_FILE_FORMAT, + ConnectorConf.get(context, HiveConnectorProperties.CONF_DEFAULT_FILE_FORMAT, + HiveConnectorProperties.ENV_HIVE_DEFAULT_FILE_FORMAT, HiveConnectorProperties.DEFAULT_FILE_FORMAT)); // Metastore table parameters: lower-case every key and stamp the file_format / location keys under a @@ -1673,11 +1675,14 @@ public void createTable(ConnectorSession session, ConnectorCreateTableRequest re // enable gate first, then the hash requirement. ConnectorBucketSpec bucketSpec = request.getBucketSpec(); if (bucketSpec != null) { - boolean bucketEnabled = Boolean.parseBoolean(env.getOrDefault( + boolean bucketEnabled = Boolean.parseBoolean(ConnectorConf.get(context, + HiveConnectorProperties.CONF_ENABLE_CREATE_BUCKET_TABLE, HiveConnectorProperties.ENV_ENABLE_CREATE_HIVE_BUCKET_TABLE, "false")); if (!bucketEnabled) { - throw new DorisConnectorException( - "Create hive bucket table need set enable_create_hive_bucket_table to true"); + throw new DorisConnectorException("Create hive bucket table need set '" + + HiveConnectorProperties.CONF_ENABLE_CREATE_BUCKET_TABLE + "' in hms.conf (or " + + HiveConnectorProperties.ENV_ENABLE_CREATE_HIVE_BUCKET_TABLE + + " in fe.conf) to true"); } if (HiveConnectorProperties.BUCKET_ALGO_RANDOM.equals(bucketSpec.getAlgorithm())) { throw new DorisConnectorException("External hive table only supports hash bucketing"); diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorProperties.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorProperties.java index dae9f256b0d62b..590a26f9c3d23a 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorProperties.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorProperties.java @@ -73,6 +73,13 @@ private HiveConnectorProperties() { new HashSet<>(Arrays.asList(CREATE_FILE_FORMAT, CREATE_LOCATION))); public static final String DORIS_PROP_PREFIX = "doris."; + // -- deployment-level settings, read from this plugin's own hms.conf -- + // The file is named after ConnectorProvider.name() ("hms"), NOT after the plugin directory + // ("plugins/connector/hive"). Each key falls back to the ENV_ name below it, which is the fe.conf key + // it used to live under and still works. + public static final String CONF_DEFAULT_FILE_FORMAT = "default_file_format"; + public static final String CONF_ENABLE_CREATE_BUCKET_TABLE = "enable_create_bucket_table"; + // -- environment keys threaded from fe-core DefaultConnectorContext (must stay byte-identical there) -- public static final String ENV_HIVE_DEFAULT_FILE_FORMAT = "hive_default_file_format"; public static final String ENV_ENABLE_CREATE_HIVE_BUCKET_TABLE = "enable_create_hive_bucket_table"; diff --git a/fe/fe-connector/fe-connector-hive/src/main/resources/hms.conf.template b/fe/fe-connector/fe-connector-hive/src/main/resources/hms.conf.template new file mode 100644 index 00000000000000..e4597e0188b662 --- /dev/null +++ b/fe/fe-connector/fe-connector-hive/src/main/resources/hms.conf.template @@ -0,0 +1,21 @@ +# Hive (HMS) connector plugin configuration. +# +# build.sh seeds hms.conf from this file on first deploy and never overwrites it, so an upgrade that +# unzips a new plugin build over this directory keeps whatever you configured here. +# This file must exist on EVERY FE node -- it is not replicated through Doris metadata. +# Changes take effect after an FE restart. +# +# The file name is the connector's name (ConnectorProvider.name() == "hms"), NOT the directory name: +# this plugin is deployed under plugins/connector/hive/ but its settings file is hms.conf. +# +# Every setting below is optional. Left commented out, each falls back to the fe.conf key named +# after it, and then to the built-in default -- so an existing deployment needs no change here. + +# Storage format for a CREATE TABLE that does not name one itself. +# Falls back to fe.conf's hive_default_file_format, whose default is orc. A CREATE TABLE may override +# it per table with the file_format property, which wins over both. +# default_file_format=orc + +# Whether CREATE TABLE may create a bucketed hive table. Off by default. +# Falls back to fe.conf's enable_create_hive_bucket_table. +# enable_create_bucket_table=false diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/FakeConnectorContext.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/FakeConnectorContext.java index 30b7205c4c2268..7c135b2427c50c 100644 --- a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/FakeConnectorContext.java +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/FakeConnectorContext.java @@ -24,9 +24,10 @@ import java.util.Map; /** - * Minimal {@link ConnectorContext} test double: carries a fixed catalog identity and an environment map (the - * channel through which fe-core threads the FE-global CREATE TABLE defaults). Everything else uses the - * interface defaults. + * Minimal {@link ConnectorContext} test double: carries a fixed catalog identity plus the two maps a + * deployment-level setting can arrive in — this plugin's own {@code hms.conf} + * ({@link #getConnectorConfig()}) and fe.conf as forwarded by the engine ({@link #getEnvironment()}). + * Everything else uses the interface defaults. */ public class FakeConnectorContext implements ConnectorContext, ConnectorStorageContext { @@ -40,6 +41,7 @@ public ConnectorStorageContext getStorageContext() { private final String catalogName; private final long catalogId; private final Map environment; + private final Map connectorConfig; public FakeConnectorContext() { this("test_catalog", 0L, Collections.emptyMap()); @@ -50,9 +52,15 @@ public FakeConnectorContext(Map environment) { } public FakeConnectorContext(String catalogName, long catalogId, Map environment) { + this(catalogName, catalogId, environment, Collections.emptyMap()); + } + + public FakeConnectorContext(String catalogName, long catalogId, Map environment, + Map connectorConfig) { this.catalogName = catalogName; this.catalogId = catalogId; this.environment = environment == null ? Collections.emptyMap() : environment; + this.connectorConfig = connectorConfig == null ? Collections.emptyMap() : connectorConfig; } @Override @@ -69,4 +77,9 @@ public long getCatalogId() { public Map getEnvironment() { return environment; } + + @Override + public Map getConnectorConfig() { + return connectorConfig; + } } diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataDdlTest.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataDdlTest.java index 2afd824d91b777..3ff1414048ab1a 100644 --- a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataDdlTest.java +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataDdlTest.java @@ -112,6 +112,67 @@ public void createTableFallsBackToOrcWhenEnvMissing() { Assertions.assertEquals("orc", client.lastCreateTable.getFileFormat()); } + // ==================== createTable: the plugin's own hms.conf ==================== + + @Test + public void createTableFileFormatFromPluginConfBeatsFeConf() { + RecordingHmsClient client = new RecordingHmsClient(); + // WHY: the point of the plugin conf channel. An administrator who sets default_file_format in + // hms.conf must get it even though fe.conf still names the old value -- reverse the precedence + // and migrating a deployment to the new file silently does nothing. + Map conf = Collections.singletonMap( + HiveConnectorProperties.CONF_DEFAULT_FILE_FORMAT, "parquet"); + Map env = Collections.singletonMap( + HiveConnectorProperties.ENV_HIVE_DEFAULT_FILE_FORMAT, "orc"); + + metadataWithConf(client, conf, env).createTable(session(), request().build()); + + Assertions.assertEquals("parquet", client.lastCreateTable.getFileFormat()); + } + + @Test + public void createTableUserFileFormatStillBeatsThePluginConf() { + RecordingHmsClient client = new RecordingHmsClient(); + // WHY: the new channel is deployment-level; it must not outrank a per-catalog CREATE TABLE + // property. Only the two deployment channels reordered relative to each other. + Map conf = Collections.singletonMap( + HiveConnectorProperties.CONF_DEFAULT_FILE_FORMAT, "parquet"); + + metadataWithConf(client, conf, Collections.emptyMap()).createTable(session(), + request().properties(Collections.singletonMap("file_format", "orc")).build()); + + Assertions.assertEquals("orc", client.lastCreateTable.getFileFormat()); + } + + @Test + public void createTableBucketGateCanBeOpenedFromThePluginConfAlone() { + RecordingHmsClient client = new RecordingHmsClient(); + // WHY: the second migrated setting, and the one where getting the channel wrong is expensive -- + // a deployment that opts in through hms.conf but is still gated by fe.conf's default 'false' + // would find bucketed creates rejected with no indication which file is in charge. + Map conf = Collections.singletonMap( + HiveConnectorProperties.CONF_ENABLE_CREATE_BUCKET_TABLE, "true"); + ConnectorBucketSpec bucket = new ConnectorBucketSpec( + Collections.singletonList("id"), 8, "doris_default"); + + metadataWithConf(client, conf, Collections.emptyMap()) + .createTable(session(), request().bucketSpec(bucket).build()); + + Assertions.assertEquals(Collections.singletonList("id"), client.lastCreateTable.getBucketCols()); + Assertions.assertEquals(8, client.lastCreateTable.getNumBuckets()); + } + + @Test + public void theConfTemplateIsNamedAfterTheProvider() { + // WHY: the engine reads .conf, and this connector's name is "hms" while its plugin + // directory is "hive". A template under any other name deploys a file nothing ever opens -- + // silently, with every setting in it ignored. Renaming getType() must break here. + String expected = new HiveConnectorProvider().name() + ".conf.template"; + Assertions.assertNotNull( + HiveConnectorMetadataDdlTest.class.getClassLoader().getResource(expected), + "the plugin must ship " + expected + " on its classpath"); + } + // ==================== createTable: transactional rejection ==================== @Test @@ -368,6 +429,12 @@ private static HiveConnectorMetadata metadata(RecordingHmsClient client, return new HiveConnectorMetadata(client, catalogProps, new FakeConnectorContext(env)); } + private static HiveConnectorMetadata metadataWithConf(RecordingHmsClient client, + Map conf, Map env) { + return new HiveConnectorMetadata(client, Collections.emptyMap(), + new FakeConnectorContext("test_catalog", 0L, env, conf)); + } + /** * Columns are nullable: hive rejects a {@code NOT NULL} column up front (validateColumns runs before every * other createTable check), so a NOT NULL fixture column would short-circuit every test in this class before From f5a3c02d338180517d59e1017b5ff271463e27c6 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:22:45 +0800 Subject: [PATCH 06/13] [feat](jdbc) read the driver settings from the plugin conf Moves jdbc_drivers_dir and force_sqlserver_jdbc_encrypt_false onto the connector's own settings file, as drivers_dir and force_sqlserver_encrypt_false in jdbc.conf. The plugin conf wins; fe.conf stays as the fallback, so a deployment that changes nothing behaves exactly as before and both @ConfFields keep working. The keys lose their jdbc_ prefix because the file name already namespaces them. JdbcUrlNormalizer.normalize now takes the resolved boolean rather than the engine environment map. It had no reason to know an fe.conf key name, and after this change the value can come from either of two files -- deciding which is the connector's job, not the URL normalizer's. doris_home stays in the engine environment: it is the FE install root, not this connector's setting, and the drivers-directory default is still built from it exactly as before. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../src/main/assembly/plugin-zip.xml | 10 +++ .../jdbc/JdbcConnectorProperties.java | 13 +++ .../connector/jdbc/JdbcDorisConnector.java | 21 +++-- .../connector/jdbc/JdbcUrlNormalizer.java | 20 ++--- .../src/main/resources/jdbc.conf.template | 17 ++++ .../connector/jdbc/JdbcUrlNormalizerTest.java | 86 +++++++++++++++++-- 6 files changed, 142 insertions(+), 25 deletions(-) create mode 100644 fe/fe-connector/fe-connector-jdbc/src/main/resources/jdbc.conf.template diff --git a/fe/fe-connector/fe-connector-jdbc/src/main/assembly/plugin-zip.xml b/fe/fe-connector/fe-connector-jdbc/src/main/assembly/plugin-zip.xml index 2e141b72f15096..d0f53bf68d6833 100644 --- a/fe/fe-connector/fe-connector-jdbc/src/main/assembly/plugin-zip.xml +++ b/fe/fe-connector/fe-connector-jdbc/src/main/assembly/plugin-zip.xml @@ -40,6 +40,16 @@ under the License. ${project.build.directory}/${project.build.finalName}.jar / + + + src/main/resources/jdbc.conf.template + / + diff --git a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProperties.java b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProperties.java index f3a3e063133453..6641eec5ed1c0b 100644 --- a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProperties.java +++ b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProperties.java @@ -30,6 +30,19 @@ private JdbcConnectorProperties() { } // -- connection -- + // -- deployment-level settings, read from this plugin's own jdbc.conf (named after + // ConnectorProvider.name()). Each falls back to the ENV_ name below it, which is the fe.conf key it + // used to live under and still works. -- + public static final String CONF_DRIVERS_DIR = "drivers_dir"; + public static final String CONF_FORCE_SQLSERVER_ENCRYPT_FALSE = "force_sqlserver_encrypt_false"; + + /** The fe.conf name of {@link #CONF_DRIVERS_DIR}, forwarded through the engine environment. */ + public static final String ENV_DRIVERS_DIR = "jdbc_drivers_dir"; + /** The fe.conf name of {@link #CONF_FORCE_SQLSERVER_ENCRYPT_FALSE}. */ + public static final String ENV_FORCE_SQLSERVER_ENCRYPT_FALSE = "force_sqlserver_jdbc_encrypt_false"; + /** Engine-wide, not this connector's: the FE install root. Stays in the engine environment. */ + public static final String ENV_DORIS_HOME = "doris_home"; + public static final String JDBC_URL = "jdbc_url"; public static final String USER = "user"; public static final String PASSWORD = "password"; diff --git a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java index 885f19a7a046c0..01e1b94df8a6d4 100644 --- a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java +++ b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java @@ -27,6 +27,7 @@ import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; import org.apache.doris.connector.api.write.ConnectorWritePlanProvider; import org.apache.doris.connector.jdbc.client.JdbcConnectorClient; +import org.apache.doris.connector.spi.ConnectorConf; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.thrift.TJdbcTable; import org.apache.doris.thrift.TOdbcTableType; @@ -86,7 +87,11 @@ public JdbcDorisConnector(Map properties, ConnectorContext conte if (rawUrl != null && !rawUrl.isEmpty()) { JdbcDbType dbType = JdbcDbType.parseFromUrl(rawUrl); normalized.put(JdbcConnectorProperties.JDBC_URL, - JdbcUrlNormalizer.normalize(rawUrl, dbType, context.getEnvironment())); + JdbcUrlNormalizer.normalize(rawUrl, dbType, + Boolean.parseBoolean(ConnectorConf.get(context, + JdbcConnectorProperties.CONF_FORCE_SQLSERVER_ENCRYPT_FALSE, + JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, + "false")))); } this.properties = Collections.unmodifiableMap(normalized); this.context = context; @@ -319,9 +324,10 @@ public void close() throws IOException { } /** - * Resolves driver URL using the environment from ConnectorContext. + * Resolves driver URL against the configured drivers directory. * If the URL is a plain filename (e.g., "mysql-connector-j-8.4.0.jar"), - * resolves it using the jdbc_drivers_dir from the environment. + * resolves it under {@code drivers_dir} from this plugin's jdbc.conf, or fe.conf's + * {@code jdbc_drivers_dir}. */ private String resolveDriverUrl(String driverUrl) { if (driverUrl == null || driverUrl.isEmpty()) { @@ -331,10 +337,11 @@ private String resolveDriverUrl(String driverUrl) { || driverUrl.startsWith("https://") || driverUrl.startsWith("/")) { return driverUrl; } - // Plain filename — resolve using jdbc_drivers_dir from environment - Map env = context.getEnvironment(); - String driversDir = env.get("jdbc_drivers_dir"); - String dorisHome = env.get("doris_home"); + // Plain filename — resolve under the configured drivers directory. doris_home is engine-wide + // rather than this connector's setting, so it keeps coming from the engine environment. + String driversDir = ConnectorConf.get(context, JdbcConnectorProperties.CONF_DRIVERS_DIR, + JdbcConnectorProperties.ENV_DRIVERS_DIR, null); + String dorisHome = context.getEnvironment().get(JdbcConnectorProperties.ENV_DORIS_HOME); if (driversDir != null && !driversDir.isEmpty()) { String newPath = driversDir + "/" + driverUrl; if (new File(newPath).exists()) { diff --git a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizer.java b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizer.java index 50987f308d2e1e..b5eea3587495a0 100644 --- a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizer.java +++ b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizer.java @@ -17,9 +17,6 @@ package org.apache.doris.connector.jdbc; -import java.util.Collections; -import java.util.Map; - /** * Normalizes JDBC URLs by adding required parameters for correct behavior. * Replicates the logic from {@code JdbcResource.handleJdbcUrl()} in fe-core, @@ -55,21 +52,25 @@ private JdbcUrlNormalizer() { *

For SQL Server: *

    *
  • {@code useBulkCopyForBatchInsert=true}
    • - *
  • {@code encrypt=false} — when {@code force_sqlserver_jdbc_encrypt_false} is set
    • + *
  • {@code encrypt=false} — when the deployment asks for it
    • *
*/ public static String normalize(String jdbcUrl, JdbcDbType dbType) { - return normalize(jdbcUrl, dbType, Collections.emptyMap()); + return normalize(jdbcUrl, dbType, false); } /** - * Normalize a JDBC URL with engine environment context. + * Normalize a JDBC URL, honoring the deployment-level SQL Server encryption override. * * @param jdbcUrl the raw JDBC URL * @param dbType the database type - * @param environment engine environment properties (from ConnectorContext) + * @param forceSqlServerEncryptFalse whether to pin {@code encrypt=false} on a SQL Server URL. + * Resolved by the caller from {@code force_sqlserver_encrypt_false} + * in the plugin's own jdbc.conf or + * {@code force_sqlserver_jdbc_encrypt_false} in fe.conf */ - public static String normalize(String jdbcUrl, JdbcDbType dbType, Map environment) { + public static String normalize(String jdbcUrl, JdbcDbType dbType, + boolean forceSqlServerEncryptFalse) { if (jdbcUrl == null || jdbcUrl.isEmpty()) { return jdbcUrl; } @@ -95,8 +96,7 @@ public static String normalize(String jdbcUrl, JdbcDbType dbType, Map/plugins/jdbc_drivers. +# drivers_dir=/opt/doris/plugins/jdbc_drivers + +# Pin encrypt=false on SQL Server JDBC URLs. Off by default. +# Falls back to fe.conf's force_sqlserver_jdbc_encrypt_false. +# force_sqlserver_encrypt_false=false diff --git a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizerTest.java b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizerTest.java index c19dc3ba941f8a..a525ad1c88bf90 100644 --- a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizerTest.java +++ b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcUrlNormalizerTest.java @@ -17,12 +17,17 @@ package org.apache.doris.connector.jdbc; +import org.apache.doris.connector.spi.ConnectorConf; +import org.apache.doris.connector.spi.ConnectorContext; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.Map; + /** * Tests for {@link JdbcUrlNormalizer}, focusing on the setParamIfAbsent - * duplicate-append fix (P1-7). + * duplicate-append fix (P1-7), plus how this connector's two deployment-level settings are resolved. */ public class JdbcUrlNormalizerTest { @@ -118,20 +123,16 @@ void testUnknownDbTypeReturnsUrlUnchanged() { @Test void testSqlServerEncryptOverrideWhenForced() { - java.util.Map env = java.util.Map.of( - "force_sqlserver_jdbc_encrypt_false", "true"); String url = "jdbc:sqlserver://host:1433;databaseName=test"; - String result = JdbcUrlNormalizer.normalize(url, JdbcDbType.SQLSERVER, env); + String result = JdbcUrlNormalizer.normalize(url, JdbcDbType.SQLSERVER, true); Assertions.assertTrue(result.contains(";encrypt=false"), - "encrypt=false should be added when force_sqlserver_jdbc_encrypt_false is true; got: " + result); + "encrypt=false should be added when the override is on; got: " + result); } @Test void testSqlServerEncryptOverrideReplacesTrue() { - java.util.Map env = java.util.Map.of( - "force_sqlserver_jdbc_encrypt_false", "true"); String url = "jdbc:sqlserver://host:1433;encrypt=true;databaseName=test"; - String result = JdbcUrlNormalizer.normalize(url, JdbcDbType.SQLSERVER, env); + String result = JdbcUrlNormalizer.normalize(url, JdbcDbType.SQLSERVER, true); Assertions.assertTrue(result.contains("encrypt=false"), "encrypt=true should be replaced with encrypt=false; got: " + result); Assertions.assertFalse(result.contains("encrypt=true"), @@ -146,6 +147,75 @@ void testSqlServerEncryptNotOverriddenByDefault() { "encrypt=false should NOT be added without force flag; got: " + result); } + @Test + void theEncryptOverrideIsReadFromThePluginConfFirstThenFeConf() { + // The resolution the connector performs before calling normalize. Asserted here rather than + // trusted: an administrator who turns the override on in jdbc.conf must not be silently + // overruled by fe.conf's default, and vice versa an untouched deployment must keep reading + // fe.conf exactly as before. + Assertions.assertEquals("true", ConnectorConf.get( + context(Map.of(JdbcConnectorProperties.CONF_FORCE_SQLSERVER_ENCRYPT_FALSE, "true"), + Map.of(JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, "false")), + JdbcConnectorProperties.CONF_FORCE_SQLSERVER_ENCRYPT_FALSE, + JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, "false")); + + Assertions.assertEquals("true", ConnectorConf.get( + context(Map.of(), Map.of(JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, "true")), + JdbcConnectorProperties.CONF_FORCE_SQLSERVER_ENCRYPT_FALSE, + JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, "false")); + + Assertions.assertEquals("false", ConnectorConf.get(context(Map.of(), Map.of()), + JdbcConnectorProperties.CONF_FORCE_SQLSERVER_ENCRYPT_FALSE, + JdbcConnectorProperties.ENV_FORCE_SQLSERVER_ENCRYPT_FALSE, "false")); + } + + @Test + void theDriversDirIsReadFromThePluginConfFirstThenFeConf() { + Assertions.assertEquals("/from/plugin/conf", ConnectorConf.get( + context(Map.of(JdbcConnectorProperties.CONF_DRIVERS_DIR, "/from/plugin/conf"), + Map.of(JdbcConnectorProperties.ENV_DRIVERS_DIR, "/from/fe/conf")), + JdbcConnectorProperties.CONF_DRIVERS_DIR, + JdbcConnectorProperties.ENV_DRIVERS_DIR, null)); + + Assertions.assertEquals("/from/fe/conf", ConnectorConf.get( + context(Map.of(), Map.of(JdbcConnectorProperties.ENV_DRIVERS_DIR, "/from/fe/conf")), + JdbcConnectorProperties.CONF_DRIVERS_DIR, + JdbcConnectorProperties.ENV_DRIVERS_DIR, null)); + } + + @Test + void theConfTemplateIsNamedAfterTheProvider() { + // The engine reads .conf, so a template under any other name deploys a file nothing ever + // opens -- silently, with every setting in it ignored. Renaming getType() must break here. + String expected = new JdbcConnectorProvider().name() + ".conf.template"; + Assertions.assertNotNull(getClass().getClassLoader().getResource(expected), + "the plugin must ship " + expected + " on its classpath"); + } + + private static ConnectorContext context(Map conf, Map env) { + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + + @Override + public Map getConnectorConfig() { + return conf; + } + + @Override + public Map getEnvironment() { + return env; + } + }; + } + private static int countOccurrences(String str, String sub) { int count = 0; int idx = 0; From 435db8ed3b8a17e2fc79f1f3e99c37440059ab77 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:24:58 +0800 Subject: [PATCH 07/13] [feat](iceberg,paimon) read the shared settings from their own plugin conf Moves jdbc_drivers_dir and hive_metastore_client_timeout_second onto these two connectors' own settings files, as drivers_dir and metastore_client_timeout_second in iceberg.conf and paimon.conf. Each plugin conf wins; fe.conf stays as the fallback, so a deployment that changes nothing behaves exactly as before and both @ConfFields keep working. Both settings are shared at the fe.conf end -- one jdbc_drivers_dir and one hive_metastore_client_timeout_second serve jdbc, iceberg and paimon. A per-plugin file cannot express that, so a deployment moving to these files sets the value in each plugin's conf. That is the accepted cost of the per-plugin model and is called out in both templates; leaving them commented out keeps the single shared fe.conf value, which is what every existing deployment gets. JdbcDriverSupport.resolveDriverUrl now takes the drivers directory and DORIS_HOME rather than the engine environment map. It lives in a module shared by connectors whose conf files differ, so which file a value comes from cannot be its decision -- the same shape AbstractHmsMetaStoreProperties already has, taking its timeout default as a parameter instead of reading the environment. The resolution itself is unchanged, including the /plugins/jdbc_drivers fallback. Paimon resolves the drivers directory through one accessor shared by both its call sites (FE driver registration and the BE-bound scan options), for the same reason both already delegate to JdbcDriverSupport: the two must resolve a given driver_url identically or FE and BE load different jars. PaimonConnectorProperties' javadoc claim of being a pure constant holder is amended -- HiveConnectorProperties and JdbcConnectorProperties have carried static accessors for a while. This has to be one commit: the signature change and its three call sites span two plugin modules, so splitting it leaves an intermediate commit that does not compile. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../src/main/assembly/plugin-zip.xml | 14 ++ .../connector/iceberg/IcebergConnector.java | 30 ++++- .../iceberg/IcebergConnectorProperties.java | 21 +++ .../src/main/resources/iceberg.conf.template | 23 ++++ .../iceberg/IcebergConnectorConfTest.java | 124 +++++++++++++++++ .../PaimonJdbcMetaStorePropertiesTest.java | 27 ++-- .../metastore/spi/JdbcDriverSupport.java | 34 ++--- .../src/main/assembly/plugin-zip.xml | 14 ++ .../connector/paimon/PaimonConnector.java | 13 +- .../paimon/PaimonConnectorProperties.java | 52 ++++++- .../paimon/PaimonScanPlanProvider.java | 5 +- .../src/main/resources/paimon.conf.template | 25 ++++ .../paimon/PaimonConnectorConfTest.java | 127 ++++++++++++++++++ 13 files changed, 469 insertions(+), 40 deletions(-) create mode 100644 fe/fe-connector/fe-connector-iceberg/src/main/resources/iceberg.conf.template create mode 100644 fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorConfTest.java create mode 100644 fe/fe-connector/fe-connector-paimon/src/main/resources/paimon.conf.template create mode 100644 fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorConfTest.java diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/assembly/plugin-zip.xml b/fe/fe-connector/fe-connector-iceberg/src/main/assembly/plugin-zip.xml index 9c46915cc7bf65..c8dd917dc9361c 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/assembly/plugin-zip.xml +++ b/fe/fe-connector/fe-connector-iceberg/src/main/assembly/plugin-zip.xml @@ -36,6 +36,20 @@ under the License. + + + + src/main/resources/iceberg.conf.template + / + + + lib diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java index cd1ffb5f9b87bf..738cf7bd1bf7bb 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java @@ -34,6 +34,7 @@ import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; +import org.apache.doris.connector.spi.ConnectorConf; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.connector.spi.ConnectorStorageContext; import org.apache.doris.filesystem.properties.S3CompatibleFileSystemProperties; @@ -922,8 +923,10 @@ private Catalog createCatalog() { IcebergConnectorProperties.TYPE_HMS, properties, storageHadoopConfig); conf = IcebergCatalogFactory.assembleHiveConf( IcebergCatalogFactory.firstNonBlank(properties, "hive.conf.resources"), - hms.toHiveConfOverrides(context.getEnvironment() - .getOrDefault("hive_metastore_client_timeout_second", "10"))); + hms.toHiveConfOverrides(ConnectorConf.get(context, + IcebergConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND))); break; } case IcebergConnectorProperties.TYPE_GLUE: @@ -1407,13 +1410,32 @@ private void maybeRegisterJdbcDriver() { LOG.info("Using dynamic JDBC driver for Iceberg JDBC catalog from: {}", driverUrl); } + /** + * The directory a bare driver jar name resolves under, from this plugin's own {@code iceberg.conf} + * or fe.conf's {@code jdbc_drivers_dir}. Null when neither names one — {@code JdbcDriverSupport} + * then falls back to {@code /plugins/jdbc_drivers}, as before. + * + *

A null context is a direct-construction unit test, which has neither channel. + */ + private String configuredDriversDir() { + return context == null ? null : ConnectorConf.get(context, + IcebergConnectorProperties.CONF_DRIVERS_DIR, + IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, null); + } + + /** The FE install root. Engine-wide rather than this connector's, so it stays in the environment. */ + private String configuredDorisHome() { + return context == null ? null + : context.getEnvironment().get(IcebergConnectorProperties.ENV_DORIS_HOME); + } + private void registerJdbcDriver(String driverUrl, String driverClassName) { try { if (StringUtils.isBlank(driverClassName)) { throw new IllegalArgumentException("driver_class is required when driver_url is specified"); } - Map env = context != null ? context.getEnvironment() : Collections.emptyMap(); - String fullDriverUrl = JdbcDriverSupport.resolveDriverUrl(driverUrl, env); + String fullDriverUrl = JdbcDriverSupport.resolveDriverUrl(driverUrl, + configuredDriversDir(), configuredDorisHome()); URL url = new URL(fullDriverUrl); String driverKey = fullDriverUrl + "#" + driverClassName; if (!REGISTERED_DRIVER_KEYS.add(driverKey)) { diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorProperties.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorProperties.java index f2febaf2e432ba..ac8289cb022256 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorProperties.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorProperties.java @@ -27,6 +27,27 @@ public final class IcebergConnectorProperties { private IcebergConnectorProperties() { } + // -- Deployment-level settings, read from this plugin's own iceberg.conf (named after + // ConnectorProvider.name()). Each falls back to the ENV_ name below it, which is the fe.conf key it + // used to live under and still works. + // + // Both are shared with other connectors at the fe.conf end -- one jdbc_drivers_dir and one + // hive_metastore_client_timeout_second serve jdbc, iceberg and paimon. A plugin conf cannot express + // that, so a deployment that moves to these files sets the value in each plugin's own conf. That is + // the accepted cost of a per-plugin file; the fe.conf keys stay as the shared fallback. -- + public static final String CONF_DRIVERS_DIR = "drivers_dir"; + public static final String CONF_METASTORE_CLIENT_TIMEOUT_SECOND = "metastore_client_timeout_second"; + + /** The fe.conf name of {@link #CONF_DRIVERS_DIR}, forwarded through the engine environment. */ + public static final String ENV_JDBC_DRIVERS_DIR = "jdbc_drivers_dir"; + /** The fe.conf name of {@link #CONF_METASTORE_CLIENT_TIMEOUT_SECOND}. */ + public static final String ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND = + "hive_metastore_client_timeout_second"; + /** Engine-wide, not this connector's: the FE install root. Stays in the engine environment. */ + public static final String ENV_DORIS_HOME = "doris_home"; + /** Legacy default when neither channel names a metastore client timeout. */ + public static final String DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND = "10"; + // -- Catalog type (second-level dispatch) -- public static final String ICEBERG_CATALOG_TYPE = "iceberg.catalog.type"; diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/resources/iceberg.conf.template b/fe/fe-connector/fe-connector-iceberg/src/main/resources/iceberg.conf.template new file mode 100644 index 00000000000000..668c9328ac3271 --- /dev/null +++ b/fe/fe-connector/fe-connector-iceberg/src/main/resources/iceberg.conf.template @@ -0,0 +1,23 @@ +# Iceberg connector plugin configuration. +# +# build.sh seeds iceberg.conf from this file on first deploy and never overwrites it, so an upgrade +# that unzips a new plugin build over this directory keeps whatever you configured here. +# This file must exist on EVERY FE node -- it is not replicated through Doris metadata. +# Changes take effect after an FE restart. +# +# Every setting below is optional. Left commented out, each falls back to the fe.conf key named +# after it, and then to the built-in default -- so an existing deployment needs no change here. +# +# NOTE: both settings below are shared with other connectors at the fe.conf end -- one +# jdbc_drivers_dir and one hive_metastore_client_timeout_second serve jdbc, iceberg and paimon. +# A per-plugin conf cannot express that, so if you move them here you must also set them in +# jdbc.conf and paimon.conf. Leaving them commented out keeps the single shared fe.conf value. + +# Directory a bare driver jar name in iceberg.jdbc.driver_url resolves under (JDBC catalog only). +# Falls back to fe.conf's jdbc_drivers_dir, whose default is /plugins/jdbc_drivers. +# drivers_dir=/opt/doris/plugins/jdbc_drivers + +# Socket timeout, in seconds, for this catalog's Hive metastore client (HMS catalog only), used +# unless the catalog overrides it. Falls back to fe.conf's hive_metastore_client_timeout_second, +# whose default is 10. +# metastore_client_timeout_second=10 diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorConfTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorConfTest.java new file mode 100644 index 00000000000000..4fd6e69b213e76 --- /dev/null +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorConfTest.java @@ -0,0 +1,124 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.iceberg; + +import org.apache.doris.connector.spi.ConnectorConf; +import org.apache.doris.connector.spi.ConnectorContext; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * How this connector's two deployment-level settings are resolved: its own {@code iceberg.conf} + * first, then the fe.conf key each used to live under. + * + *

Asserted rather than trusted, because both failure directions are silent. Reading fe.conf first + * would make an administrator's edit to iceberg.conf do nothing. Dropping the fe.conf fallback would + * change an untouched deployment's metastore timeout and drivers directory on upgrade, with nothing + * in either file to show why. + */ +public class IcebergConnectorConfTest { + + @Test + public void driversDirPrefersThePluginConfThenFeConf() { + Assertions.assertEquals("/from/plugin/conf", ConnectorConf.get( + context(Collections.singletonMap(IcebergConnectorProperties.CONF_DRIVERS_DIR, + "/from/plugin/conf"), + Collections.singletonMap(IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, + "/from/fe/conf")), + IcebergConnectorProperties.CONF_DRIVERS_DIR, + IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, null)); + + Assertions.assertEquals("/from/fe/conf", ConnectorConf.get( + context(Collections.emptyMap(), + Collections.singletonMap(IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, + "/from/fe/conf")), + IcebergConnectorProperties.CONF_DRIVERS_DIR, + IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, null)); + + Assertions.assertNull(ConnectorConf.get(context(Collections.emptyMap(), Collections.emptyMap()), + IcebergConnectorProperties.CONF_DRIVERS_DIR, + IcebergConnectorProperties.ENV_JDBC_DRIVERS_DIR, null)); + } + + @Test + public void metastoreTimeoutPrefersThePluginConfThenFeConfThenTen() { + Assertions.assertEquals("30", ConnectorConf.get( + context(Collections.singletonMap( + IcebergConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, "30"), + Collections.singletonMap( + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, "20")), + IcebergConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + + Assertions.assertEquals("20", ConnectorConf.get( + context(Collections.emptyMap(), Collections.singletonMap( + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, "20")), + IcebergConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + + // The literal the call site used before this channel existed; keeping it is what makes a + // deployment with neither file behave exactly as it did. + Assertions.assertEquals("10", ConnectorConf.get( + context(Collections.emptyMap(), Collections.emptyMap()), + IcebergConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + IcebergConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + } + + @Test + public void theConfTemplateIsNamedAfterTheProvider() { + // The engine reads .conf, so a template under any other name deploys a file nothing ever + // opens -- silently, with every setting in it ignored. Renaming getType() must break here. + String expected = new IcebergConnectorProvider().name() + ".conf.template"; + Assertions.assertNotNull(getClass().getClassLoader().getResource(expected), + "the plugin must ship " + expected + " on its classpath"); + } + + private static ConnectorContext context(Map conf, Map env) { + Map confCopy = new HashMap<>(conf); + Map envCopy = new HashMap<>(env); + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + + @Override + public Map getConnectorConfig() { + return confCopy; + } + + @Override + public Map getEnvironment() { + return envCopy; + } + }; + } +} diff --git a/fe/fe-connector/fe-connector-metastore-paimon/src/test/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStorePropertiesTest.java b/fe/fe-connector/fe-connector-metastore-paimon/src/test/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStorePropertiesTest.java index cc6842020b4e4a..9916654c9b6139 100644 --- a/fe/fe-connector/fe-connector-metastore-paimon/src/test/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStorePropertiesTest.java +++ b/fe/fe-connector/fe-connector-metastore-paimon/src/test/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStorePropertiesTest.java @@ -74,21 +74,28 @@ public void validateChecksWarehouseThenUriThenDriverClass() { @Test public void resolveDriverUrl() { - Map env = new HashMap<>(); + // The drivers directory is now passed in rather than read from the engine environment here: + // which settings file it comes from is the calling connector's business, and this module is + // shared by connectors whose conf files differ. The resolution itself is unchanged. // already scheme-bearing -> as-is - Assertions.assertEquals("https://host/d.jar", JdbcDriverSupport.resolveDriverUrl("https://host/d.jar", env)); + Assertions.assertEquals("https://host/d.jar", + JdbcDriverSupport.resolveDriverUrl("https://host/d.jar", null, null)); // absolute path -> as-is (no driversDir prepend) - Assertions.assertEquals("/opt/drivers/d.jar", JdbcDriverSupport.resolveDriverUrl("/opt/drivers/d.jar", env)); + Assertions.assertEquals("/opt/drivers/d.jar", + JdbcDriverSupport.resolveDriverUrl("/opt/drivers/d.jar", null, null)); // bare jar with explicit drivers dir - env.put("jdbc_drivers_dir", "/custom/drivers"); - Assertions.assertEquals("file:///custom/drivers/d.jar", JdbcDriverSupport.resolveDriverUrl("d.jar", env)); + Assertions.assertEquals("file:///custom/drivers/d.jar", + JdbcDriverSupport.resolveDriverUrl("d.jar", "/custom/drivers", "/dh")); // bare jar falling back to doris_home/plugins/jdbc_drivers - Map env2 = new HashMap<>(); - env2.put("doris_home", "/dh"); - Assertions.assertEquals("file:///dh/plugins/jdbc_drivers/d.jar", JdbcDriverSupport.resolveDriverUrl("d.jar", env2)); - // empty env -> doris_home defaults to "." + Assertions.assertEquals("file:///dh/plugins/jdbc_drivers/d.jar", + JdbcDriverSupport.resolveDriverUrl("d.jar", null, "/dh")); + // a blank drivers dir falls back the same way as an absent one -- 'drivers_dir=' in a conf + // file means "not configured", not "resolve under the empty path". + Assertions.assertEquals("file:///dh/plugins/jdbc_drivers/d.jar", + JdbcDriverSupport.resolveDriverUrl("d.jar", " ", "/dh")); + // neither known -> doris_home defaults to "." Assertions.assertEquals("file://./plugins/jdbc_drivers/d.jar", - JdbcDriverSupport.resolveDriverUrl("d.jar", new HashMap<>())); + JdbcDriverSupport.resolveDriverUrl("d.jar", null, null)); } @Test diff --git a/fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/JdbcDriverSupport.java b/fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/JdbcDriverSupport.java index 176763295a3d3e..f1703299f3eb2a 100644 --- a/fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/JdbcDriverSupport.java +++ b/fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/JdbcDriverSupport.java @@ -19,8 +19,6 @@ import org.apache.commons.lang3.StringUtils; -import java.util.Map; - /** * Shared JDBC driver-url resolution. Only the PURE resolver lives here (a function of the raw * {@code driver_url} + the engine environment map). The live driver REGISTRATION @@ -36,16 +34,23 @@ private JdbcDriverSupport() { /** * Resolves a JDBC {@code driver_url} to a full, scheme-bearing URL string. A value already * carrying a scheme ({@code "://"}) is used as-is; an absolute path (starting with {@code "/"}) is - * returned unchanged; otherwise it is treated as a bare jar file name and resolved against the - * engine's configured {@code jdbc_drivers_dir} (defaulting to - * {@code $DORIS_HOME/plugins/jdbc_drivers}). Mirrors the minimal {@code JdbcResource.getFullDriverUrl} - * resolution (no file-existence / legacy old-dir / cloud-download handling), so the FE driver - * registration and the BE-bound options resolve a given {@code driver_url} identically. + * returned unchanged; otherwise it is treated as a bare jar file name and resolved against + * {@code driversDir} (defaulting to {@code $DORIS_HOME/plugins/jdbc_drivers}). Mirrors the minimal + * {@code JdbcResource.getFullDriverUrl} resolution (no file-existence / legacy old-dir / + * cloud-download handling), so the FE driver registration and the BE-bound options resolve a given + * {@code driver_url} identically. + * + *

Both directories are passed in rather than read from the engine environment here: which + * settings file a drivers directory comes from is the calling connector's business (its own + * {@code .conf} first, then fe.conf's {@code jdbc_drivers_dir}), and this module is shared by + * connectors whose conf files differ. Same shape as + * {@code AbstractHmsMetaStoreProperties}, which likewise takes its default as a parameter. * - * @param driverUrl the raw driver_url; must be non-null and non-blank (the caller's responsibility) - * @param env the engine environment map (e.g. {@code jdbc_drivers_dir}, {@code doris_home}); never null + * @param driverUrl the raw driver_url; must be non-null and non-blank (the caller's responsibility) + * @param driversDir directory a bare jar name resolves under; blank falls back to the default below + * @param dorisHome the FE install root, used only to build that default; blank means "." */ - public static String resolveDriverUrl(String driverUrl, Map env) { + public static String resolveDriverUrl(String driverUrl, String driversDir, String dorisHome) { if (driverUrl.contains("://")) { return driverUrl; } @@ -53,11 +58,10 @@ public static String resolveDriverUrl(String driverUrl, Map env) // Absolute path, no scheme: legacy returns it as-is (no driversDir prepend). return driverUrl; } - String driversDir = env.get("jdbc_drivers_dir"); - if (StringUtils.isBlank(driversDir)) { - String dorisHome = env.getOrDefault("doris_home", "."); - driversDir = dorisHome + "/plugins/jdbc_drivers"; + String resolvedDir = driversDir; + if (StringUtils.isBlank(resolvedDir)) { + resolvedDir = (StringUtils.isBlank(dorisHome) ? "." : dorisHome) + "/plugins/jdbc_drivers"; } - return "file://" + driversDir + "/" + driverUrl; + return "file://" + resolvedDir + "/" + driverUrl; } } diff --git a/fe/fe-connector/fe-connector-paimon/src/main/assembly/plugin-zip.xml b/fe/fe-connector/fe-connector-paimon/src/main/assembly/plugin-zip.xml index 9deeb4c7889d5f..1f963509839ba9 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/assembly/plugin-zip.xml +++ b/fe/fe-connector/fe-connector-paimon/src/main/assembly/plugin-zip.xml @@ -36,6 +36,20 @@ under the License. + + + + src/main/resources/paimon.conf.template + / + + + lib diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java index dc660eaff824ee..670ee5b006cdf0 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java @@ -28,6 +28,7 @@ import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; +import org.apache.doris.connector.spi.ConnectorConf; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.connector.spi.ConnectorStorageContext; import org.apache.doris.filesystem.properties.StorageProperties; @@ -50,7 +51,6 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; -import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -422,8 +422,10 @@ private Catalog createCatalog() { MetaStoreProviders.bind(properties, storageHadoopConfig); HiveConf hc = PaimonCatalogFactory.assembleHiveConf( PaimonCatalogFactory.firstNonBlank(properties, "hive.conf.resources"), - hms.toHiveConfOverrides(context.getEnvironment() - .getOrDefault("hive_metastore_client_timeout_second", "10"))); + hms.toHiveConfOverrides(ConnectorConf.get(context, + PaimonConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND))); return createCatalogFromContext(CatalogContext.create(options, hc), flavor, "Failed to create Paimon catalog with HMS metastore"); } @@ -532,8 +534,9 @@ private void maybeRegisterJdbcDriver() { * allow-list (a pre-existing fe-core gap shared by all plugin connectors — see deviations-log). */ private String resolveFullDriverUrl(String driverUrl) { - Map env = context != null ? context.getEnvironment() : Collections.emptyMap(); - return JdbcDriverSupport.resolveDriverUrl(driverUrl, env); + return JdbcDriverSupport.resolveDriverUrl(driverUrl, + PaimonConnectorProperties.configuredDriversDir(context), + PaimonConnectorProperties.configuredDorisHome(context)); } private void registerJdbcDriver(String driverUrl, String driverClassName) { diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorProperties.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorProperties.java index ae3fe94a25c77d..e8ac1b4257b502 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorProperties.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorProperties.java @@ -17,11 +17,13 @@ package org.apache.doris.connector.paimon; +import org.apache.doris.connector.spi.ConnectorConf; +import org.apache.doris.connector.spi.ConnectorContext; + /** - * Property key constants for Paimon connector configuration. - * - *

Pure static-constant holder (no logic), mirroring the role of - * {@code MCConnectorProperties}. Where a Doris-facing property accepts multiple + * Property key constants for Paimon connector configuration, plus the accessors that read this + * connector's deployment-level settings (same shape as {@code HiveConnectorProperties.getInt} / + * {@code JdbcConnectorProperties.getInt}). Where a Doris-facing property accepts multiple * aliases (matching the legacy fe-core {@code @ConnectorProperty(names = {...})} * declarations), the aliases are exposed as a {@code String[]} in alias-priority * order so {@link PaimonCatalogFactory} can resolve them with @@ -29,6 +31,48 @@ */ public final class PaimonConnectorProperties { + // -- Deployment-level settings, read from this plugin's own paimon.conf (named after + // ConnectorProvider.name()). Each falls back to the ENV_ name below it, which is the fe.conf key it + // used to live under and still works. + // + // Both are shared with other connectors at the fe.conf end -- one jdbc_drivers_dir and one + // hive_metastore_client_timeout_second serve jdbc, iceberg and paimon. A plugin conf cannot express + // that, so a deployment that moves to these files sets the value in each plugin's own conf. That is + // the accepted cost of a per-plugin file; the fe.conf keys stay as the shared fallback. -- + public static final String CONF_DRIVERS_DIR = "drivers_dir"; + public static final String CONF_METASTORE_CLIENT_TIMEOUT_SECOND = "metastore_client_timeout_second"; + + /** The fe.conf name of {@link #CONF_DRIVERS_DIR}, forwarded through the engine environment. */ + public static final String ENV_JDBC_DRIVERS_DIR = "jdbc_drivers_dir"; + /** The fe.conf name of {@link #CONF_METASTORE_CLIENT_TIMEOUT_SECOND}. */ + public static final String ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND = + "hive_metastore_client_timeout_second"; + /** Engine-wide, not this connector's: the FE install root. Stays in the engine environment. */ + public static final String ENV_DORIS_HOME = "doris_home"; + /** Legacy default when neither channel names a metastore client timeout. */ + public static final String DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND = "10"; + + /** + * The directory a bare driver jar name resolves under, from this plugin's own {@code paimon.conf} + * or fe.conf's {@code jdbc_drivers_dir}. Null when neither names one — {@code JdbcDriverSupport} + * then falls back to {@code /plugins/jdbc_drivers}, as before. + * + *

Shared by the two call sites (FE driver registration in {@code PaimonConnector} and the + * BE-bound scan options in {@code PaimonScanPlanProvider}) so they cannot resolve a given + * {@code driver_url} differently — which is the same reason both delegate to + * {@code JdbcDriverSupport}. A null context is a direct-construction unit test, which has neither + * channel. + */ + public static String configuredDriversDir(ConnectorContext context) { + return context == null ? null + : ConnectorConf.get(context, CONF_DRIVERS_DIR, ENV_JDBC_DRIVERS_DIR, null); + } + + /** The FE install root. Engine-wide rather than this connector's, so it stays in the environment. */ + public static String configuredDorisHome(ConnectorContext context) { + return context == null ? null : context.getEnvironment().get(ENV_DORIS_HOME); + } + /** Paimon catalog backend type: filesystem, hms, rest, jdbc. */ public static final String PAIMON_CATALOG_TYPE = "paimon.catalog.type"; diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java index e24c70723ebe2f..c0a46f78a42290 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java @@ -1642,8 +1642,9 @@ Map getBackendPaimonOptions() { String driverUrl = PaimonCatalogFactory.firstNonBlank( properties, PaimonConnectorProperties.JDBC_DRIVER_URL); if (driverUrl != null) { - Map env = context != null ? context.getEnvironment() : Collections.emptyMap(); - options.put("jdbc.driver_url", JdbcDriverSupport.resolveDriverUrl(driverUrl, env)); + options.put("jdbc.driver_url", JdbcDriverSupport.resolveDriverUrl(driverUrl, + PaimonConnectorProperties.configuredDriversDir(context), + PaimonConnectorProperties.configuredDorisHome(context))); String driverClass = PaimonCatalogFactory.firstNonBlank( properties, PaimonConnectorProperties.JDBC_DRIVER_CLASS); if (driverClass != null) { diff --git a/fe/fe-connector/fe-connector-paimon/src/main/resources/paimon.conf.template b/fe/fe-connector/fe-connector-paimon/src/main/resources/paimon.conf.template new file mode 100644 index 00000000000000..f71c1e28fdc6b1 --- /dev/null +++ b/fe/fe-connector/fe-connector-paimon/src/main/resources/paimon.conf.template @@ -0,0 +1,25 @@ +# Paimon connector plugin configuration. +# +# build.sh seeds paimon.conf from this file on first deploy and never overwrites it, so an upgrade +# that unzips a new plugin build over this directory keeps whatever you configured here. +# This file must exist on EVERY FE node -- it is not replicated through Doris metadata. +# Changes take effect after an FE restart. +# +# Every setting below is optional. Left commented out, each falls back to the fe.conf key named +# after it, and then to the built-in default -- so an existing deployment needs no change here. +# +# NOTE: both settings below are shared with other connectors at the fe.conf end -- one +# jdbc_drivers_dir and one hive_metastore_client_timeout_second serve jdbc, iceberg and paimon. +# A per-plugin conf cannot express that, so if you move them here you must also set them in +# jdbc.conf and iceberg.conf. Leaving them commented out keeps the single shared fe.conf value. + +# Directory a bare driver jar name in paimon.jdbc.driver_url (or jdbc.driver_url) resolves under +# (JDBC catalog only). The same value is sent to BE with the scan options, so FE and BE resolve a +# given driver_url identically. +# Falls back to fe.conf's jdbc_drivers_dir, whose default is /plugins/jdbc_drivers. +# drivers_dir=/opt/doris/plugins/jdbc_drivers + +# Socket timeout, in seconds, for this catalog's Hive metastore client (HMS catalog only), used +# unless the catalog overrides it. Falls back to fe.conf's hive_metastore_client_timeout_second, +# whose default is 10. +# metastore_client_timeout_second=10 diff --git a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorConfTest.java b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorConfTest.java new file mode 100644 index 00000000000000..01af5258223bc6 --- /dev/null +++ b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorConfTest.java @@ -0,0 +1,127 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.paimon; + +import org.apache.doris.connector.spi.ConnectorConf; +import org.apache.doris.connector.spi.ConnectorContext; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * How this connector's two deployment-level settings are resolved: its own {@code paimon.conf} first, + * then the fe.conf key each used to live under. + * + *

Asserted rather than trusted, because both failure directions are silent. Reading fe.conf first + * would make an administrator's edit to paimon.conf do nothing. Dropping the fe.conf fallback would + * change an untouched deployment's metastore timeout and drivers directory on upgrade, with nothing + * in either file to show why. + */ +public class PaimonConnectorConfTest { + + @Test + public void driversDirPrefersThePluginConfThenFeConf() { + Assertions.assertEquals("/from/plugin/conf", PaimonConnectorProperties.configuredDriversDir( + context(Collections.singletonMap(PaimonConnectorProperties.CONF_DRIVERS_DIR, + "/from/plugin/conf"), + Collections.singletonMap(PaimonConnectorProperties.ENV_JDBC_DRIVERS_DIR, + "/from/fe/conf")))); + + Assertions.assertEquals("/from/fe/conf", PaimonConnectorProperties.configuredDriversDir( + context(Collections.emptyMap(), + Collections.singletonMap(PaimonConnectorProperties.ENV_JDBC_DRIVERS_DIR, + "/from/fe/conf")))); + + Assertions.assertNull(PaimonConnectorProperties.configuredDriversDir( + context(Collections.emptyMap(), Collections.emptyMap()))); + } + + @Test + public void nullContextResolvesToNothingRatherThanThrowing() { + // Both accessors are reached from direct-construction unit tests that pass no context at all + // (PaimonConnector.resolveFullDriverUrl / PaimonScanPlanProvider both null-check today). + Assertions.assertNull(PaimonConnectorProperties.configuredDriversDir(null)); + Assertions.assertNull(PaimonConnectorProperties.configuredDorisHome(null)); + } + + @Test + public void metastoreTimeoutPrefersThePluginConfThenFeConfThenTen() { + Assertions.assertEquals("30", ConnectorConf.get( + context(Collections.singletonMap( + PaimonConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, "30"), + Collections.singletonMap( + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, "20")), + PaimonConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + + Assertions.assertEquals("20", ConnectorConf.get( + context(Collections.emptyMap(), Collections.singletonMap( + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, "20")), + PaimonConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + + // The literal the call site used before this channel existed; keeping it is what makes a + // deployment with neither file behave exactly as it did. + Assertions.assertEquals("10", ConnectorConf.get( + context(Collections.emptyMap(), Collections.emptyMap()), + PaimonConnectorProperties.CONF_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.ENV_HIVE_METASTORE_CLIENT_TIMEOUT_SECOND, + PaimonConnectorProperties.DEFAULT_METASTORE_CLIENT_TIMEOUT_SECOND)); + } + + @Test + public void theConfTemplateIsNamedAfterTheProvider() { + // The engine reads .conf, so a template under any other name deploys a file nothing ever + // opens -- silently, with every setting in it ignored. Renaming getType() must break here. + String expected = new PaimonConnectorProvider().name() + ".conf.template"; + Assertions.assertNotNull(getClass().getClassLoader().getResource(expected), + "the plugin must ship " + expected + " on its classpath"); + } + + private static ConnectorContext context(Map conf, Map env) { + Map confCopy = new HashMap<>(conf); + Map envCopy = new HashMap<>(env); + return new ConnectorContext() { + @Override + public String getCatalogName() { + return "test_catalog"; + } + + @Override + public long getCatalogId() { + return 1L; + } + + @Override + public Map getConnectorConfig() { + return confCopy; + } + + @Override + public Map getEnvironment() { + return envCopy; + } + }; + } +} From 711949a997bb12c3fb543ba517f358f99eb471cf Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:28:35 +0800 Subject: [PATCH 08/13] [chore](connector) drop the unread jdbc_driver_secure_path from the environment No connector reads it. The JDBC driver allow-list is enforced in fe-core by JdbcResource, which reads Config.jdbc_driver_secure_path directly, so this env entry was only ever written -- a dead key that reads like a connector setting and invites someone to build on it. Config.jdbc_driver_secure_path itself stays: JdbcResource is a legitimate in-engine user of it, and the fe.conf key is unchanged for operators. The comment added to buildEnvironment says what the map is now for, so the next person adding a deployment-level setting reaches for the plugin's own conf instead of adding a tenth key here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .../doris/connector/DefaultConnectorContext.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/connector/DefaultConnectorContext.java b/fe/fe-core/src/main/java/org/apache/doris/connector/DefaultConnectorContext.java index 4f3adf1ef6f53b..6439784c1ecaab 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/connector/DefaultConnectorContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/connector/DefaultConnectorContext.java @@ -572,6 +572,17 @@ private static String withTrailingSlash(String uri) { return uri.endsWith("/") ? uri : uri + "/"; } + /** + * The fe.conf values forwarded to every connector. + * + *

Do not add to this. A key here is an engine change per connector setting, and every one + * of them ties a connector's key name into fe-core. A connector that needs a deployment-level + * setting declares it in its own {@code .conf} instead, which the engine reads generically — + * see {@code ConnectorContext.getConnectorConfig()} and {@code ConnectorConf.get}. What is left + * below is either shared by several connectors (the jdbc/hive metastore keys, kept as the fallback + * for deployments that have not moved to the per-plugin files) or not a connector setting at all + * ({@code doris_home}, {@code doris_version}). + */ private static Map buildEnvironment() { Map env = new HashMap<>(); String dorisHome = EnvUtils.getDorisHome(); @@ -581,7 +592,6 @@ private static Map buildEnvironment() { env.put("jdbc_drivers_dir", Config.jdbc_drivers_dir); env.put("force_sqlserver_jdbc_encrypt_false", String.valueOf(Config.force_sqlserver_jdbc_encrypt_false)); - env.put("jdbc_driver_secure_path", Config.jdbc_driver_secure_path); // HMS metastore client socket-timeout default (C4): the metastore-spi cannot read FE Config // (no fe-common dependency), so the FE-configured value is threaded through the environment and // applied by HmsMetaStoreProperties.toHiveConfOverrides when the user has not overridden it. From 7fddf0cbb14b4de44fdfa2d3e613d46e89cc5f6a Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 13:28:35 +0800 Subject: [PATCH 09/13] [docs](connector) document the plugin conf channel Rule 7 listed three knob channels and named the fe.conf one "the one knob shape that requires an engine change per key". That is no longer the shape a new connector should reach for, and leaving the rule as written would keep pointing people at Config.java. It now lists four: the plugin's own .conf is the deployment-level channel, and the fe.conf/getEnvironment one is marked closed to new keys and explained as the fallback that keeps existing deployments working. The connector README gains the corresponding step in "Adding a New Connector", including the two things that are not guessable: the keys take no connector prefix (the file name namespaces them), and is ConnectorProvider.name() rather than the plugin directory name -- plugins/connector/hive/ holds hms.conf. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- fe/fe-connector/README.md | 18 ++++++++++++++- .../doris/connector/api/package-info.java | 23 +++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/fe/fe-connector/README.md b/fe/fe-connector/README.md index 5f142613bcdb3f..5a1dcef5e2d883 100644 --- a/fe/fe-connector/README.md +++ b/fe/fe-connector/README.md @@ -211,7 +211,23 @@ metastore/shade/cache). For a write path, the richest example is (example: `RecordingConnectorContext`). Never touch `connector-metadata-methods.txt` unless you changed the shared SPI surface itself. -14. **Packaging.** Add `src/main/assembly/plugin-zip.xml` (copy from es or +14. **Deployment-level settings** (if any). A value that is one-per-FE rather + than one-per-catalog goes in your plugin's own settings file, NOT in + fe.conf: ship `src/main/resources/.conf.template`, add it to your + assembly's `` at the zip root, and read it with + `ConnectorConf.get(context, "", null, "")`. `` is + `ConnectorProvider.name()`, which is **not** necessarily your plugin + directory name — `plugins/connector/hive/` holds `hms.conf` and + `plugins/connector/trino/` holds `trino-connector.conf`. Guard that with a + test asserting `name() + ".conf.template"` is on the classpath (copy + `IcebergConnectorConfTest#theConfTemplateIsNamedAfterTheProvider`); a + template under any other name deploys a file the engine never opens. + `build.sh` seeds the live `.conf` from the template generically, so it + needs no change. Do NOT add a key to `Config.java` or to + `DefaultConnectorContext.buildEnvironment` — that is an engine change per + setting, and the keys still there are only the ones several connectors + share plus the fe.conf fallbacks kept for existing deployments. +15. **Packaging.** Add `src/main/assembly/plugin-zip.xml` (copy from es or paimon). Verify your module through `package`/`install`, not just `test-compile` — shades and the plugin zip only materialize then. 15. **Gates and e2e.** Your module must pass the forbidden-import gate (runs diff --git a/fe/fe-connector/fe-connector-api/src/main/java/org/apache/doris/connector/api/package-info.java b/fe/fe-connector/fe-connector-api/src/main/java/org/apache/doris/connector/api/package-info.java index bbc502ae22d627..a0e28e0bb43c81 100644 --- a/fe/fe-connector/fe-connector-api/src/main/java/org/apache/doris/connector/api/package-info.java +++ b/fe/fe-connector/fe-connector-api/src/main/java/org/apache/doris/connector/api/package-info.java @@ -160,8 +160,8 @@ * *

Rule 7 — where a connector's tunable knobs live

* - *

Pick the channel by the SCOPE of the value. Only the second one obliges anyone to touch the engine, so - * a knob that can be catalog-scoped should be.

+ *

Pick the channel by the SCOPE of the value. None of them requires an engine change any more, so a + * knob that can be catalog-scoped should be, and one that cannot belongs in the plugin's own conf file.

* *
    *
  • Per catalog → a key in {@code CREATE CATALOG ... PROPERTIES(...)}. The engine hands the @@ -174,9 +174,22 @@ * declared in {@code HiveConnectorProperties} and read in {@code HiveScanPlanProvider} / * {@code HiveConnector}; those key strings appear nowhere in {@code fe-core}.
    • *
  • Per FE process (one deployment-level value for every catalog, e.g. a driver directory) - * → an {@code fe.conf} field forwarded through {@code ConnectorContext.getEnvironment()} by - * {@code DefaultConnectorContext.buildEnvironment}. This is the one knob shape that requires an engine - * change per key, so use it only when the value genuinely is not per catalog.
    • + * → a key in the plugin's own {@code .conf}, read with {@code ConnectorConf.get}. The engine + * locates and parses {@code /.conf} generically + * ({@code ConnectorPluginManager.loadPlugins}) and serves it back through + * {@code ConnectorContext.getConnectorConfig()}, so no key name of yours reaches {@code fe-core} and + * adding one costs the engine nothing. Do not prefix these keys — the file name already + * namespaces them. Ship {@code src/main/resources/.conf.template} and add it to your assembly's + * {@code } at the zip root; {@code build.sh} seeds the live {@code .conf} from it. {@code } + * is {@code ConnectorProvider.name()} and need not equal your plugin directory name + * ({@code plugins/connector/hive/} holds {@code hms.conf}). + *
  • Per FE process, legacy → an {@code fe.conf} field forwarded through + * {@code ConnectorContext.getEnvironment()} by {@code DefaultConnectorContext.buildEnvironment}. This is + * the one shape that requires an engine change per key, and it is closed to new keys. What is + * still there is either shared by several connectors or not a connector setting at all + * ({@code doris_home}, {@code doris_version}); the connector settings among them are kept as the + * fallback {@code ConnectorConf.get} consults after the plugin conf, so deployments configured before + * the conf files existed keep working untouched.
    • *
  • Per session → read the query's session variables from * {@link ConnectorSession#getSessionProperties()}. The connector does not declare them; it looks up the * names it cares about.
    • From eb31e32e444b3024751d275bcd4771e81ac63818 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 14:40:37 +0800 Subject: [PATCH 10/13] [chore](connector) exclude the plugin conf templates from the license check build.sh seeds each connector's live .conf from its template verbatim, so the template's content is the file an administrator edits in plugins/connector//. Listed by name rather than through a **/*.conf.template glob, so a new template is a deliberate entry here rather than something a wildcard silently absorbs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01W8r8ffK711cExL5p6SB1fr --- .licenserc.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.licenserc.yaml b/.licenserc.yaml index 1d4d42d36218c4..afb8d66123913e 100644 --- a/.licenserc.yaml +++ b/.licenserc.yaml @@ -59,6 +59,16 @@ header: - "fe/fe-filesystem/fe-filesystem-spi/src/test/resources/filesystem-plugin-surface.txt" - "fe/fe-authentication/fe-authentication-spi/src/test/resources/authentication-plugin-surface.txt" - "fe/fe-core/src/test/resources/lineage-plugin-surface.txt" + # Connector plugin settings templates. build.sh seeds each connector's live + # .conf from its template verbatim (cp -n), so the template's content IS + # the file an administrator edits in plugins/connector//. Matched by name + # rather than by a **/*.conf.template glob, so a new one is a deliberate entry + # here rather than something a wildcard silently absorbs. + - "fe/fe-connector/fe-connector-hive/src/main/resources/hms.conf.template" + - "fe/fe-connector/fe-connector-iceberg/src/main/resources/iceberg.conf.template" + - "fe/fe-connector/fe-connector-jdbc/src/main/resources/jdbc.conf.template" + - "fe/fe-connector/fe-connector-paimon/src/main/resources/paimon.conf.template" + - "fe/fe-connector/fe-connector-trino/src/main/resources/trino-connector.conf.template" # Golden 4.1.3 upgrade fixtures, emitted by Gen413Fixtures running real 4.1.3 # bytecode (see the sibling PROVENANCE.txt) and stamped "do not edit by hand". # These are the generator's only non-binary outputs -- its .bin files are skipped From 2cd5320fab02e86b68f815dfd8fb7250fb523e28 Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 16:32:02 +0800 Subject: [PATCH 11/13] [fix](fs) probe OBS availability from the jar instead of by linking the class ObsFileSystemProperties picks between fs.obs.impl=OBSFileSystem and the S3AFileSystem fallback with a static probe for org.apache.hadoop.fs.obs.OBSFileSystem. It probed with Class.forName, which answers a strictly harder question than the one being asked: loading the class also links its superclass org.apache.hadoop.fs.FileSystem, and hadoop-common is deliberately not part of this plugin -- hadoop-huaweicloud declares it provided and plugin-zip.xml packs only the runtime closure, so lib/ holds the OBS connector with no hadoop-common behind it. That has one consequence already visible and one waiting: - Wherever hadoop-common is absent, Class.forName throws NoClassDefFoundError. It is a LinkageError, not a ClassNotFoundException, so the catch did not hold it and it aborted the class's static initializer instead; every later touch of ObsFileSystemProperties then failed with "Could not initialize class", not only the hadoop map. This module's own test classpath is exactly that shape, so 25 of its 28 tests fail today. - Merely widening the catch would trade the crash for a lie. The probe would report OBS absent and silently downgrade fs.obs.impl to S3AFileSystem as soon as fe-core stops carrying hadoop -- which #66324 leaves open as the next step -- even though the connector is right there in lib/, and the consumers that actually instantiate it (fe-connector-paimon, be-java-extensions/hadoop-deps) bring their own hadoop. Resolving the class file as a resource asks the intended question, against the same classloader and the same delegation, without linking anything. It is also what this dependency's comment in pom.xml already claims the probe does: "the jar has to sit in this plugin for the probe to tell the truth". The new test pins the native impl and asserts the classpath premise it rests on, so adding hadoop-common later fails loudly instead of quietly turning the test into a tautology. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016YeHiB85SmvZyKCq7FzuJD --- .../obs/ObsFileSystemProperties.java | 20 +++++++++----- .../obs/ObsFileSystemPropertiesTest.java | 27 +++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/fe/fe-filesystem/fe-filesystem-obs/src/main/java/org/apache/doris/filesystem/obs/ObsFileSystemProperties.java b/fe/fe-filesystem/fe-filesystem-obs/src/main/java/org/apache/doris/filesystem/obs/ObsFileSystemProperties.java index 60ec22fb8a3abe..f76c0ce239629e 100644 --- a/fe/fe-filesystem/fe-filesystem-obs/src/main/java/org/apache/doris/filesystem/obs/ObsFileSystemProperties.java +++ b/fe/fe-filesystem/fe-filesystem-obs/src/main/java/org/apache/doris/filesystem/obs/ObsFileSystemProperties.java @@ -417,12 +417,20 @@ private static void putIfNotBlank(Map map, String key, String va } private static boolean isClassAvailable(String className) { - try { - Class.forName(className, false, ObsFileSystemProperties.class.getClassLoader()); - return true; - } catch (ClassNotFoundException e) { - return false; - } + // Read as a resource rather than Class.forName: the question is whether the OBS connector + // ships in this plugin, and loading the class answers a strictly harder one. OBSFileSystem + // comes from hadoop-huaweicloud, which declares its hadoop-common parent provided, so + // hadoop-common is deliberately absent from this plugin's runtime closure and from lib/. + // Class.forName has to link the missing superclass org.apache.hadoop.fs.FileSystem and + // throws NoClassDefFoundError -- a LinkageError, not a ClassNotFoundException, so the + // former catch did not hold it and it aborted this class's static initializer instead. + // Widening the catch would only trade the crash for a lie: the probe would report OBS + // absent and silently downgrade fs.obs.impl to S3AFileSystem as soon as the host stops + // supplying hadoop-common, even though the connector is right there in lib/ and the + // consumers that actually instantiate it (fe-connector-paimon, be-java-extensions/ + // hadoop-deps) carry their own hadoop. Resolving the class file uses the same classloader + // and the same delegation without linking anything. + return ObsFileSystemProperties.class.getResource("/" + className.replace('.', '/') + ".class") != null; } @Override diff --git a/fe/fe-filesystem/fe-filesystem-obs/src/test/java/org/apache/doris/filesystem/obs/ObsFileSystemPropertiesTest.java b/fe/fe-filesystem/fe-filesystem-obs/src/test/java/org/apache/doris/filesystem/obs/ObsFileSystemPropertiesTest.java index 2aa9b7048b096c..a9f288e5f6ba41 100644 --- a/fe/fe-filesystem/fe-filesystem-obs/src/test/java/org/apache/doris/filesystem/obs/ObsFileSystemPropertiesTest.java +++ b/fe/fe-filesystem/fe-filesystem-obs/src/test/java/org/apache/doris/filesystem/obs/ObsFileSystemPropertiesTest.java @@ -129,6 +129,33 @@ void toMaps_emitObsTuningDefaultsWhenNotConfigured() { Assertions.assertEquals("10000", hadoopKv.get("fs.s3a.connection.timeout")); } + @Test + void hadoopMap_selectsObsFileSystemWithoutLinkingIt() { + // Premise this test rests on, and the exact shape of a deployment where fe-core no longer + // supplies hadoop: hadoop-huaweicloud puts org.apache.hadoop.fs.obs.OBSFileSystem on this + // module's classpath, while hadoop-common -- which owns its superclass + // org.apache.hadoop.fs.FileSystem -- is deliberately not part of this plugin. Loading the + // class therefore fails with NoClassDefFoundError, a LinkageError and NOT a + // ClassNotFoundException. Asserted so that adding hadoop-common later fails here loudly + // rather than quietly turning the rest of this test into a tautology. + Assertions.assertThrows(NoClassDefFoundError.class, + () -> Class.forName("org.apache.hadoop.fs.obs.OBSFileSystem", false, + ObsFileSystemProperties.class.getClassLoader())); + + // The probe asks whether the OBS connector ships in this plugin, not whether this JVM can + // link it, so an unlinkable-but-present OBSFileSystem must still select the native impl -- + // the consumers that instantiate it carry their own hadoop. Probing with Class.forName got + // both halves wrong here: it threw, and catching the LinkageError would have downgraded + // fs.obs.impl to S3AFileSystem while the connector sat in lib/. + ObsFileSystemProperties properties = ObsFileSystemProperties.of(Map.of( + "obs.endpoint", "https://obs.cn-north-4.myhuaweicloud.com")); + + Map hadoopKv = properties.toHadoopConfigurationMap(); + Assertions.assertEquals("org.apache.hadoop.fs.obs.OBSFileSystem", hadoopKv.get("fs.obs.impl")); + Assertions.assertEquals("org.apache.hadoop.fs.obs.OBS", + hadoopKv.get("fs.AbstractFileSystem.obs.impl")); + } + @Test void bind_rejectsPartialStaticCredentialsLikeFeCore() { IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, From 09e138c241759f758577c62b17b02d8507d6393d Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 16:51:05 +0800 Subject: [PATCH 12/13] [fix](ci) fail FE UT when a nested module's tests fail The FE UT job parses results with the report pattern fe/*/target/surefire-reports/*.xml. That single wildcard only reaches the 17 modules sitting directly under fe/; the other 56 -- everything under fe-filesystem/, fe-connector/, fe-authentication/ and be-java-extensions/ -- is invisible to it. The coverage run also passes -Dmaven.test.failure.ignore=true, which it needs in order to finish every module and still emit a jacoco report, so maven exits 0 and the reactor prints SUCCESS for a module whose tests failed. Together those two make a nested module's failures unobservable: the job goes green reporting "failed: 0". Build 1012189 is the worked example -- 25 of fe-filesystem-obs' 28 tests failed there and nothing anywhere said so. So gate on the reports the parser cannot see, and leave the ones it can reach to the job itself: it owns those results, and its per-test mutes have to keep working. Two things that are easy to get wrong here: - The obvious way to express "skip the ones the parser already sees" is a [[ ]] glob against the parser's own pattern, and it is wrong: inside [[ ]] a * matches / as well, so fe/*/target/... also swallows every nested module this is meant to catch, and the check silently passes everything. Comparing the shape of the module path is exact. - The totals are read from the root element only, since a stack trace quoted inside a later can otherwise be mistaken for it. Scoped to the full run: --run leaves every other module's reports from an earlier invocation untouched, and those are not that run's results. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016YeHiB85SmvZyKCq7FzuJD --- run-fe-ut.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/run-fe-ut.sh b/run-fe-ut.sh index 27adbc745e52f6..a941a4a7651a70 100755 --- a/run-fe-ut.sh +++ b/run-fe-ut.sh @@ -36,6 +36,49 @@ is_valid_extra_module_feature() { [[ "${feature}" =~ ^[A-Za-z][A-Za-z0-9_-]*$ ]] } +# The CI job parses FE UT results with the report pattern fe/*/target/surefire-reports/*.xml. That +# single wildcard only reaches the modules sitting directly under fe/; every module nested one level +# deeper -- fe-filesystem/*, fe-connector/*, fe-authentication/* -- is invisible to it. Combined +# with the -Dmaven.test.failure.ignore=true that the coverage run needs in order to finish every +# module and still emit a jacoco report, a failing nested module leaves maven at exit 0, the reactor +# printing SUCCESS for it, and the job green with "failed: 0". +# +# So gate here on the reports the CI parser cannot see. The ones it can see are deliberately left to +# it: it owns those results, and its per-test mutes have to keep working. +fail_on_unparsed_test_failures() { + local report module header failures errors + local -a broken=() + + while IFS= read -r report; do + # The module path relative to fe/, e.g. "fe-core" or "fe-filesystem/fe-filesystem-obs". + # No slash in it means the module sits directly under fe/, which is exactly what the CI + # pattern's single wildcard reaches -- leave those to the CI parser. Deliberately not + # written as a [[ ]] glob against the pattern itself: there, * also matches /, so + # fe/*/target/... would swallow the nested modules this function exists to catch. + module="${report#"${DORIS_HOME}"/fe/}" + module="${module%%/target/*}" + [[ "${module}" != */* ]] && continue + + # The totals live on the root element. -m1 so that a stack trace quoted inside + # some later can never be mistaken for it. + header="$(grep -m1 -o ']*>' "${report}")" || continue + failures="$(sed -n 's/.*failures="\([0-9]*\)".*/\1/p' <<<"${header}")" + errors="$(sed -n 's/.*errors="\([0-9]*\)".*/\1/p' <<<"${header}")" + + if [[ "${failures:-0}" -gt 0 || "${errors:-0}" -gt 0 ]]; then + broken+=("${report#"${DORIS_HOME}/"} -- failures=${failures:-0} errors=${errors:-0}") + fi + done < <(find "${DORIS_HOME}/fe" -type f -path '*/target/surefire-reports/*.xml') + + if [[ "${#broken[@]}" -ne 0 ]]; then + echo "" + echo "FE UT failed in ${#broken[@]} test class(es) whose module the CI report pattern does not reach:" + printf ' %s\n' "${broken[@]}" + echo "" + return 1 + fi +} + parse_extra_fe_modules() { local spec_value="$1" local entry feature module_path existing @@ -202,4 +245,9 @@ else "${MVN_CMD}" test -pl "${MVN_MODULES}" -am -Dcheckstyle.skip=true -DfailIfNoTests=false \ -Dmaven.build.cache.enabled=false fi + + # Only reachable when maven itself exited 0, which under -Dmaven.test.failure.ignore=true it + # does even with failing tests. Deliberately not run for --run: that invocation leaves every + # other module's reports from an earlier run untouched, and those are not this run's results. + fail_on_unparsed_test_failures fi From b489a50678b0bf8e786d949113f7339556645c3f Mon Sep 17 00:00:00 2001 From: morningman Date: Sat, 1 Aug 2026 18:39:30 +0800 Subject: [PATCH 13/13] [fix](test) match the hive bucket-gate message that names both conf keys The bucket gate moved onto the plugin conf channel, so it can now be off in either hms.conf (enable_create_bucket_table) or fe.conf (enable_create_hive_bucket_table), and the rejection message was reworded to name both. test_hive_ddl still pinned the old single-key wording and failed. Updating the assertion rather than the message: with the plugin conf winning, a message that names only the fe.conf key sends an administrator whose hms.conf holds the toggle false to edit a file that cannot turn it back on. Co-Authored-By: Claude Opus 5 (1M context) --- .../suites/external_table_p0/hive/ddl/test_hive_ddl.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regression-test/suites/external_table_p0/hive/ddl/test_hive_ddl.groovy b/regression-test/suites/external_table_p0/hive/ddl/test_hive_ddl.groovy index b654819826ed27..75f92730664205 100644 --- a/regression-test/suites/external_table_p0/hive/ddl/test_hive_ddl.groovy +++ b/regression-test/suites/external_table_p0/hive/ddl/test_hive_ddl.groovy @@ -462,7 +462,7 @@ suite("test_hive_ddl", "p0,external") { 'replication_num' = '1' ); """ - exception "Create hive bucket table need set enable_create_hive_bucket_table to true" + exception "Create hive bucket table need set 'enable_create_bucket_table' in hms.conf (or enable_create_hive_bucket_table in fe.conf) to true" } sql """ SWITCH internal """