This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-1.1
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.1 by this push:
new f153e6d055 [#10034] fix(core): Fix loading table problem due to
incorrect SQL sentence in fetch column infos (#10062)(cherry-pick) (#10079)
f153e6d055 is described below
commit f153e6d055f3068de134e5f357f235d9d5e26033
Author: Qi Yu <[email protected]>
AuthorDate: Mon Mar 2 11:26:56 2026 +0800
[#10034] fix(core): Fix loading table problem due to incorrect SQL sentence
in fetch column infos (#10062)(cherry-pick) (#10079)
### What changes were proposed in this pull request?
This pull request introduces improvements to logging for column updates,
refines the SQL query for selecting columns to prefer non-deleted
records, and adds a new test to verify this behavior. The most important
changes are grouped below by theme.
### Why are the changes needed?
It's a bug.
Fix: #10034
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
Newly added UT and test locally.
---
.github/workflows/build.yml | 2 +-
.../catalog/TableOperationDispatcher.java | 54 ++++++++++++++++--
.../provider/base/TableColumnBaseSQLProvider.java | 5 +-
.../service/TestTableColumnMetaService.java | 64 ++++++++++++++++++++++
4 files changed, 118 insertions(+), 7 deletions(-)
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 3da6d54a18..8936a8e1c8 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -110,7 +110,7 @@ jobs:
strategy:
matrix:
java-version: [ 17 ]
- timeout-minutes: 60
+ timeout-minutes: 90
needs: changes
if: needs.changes.outputs.source_changes == 'true'
# Steps represent a sequence of tasks that will be executed as part of the
job
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
index 7446135834..57499b6f6d 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
@@ -671,6 +671,48 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
&& Objects.equal(left.defaultValue(), right.defaultValue());
}
+ private String columnDifferenceContent(
+ Column catalogColumn, int catalogPosition, ColumnEntity entityColumn) {
+ List<String> differences = Lists.newArrayList();
+ if (!Objects.equal(catalogColumn.name(), entityColumn.name())) {
+ differences.add(
+ String.format("name[catalog=%s, store=%s]", catalogColumn.name(),
entityColumn.name()));
+ }
+ if (catalogPosition != entityColumn.position()) {
+ differences.add(
+ String.format(
+ "position[catalog=%s, store=%s]", catalogPosition,
entityColumn.position()));
+ }
+ if (!Objects.equal(catalogColumn.dataType(), entityColumn.dataType())) {
+ differences.add(
+ String.format(
+ "type[catalog=%s, store=%s]", catalogColumn.dataType(),
entityColumn.dataType()));
+ }
+ if (!Objects.equal(catalogColumn.comment(), entityColumn.comment())) {
+ differences.add(
+ String.format(
+ "comment[catalog=%s, store=%s]", catalogColumn.comment(),
entityColumn.comment()));
+ }
+ if (catalogColumn.nullable() != entityColumn.nullable()) {
+ differences.add(
+ String.format(
+ "nullable[catalog=%s, store=%s]", catalogColumn.nullable(),
entityColumn.nullable()));
+ }
+ if (catalogColumn.autoIncrement() != entityColumn.autoIncrement()) {
+ differences.add(
+ String.format(
+ "autoIncrement[catalog=%s, store=%s]",
+ catalogColumn.autoIncrement(), entityColumn.autoIncrement()));
+ }
+ if (!Objects.equal(catalogColumn.defaultValue(),
entityColumn.defaultValue())) {
+ differences.add(
+ String.format(
+ "defaultValue[catalog=%s, store=%s]",
+ catalogColumn.defaultValue(), entityColumn.defaultValue()));
+ }
+ return String.join(", ", differences);
+ }
+
private Pair<Boolean, List<ColumnEntity>> updateColumnsIfNecessary(
Table tableFromCatalog, TableEntity tableFromGravitino) {
if (tableFromCatalog == null || tableFromGravitino == null) {
@@ -705,11 +747,12 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
columnsNeedsUpdate = true;
} else if (!isSameColumn(columnPair.getRight(), columnPair.getLeft(),
entry.getValue())) {
- // If the column need to be updated, we create a new ColumnEntity with
the same id
+ // Add debug log to print the difference between the two columns
LOG.debug(
"Column {} is found in the table from underlying source, but it is
different "
- + "from the one in the table entity, it will be updated",
- entry.getKey());
+ + "from the one in the table entity, it will be updated,
differences: {}",
+ entry.getKey(),
+ columnDifferenceContent(columnPair.getRight(),
columnPair.getLeft(), entry.getValue()));
Column column = columnPair.getRight();
ColumnEntity updatedColumnEntity =
@@ -744,9 +787,10 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
for (Map.Entry<String, Pair<Integer, Column>> entry :
columnsFromCatalogTable.entrySet()) {
if (!columnsFromTableEntity.containsKey(entry.getKey())) {
LOG.debug(
- "Column {} is found in the table from underlying source but not in
the table "
+ "Column {} of table: {} is found in the table from underlying
source but not in the table "
+ "entity, it will be added to the table entity",
- entry.getKey());
+ entry.getKey(),
+ tableFromGravitino.id());
ColumnEntity newColumnEntity =
ColumnEntity.toColumnEntity(
entry.getValue().getRight(),
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java
index f42cee072f..98ac81ecbf 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java
@@ -123,7 +123,10 @@ public class TableColumnBaseSQLProvider {
+ " FROM "
+ TableColumnMapper.COLUMN_TABLE_NAME
+ " WHERE table_id = #{tableId} AND column_name = #{columnName} AND
deleted_at = 0"
- + " ORDER BY table_version DESC LIMIT 1";
+ // Update a column will generate two records with the same version,
one with op_type = 2
+ // (update) and another with op_type = 3 (delete). We should not
return NULL if both records
+ // exist with the same version, otherwise the caller will think the
column does not exist.
+ + " ORDER BY table_version DESC, column_op_type ASC, id DESC LIMIT 1";
}
public String selectColumnPOById(@Param("columnId") Long columnId) {
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestTableColumnMetaService.java
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestTableColumnMetaService.java
index 528db047e5..057a12ac16 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestTableColumnMetaService.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestTableColumnMetaService.java
@@ -529,6 +529,70 @@ public class TestTableColumnMetaService extends
TestJDBCBackend {
() ->
TableColumnMetaService.getInstance().getColumnPOById(updatedColumn.id()));
}
+ @TestTemplate
+ public void testGetColumnIdByNamePrefersNonDeletedRecordInSameVersion()
throws IOException {
+ String catalogName = "catalog1";
+ String schemaName = "schema1";
+ createParentEntities(METALAKE_NAME, catalogName, schemaName, AUDIT_INFO);
+
+ ColumnEntity oldColumn =
+ ColumnEntity.builder()
+ .withId(RandomIdGenerator.INSTANCE.nextId())
+ .withName("column1")
+ .withPosition(0)
+ .withComment("comment1")
+ .withDataType(Types.IntegerType.get())
+ .withNullable(true)
+ .withAutoIncrement(false)
+ .withDefaultValue(Literals.integerLiteral(1))
+ .withAuditInfo(AUDIT_INFO)
+ .build();
+
+ TableEntity createdTable =
+ TableEntity.builder()
+ .withId(RandomIdGenerator.INSTANCE.nextId())
+ .withName("table_same_name")
+ .withNamespace(Namespace.of(METALAKE_NAME, catalogName,
schemaName))
+ .withColumns(Lists.newArrayList(oldColumn))
+ .withAuditInfo(AUDIT_INFO)
+ .build();
+ TableMetaService.getInstance().insertTable(createdTable, false);
+
+ TableEntity retrievedTable =
+
TableMetaService.getInstance().getTableByIdentifier(createdTable.nameIdentifier());
+
+ // Replace the column with a new column id but the same name in one table
update.
+ ColumnEntity newColumn =
+ ColumnEntity.builder()
+ .withId(RandomIdGenerator.INSTANCE.nextId())
+ .withName("column1")
+ .withPosition(0)
+ .withComment("comment1_new")
+ .withDataType(Types.LongType.get())
+ .withNullable(true)
+ .withAutoIncrement(false)
+ .withDefaultValue(Literals.longLiteral(2L))
+ .withAuditInfo(AUDIT_INFO)
+ .build();
+
+ TableEntity updatedTable =
+ TableEntity.builder()
+ .withId(retrievedTable.id())
+ .withName(retrievedTable.name())
+ .withNamespace(retrievedTable.namespace())
+ .withColumns(Lists.newArrayList(newColumn))
+ .withAuditInfo(retrievedTable.auditInfo())
+ .build();
+
+ TableMetaService.getInstance()
+ .updateTable(retrievedTable.nameIdentifier(), old -> updatedTable);
+
+ Long selectedColumnId =
+ TableColumnMetaService.getInstance()
+ .getColumnIdByTableIdAndName(retrievedTable.id(),
newColumn.name());
+ Assertions.assertEquals(newColumn.id(), selectedColumnId);
+ }
+
private void compareTwoColumns(
List<ColumnEntity> expectedColumns, List<ColumnEntity> actualColumns) {
Assertions.assertEquals(expectedColumns.size(), actualColumns.size());
