This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 9430b27e682 [branch-2.1][improvement](jdbc catalog) improvement some
jdbc catalog properties check order (#38770)
9430b27e682 is described below
commit 9430b27e6825ee8c3829181aa8d994d85fce7545
Author: zy-kkk <zhongy...@gmail.com>
AuthorDate: Mon Aug 5 09:14:04 2024 +0800
[branch-2.1][improvement](jdbc catalog) improvement some jdbc catalog
properties check order (#38770)
pick (#38439)
1. Move the execution of testJdbcConnection() to checkWhenCreating
instead of the constructor
2. Move the logic of renaming lower_case_table_names to
lower_case_meta_names to setDefaultPropsIfMissing
---
.../apache/doris/datasource/CatalogFactory.java | 2 +-
.../apache/doris/datasource/CatalogProperty.java | 4 ++
.../doris/datasource/jdbc/JdbcExternalCatalog.java | 57 +++++++++++-----------
.../datasource/jdbc/JdbcExternalCatalogTest.java | 26 +++++++---
4 files changed, 52 insertions(+), 37 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java
index fdd3b8fbe02..44e0470b069 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java
@@ -122,7 +122,7 @@ public class CatalogFactory {
catalog = new EsExternalCatalog(catalogId, name, resource,
props, comment);
break;
case "jdbc":
- catalog = new JdbcExternalCatalog(catalogId, name, resource,
props, comment, isReplay);
+ catalog = new JdbcExternalCatalog(catalogId, name, resource,
props, comment);
break;
case "iceberg":
catalog =
IcebergExternalCatalogFactory.createCatalog(catalogId, name, resource, props,
comment);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
index 50aea847e8f..a54a28d59dd 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
@@ -115,6 +115,10 @@ public class CatalogProperty implements Writable {
this.properties.put(key, val);
}
+ public void deleteProperty(String key) {
+ this.properties.remove(key);
+ }
+
@Override
public void write(DataOutput out) throws IOException {
Text.writeString(out, GsonUtils.GSON.toJson(this));
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
index 568c9e8480c..73b6639c7b9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
@@ -72,11 +72,10 @@ public class JdbcExternalCatalog extends ExternalCatalog {
private transient JdbcClient jdbcClient;
public JdbcExternalCatalog(long catalogId, String name, String resource,
Map<String, String> props,
- String comment, boolean isReplay)
+ String comment)
throws DdlException {
super(catalogId, name, InitCatalogLog.Type.JDBC, comment);
- this.catalogProperty = new CatalogProperty(resource,
processCompatibleProperties(props, isReplay));
- testJdbcConnection(isReplay);
+ this.catalogProperty = new CatalogProperty(resource,
processCompatibleProperties(props));
}
@Override
@@ -99,6 +98,21 @@ public class JdbcExternalCatalog extends ExternalCatalog {
getConnectionPoolMaxWaitTime(),
getConnectionPoolMaxLifeTime());
}
+ @Override
+ public void setDefaultPropsIfMissing(boolean isReplay) {
+ super.setDefaultPropsIfMissing(isReplay);
+ // Modify lower_case_table_names to lower_case_meta_names if it exists
+ if
(catalogProperty.getProperties().containsKey("lower_case_table_names") &&
isReplay) {
+ String lowerCaseTableNamesValue =
catalogProperty.getProperties().get("lower_case_table_names");
+ catalogProperty.addProperty("lower_case_meta_names",
lowerCaseTableNamesValue);
+ catalogProperty.deleteProperty("lower_case_table_names");
+ LOG.info("Modify lower_case_table_names to lower_case_meta_names,
value: {}", lowerCaseTableNamesValue);
+ } else if
(catalogProperty.getProperties().containsKey("lower_case_table_names") &&
!isReplay) {
+ throw new IllegalArgumentException("Jdbc catalog property
lower_case_table_names is not supported,"
+ + " please use lower_case_meta_names instead.");
+ }
+ }
+
@Override
public void onRefresh(boolean invalidCache) {
super.onRefresh(invalidCache);
@@ -115,24 +129,12 @@ public class JdbcExternalCatalog extends ExternalCatalog {
}
}
- protected Map<String, String> processCompatibleProperties(Map<String,
String> props, boolean isReplay)
+ protected Map<String, String> processCompatibleProperties(Map<String,
String> props)
throws DdlException {
Map<String, String> properties = Maps.newHashMap();
for (Map.Entry<String, String> kv : props.entrySet()) {
properties.put(StringUtils.removeStart(kv.getKey(),
JdbcResource.JDBC_PROPERTIES_PREFIX), kv.getValue());
}
-
- // Modify lower_case_table_names to lower_case_meta_names if it exists
- if (properties.containsKey("lower_case_table_names") && isReplay) {
- String lowerCaseTableNamesValue =
properties.get("lower_case_table_names");
- properties.put("lower_case_meta_names", lowerCaseTableNamesValue);
- properties.remove("lower_case_table_names");
- LOG.info("Modify lower_case_table_names to lower_case_meta_names,
value: {}", lowerCaseTableNamesValue);
- } else if (properties.containsKey("lower_case_table_names") &&
!isReplay) {
- throw new DdlException("Jdbc catalog property
lower_case_table_names is not supported,"
- + " please use lower_case_meta_names instead");
- }
-
String jdbcUrl = properties.getOrDefault(JdbcResource.JDBC_URL, "");
if (!Strings.isNullOrEmpty(jdbcUrl)) {
jdbcUrl = JdbcResource.handleJdbcUrl(jdbcUrl);
@@ -272,6 +274,7 @@ public class JdbcExternalCatalog extends ExternalCatalog {
catalogProperty.addProperty(JdbcResource.CHECK_SUM,
computedChecksum);
}
}
+ testJdbcConnection();
}
/**
@@ -313,22 +316,20 @@ public class JdbcExternalCatalog extends ExternalCatalog {
jdbcTable.setConnectionPoolKeepAlive(this.isConnectionPoolKeepAlive());
}
- private void testJdbcConnection(boolean isReplay) throws DdlException {
+ private void testJdbcConnection() throws DdlException {
if (FeConstants.runningUnitTest) {
// skip test connection in unit test
return;
}
- if (!isReplay) {
- if (isTestConnection()) {
- try {
- initLocalObjectsImpl();
- testFeToJdbcConnection();
- testBeToJdbcConnection();
- } finally {
- if (jdbcClient != null) {
- jdbcClient.closeClient();
- jdbcClient = null;
- }
+ if (isTestConnection()) {
+ try {
+ initLocalObjectsImpl();
+ testFeToJdbcConnection();
+ testBeToJdbcConnection();
+ } finally {
+ if (jdbcClient != null) {
+ jdbcClient.closeClient();
+ jdbcClient = null;
}
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalogTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalogTest.java
index 35ef5fd56f2..5a7379b2e84 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalogTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalogTest.java
@@ -41,24 +41,34 @@ public class JdbcExternalCatalogTest {
properties.put(JdbcResource.DRIVER_URL, "ojdbc8.jar");
properties.put(JdbcResource.JDBC_URL,
"jdbc:oracle:thin:@127.0.0.1:1521:XE");
properties.put(JdbcResource.DRIVER_CLASS,
"oracle.jdbc.driver.OracleDriver");
- jdbcExternalCatalog = new JdbcExternalCatalog(1L, "testCatalog", null,
properties, "testComment", false);
+ jdbcExternalCatalog = new JdbcExternalCatalog(1L, "testCatalog", null,
properties, "testComment");
}
@Test
- public void testProcessCompatibleProperties() throws DdlException {
+ public void testProcessCompatibleProperties() {
// Create a properties map with lower_case_table_names
- Map<String, String> inputProps = new HashMap<>();
- inputProps.put("lower_case_table_names", "true");
+
jdbcExternalCatalog.getCatalogProperty().addProperty("lower_case_table_names",
"true");
// Call processCompatibleProperties
- Map<String, String> resultProps =
jdbcExternalCatalog.processCompatibleProperties(inputProps, true);
+ jdbcExternalCatalog.setDefaultPropsIfMissing(true);
// Assert that lower_case_meta_names is present and has the correct
value
- Assert.assertTrue(resultProps.containsKey("lower_case_meta_names"));
- Assert.assertEquals("true", resultProps.get("lower_case_meta_names"));
+ Assert.assertTrue(
+
jdbcExternalCatalog.getCatalogProperty().getProperties().containsKey("lower_case_meta_names"));
+ Assert.assertEquals("true",
+
jdbcExternalCatalog.getCatalogProperty().getProperties().get("lower_case_meta_names"));
// Assert that lower_case_table_names is not present
- Assert.assertFalse(resultProps.containsKey("lower_case_table_names"));
+ Assert.assertFalse(
+
jdbcExternalCatalog.getCatalogProperty().getProperties().containsKey("lower_case_table_names"));
+
+
jdbcExternalCatalog.getCatalogProperty().addProperty("lower_case_table_names",
"true");
+ IllegalArgumentException exceptione =
Assert.assertThrows(IllegalArgumentException.class,
+ () -> jdbcExternalCatalog.setDefaultPropsIfMissing(false));
+ Assert.assertEquals(
+ "Jdbc catalog property lower_case_table_names is not
supported, please use lower_case_meta_names instead.",
+ exceptione.getMessage());
+
jdbcExternalCatalog.getCatalogProperty().deleteProperty("lower_case_table_names");
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
