This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 8f7b9e01a3 [#13346] fix(doris): Preserve table comments when loading
tables (#13347)
8f7b9e01a3 is described below
commit 8f7b9e01a3325ef8ed44e5a690fed0db01e86bdc
Author: Qi Yu <[email protected]>
AuthorDate: Sun Sep 20 18:52:02 2026 +0800
[#13346] fix(doris): Preserve table comments when loading tables (#13347)
### What changes were proposed in this pull request?
Keep the existing `information_schema.TABLES.TABLE_COMMENT` fallback and
also use it when JDBC reports `OLAP` as `REMARKS`. Add regression
coverage for empty, valid, and engine-valued JDBC comments, plus
integration coverage for Doris 1.2 and 4.x.
### Why are the changes needed?
On affected Doris versions, JDBC metadata reports the table engine
(`OLAP`) as `REMARKS`. The connector currently skips its SQL comment
lookup whenever `REMARKS` is nonempty, so a table created with a comment
loads with the wrong comment. The SQL result also carries Gravitino's ID
suffix, which the existing catalog load path removes.
Fix: #13346
### Does this PR introduce _any_ user-facing change?
Loaded Doris tables return the actual user comment instead of `OLAP`. No
API or property key changes.
### How was this patch tested?
- `./gradlew spotlessApply :catalogs:catalog-jdbc-doris:test -PskipITs
-PskipDockerTests=true -PskipWeb=true --console=plain`
- `./gradlew :catalogs:catalog-jdbc-doris:test --tests
org.apache.gravitino.catalog.doris.integration.test.CatalogDorisIT.testTableCommentRoundTrip
-PskipDockerTests=false -PdorisMultiVersionTest -PskipWeb=true
--console=plain`
The Doris 4.x integration test was added but could not complete locally
because its container startup stalled.
---
.../doris/operation/DorisTableOperations.java | 7 +-
.../doris/integration/test/CatalogDoris4xIT.java | 18 +++++
.../doris/integration/test/CatalogDorisIT.java | 40 ++++++++++
.../operation/TestDorisTableCommentOperations.java | 92 ++++++++++++++++++++++
4 files changed, 155 insertions(+), 2 deletions(-)
diff --git
a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
index 40b31f3588..d093c30613 100644
---
a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
+++
b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
@@ -662,11 +662,14 @@ public class DorisTableOperations extends
JdbcTableOperations {
protected void correctJdbcTableFields(
Connection connection, String databaseName, String tableName,
JdbcTable.Builder tableBuilder)
throws SQLException {
- if (StringUtils.isNotEmpty(tableBuilder.comment())) {
+ if (StringUtils.isNotEmpty(tableBuilder.comment())
+ && !"OLAP".equalsIgnoreCase(tableBuilder.comment())) {
return;
}
- // Doris Cannot get comment from JDBC 8.x, so we need to get comment from
sql
+ // Doris JDBC metadata can report the OLAP engine as REMARKS. Query the
actual table comment
+ // from information_schema when REMARKS is empty or contains that engine
name. Preserve the
+ // Gravitino ID suffix so JdbcCatalogOperations can extract it when
loading the table.
StringBuilder comment = new StringBuilder();
String sql =
"SELECT TABLE_COMMENT FROM information_schema.TABLES WHERE
TABLE_SCHEMA = ? AND TABLE_NAME = ?";
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
index e838a8274d..46e80b6de5 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
@@ -166,6 +166,24 @@ public class CatalogDoris4xIT extends BaseIT {
return Distributions.hash(1, NamedReference.field(colName1));
}
+ @Test
+ void testTableCommentRoundTrip() {
+ TableCatalog tables = catalog.asTableCatalog();
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName,
"comment_roundtrip");
+ String comment = "crud probe";
+
+ tables.createTable(
+ tableIdentifier,
+ basicColumns(),
+ comment,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ hashDist(),
+ null);
+
+ assertEquals(comment, tables.loadTable(tableIdentifier).comment());
+ }
+
@Test
void testCreateTableWithInvertedIndex() {
TableCatalog tc = catalog.asTableCatalog();
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
index bdfc8ce065..8b9af56ca2 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
@@ -32,6 +32,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.io.IOException;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.Arrays;
@@ -47,6 +51,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Schema;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.SupportsSchemas;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.client.GravitinoMetalake;
@@ -217,6 +222,41 @@ public class CatalogDorisIT extends BaseIT {
assertEquals(createdSchema.properties().get(propKey), propValue);
}
+ @Test
+ void testTableCommentRoundTrip() throws Exception {
+ TableCatalog tables = catalog.asTableCatalog();
+ NameIdentifier tableIdentifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName("comment_roundtrip"));
+ String comment = "crud probe";
+
+ tables.createTable(
+ tableIdentifier,
+ createColumns(),
+ comment,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ createDistribution(),
+ null);
+
+ assertEquals(comment, tables.loadTable(tableIdentifier).comment());
+
+ String sql =
+ "SELECT TABLE_COMMENT FROM information_schema.TABLES WHERE
TABLE_SCHEMA = ? AND TABLE_NAME = ?";
+ try (Connection connection =
+ DriverManager.getConnection(
+ jdbcUrl + schemaName, DorisContainer.USER_NAME,
DorisContainer.PASSWORD);
+ PreparedStatement statement = connection.prepareStatement(sql)) {
+ statement.setString(1, schemaName);
+ statement.setString(2, tableIdentifier.name());
+ try (ResultSet result = statement.executeQuery()) {
+ assertTrue(result.next());
+ String storedComment = result.getString("TABLE_COMMENT");
+ assertTrue(StringIdentifier.fromComment(storedComment) != null);
+ assertEquals(comment,
StringIdentifier.removeIdFromComment(storedComment));
+ }
+ }
+ }
+
@Test
void testTablePropertiesRoundTrip() {
// Verify writable table properties survive the create → Doris 1.2 → load
round-trip.
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableCommentOperations.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableCommentOperations.java
new file mode 100644
index 0000000000..5d16853a2d
--- /dev/null
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableCommentOperations.java
@@ -0,0 +1,92 @@
+/*
+ * 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.gravitino.catalog.doris.operation;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.startsWith;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
+import org.junit.jupiter.api.Test;
+
+class TestDorisTableCommentOperations {
+ @Test
+ void testEngineMetadataDoesNotReplaceTableComment() throws Exception {
+ Connection connection = mock(Connection.class);
+ PreparedStatement commentStatement = mock(PreparedStatement.class);
+ ResultSet commentResult = mock(ResultSet.class);
+ PreparedStatement statusStatement = mock(PreparedStatement.class);
+ ResultSet statusResult = mock(ResultSet.class);
+ when(connection.prepareStatement(startsWith("SELECT TABLE_COMMENT")))
+ .thenReturn(commentStatement);
+ when(commentStatement.executeQuery()).thenReturn(commentResult);
+ when(commentResult.next()).thenReturn(true, false);
+ when(commentResult.getString("TABLE_COMMENT")).thenReturn("crud probe");
+ when(connection.prepareStatement(startsWith("SHOW ALTER TABLE COLUMN")))
+ .thenReturn(statusStatement);
+ when(statusStatement.executeQuery()).thenReturn(statusResult);
+
+ JdbcTable.Builder tableBuilder = JdbcTable.builder().withComment("OLAP");
+ new DorisTableOperations().correctJdbcTableFields(connection, "db", "t",
tableBuilder);
+
+ assertEquals("crud probe", tableBuilder.comment());
+ verify(commentStatement).setString(1, "db");
+ verify(commentStatement).setString(2, "t");
+ }
+
+ @Test
+ void testEmptyJdbcCommentUsesInformationSchema() throws Exception {
+ Connection connection = mock(Connection.class);
+ PreparedStatement commentStatement = mock(PreparedStatement.class);
+ ResultSet commentResult = mock(ResultSet.class);
+ PreparedStatement statusStatement = mock(PreparedStatement.class);
+ ResultSet statusResult = mock(ResultSet.class);
+ when(connection.prepareStatement(startsWith("SELECT TABLE_COMMENT")))
+ .thenReturn(commentStatement);
+ when(commentStatement.executeQuery()).thenReturn(commentResult);
+ when(commentResult.next()).thenReturn(true, false);
+ when(commentResult.getString("TABLE_COMMENT")).thenReturn("crud probe");
+ when(connection.prepareStatement(startsWith("SHOW ALTER TABLE COLUMN")))
+ .thenReturn(statusStatement);
+ when(statusStatement.executeQuery()).thenReturn(statusResult);
+
+ JdbcTable.Builder tableBuilder = JdbcTable.builder();
+ new DorisTableOperations().correctJdbcTableFields(connection, "db", "t",
tableBuilder);
+
+ assertEquals("crud probe", tableBuilder.comment());
+ }
+
+ @Test
+ void testValidJdbcCommentNeedsNoFallback() throws Exception {
+ Connection connection = mock(Connection.class);
+ JdbcTable.Builder tableBuilder = JdbcTable.builder().withComment("crud
probe");
+
+ new DorisTableOperations().correctJdbcTableFields(connection, "db", "t",
tableBuilder);
+
+ assertEquals("crud probe", tableBuilder.comment());
+ verify(connection, never()).prepareStatement(anyString());
+ }
+}
