This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-61ee16ae-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 802d67b8365eb465ecdd5d75ed8d08cc79ac0793 Author: MehulBatra <[email protected]> AuthorDate: Mon Jul 13 14:14:37 2026 +0530 [#11980] fix(core): sort column ordinal positions while fetching from entity store (#11979) Closes: #11980 <!-- 1. Title: [#<issue>] <type>(<scope>): <subject> Examples: - "[#123] feat(operator): Support xxx" - "[#233] fix: Check null before access result in xxx" - "[MINOR] refactor: Fix typo in variable name" - "[MINOR] docs: Fix typo in README" - "[#255] test: Fix flaky test NameOfTheTest" Reference: https://www.conventionalcommits.org/en/v1.0.0/ 2. If the PR is unfinished, please mark this PR as draft. --> ### What changes were proposed in this pull request? Proposed fix: Sort by position in toGenericTable(): ``` .withColumns( tableEntity.columns().stream() .sorted(Comparator.comparingInt(ColumnEntity::position)) .map(this::toGenericColumn) .toArray(Column[]::new)) ``` column_position is already persisted correctly, so this fixes existing tables on read with no migration or re-registration. Comparator and ColumnEntity are already imported in the file. ### Why are the changes needed? ManagedTableOperations.toGenericTable() builds the column list from tableEntity.columns() without sorting by position, so loaded tables can return columns in an arbitrary, unstable order even though column_position is stored correctly. This breaks position-based consumers (e.g. the Lance REST catalog), causing wrong column mapping or crashes. The fix sorts columns by position on load, matching what the alter path already does. Fix: #11980 ### Does this PR introduce _any_ user-facing change? No API or property changes. Behavior fix only: loadTable/describe now always returns columns in their declared position order (previously they could come back unordered). No user-facing API changes, and no property keys added or removed. ### How was this patch tested? Before Fix: <img width="1803" height="606" alt="image" src="https://github.com/user-attachments/assets/34b7c62f-91e2-44d7-9a54-52a270ba8c9e"; /> After Fix: <img width="1395" height="430" alt="image" src="https://github.com/user-attachments/assets/26f4cfe6-7ac4-4719-bde5-d0fc868bdee8"; /> Without sorting: <img width="1395" height="523" alt="image" src="https://github.com/user-attachments/assets/0146aa9f-6918-497a-a83e-88f2a65e7f3d"; /> With sorting: <img width="1395" height="523" alt="image" src="https://github.com/user-attachments/assets/5e8f9c34-3d28-48fe-b7ff-6d272776afa9"; /> --- .../gravitino/catalog/ManagedTableOperations.java | 7 +++- .../catalog/TestManagedTableOperations.java | 42 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java index 39710b2cce..fe761e7962 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java +++ b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java @@ -472,7 +472,12 @@ public abstract class ManagedTableOperations implements TableCatalog { .withName(tableEntity.name()) .withComment(tableEntity.comment()) .withColumns( - tableEntity.columns().stream().map(this::toGenericColumn).toArray(Column[]::new)) + // Columns may be returned unordered from the store; sort by position + // so the table's column order matches what was declared. + tableEntity.columns().stream() + .sorted(Comparator.comparingInt(ColumnEntity::position)) + .map(this::toGenericColumn) + .toArray(Column[]::new)) .withProperties(tableEntity.properties()) .withAuditInfo(tableEntity.auditInfo()) .withSortOrders(tableEntity.sortOrders()) diff --git a/core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java b/core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java index 76e6698f5a..c660a40b67 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java @@ -18,8 +18,10 @@ */ package org.apache.gravitino.catalog; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -31,6 +33,9 @@ import org.apache.gravitino.connector.GenericColumn; import org.apache.gravitino.connector.SupportsSchemas; import org.apache.gravitino.exceptions.NoSuchTableException; import org.apache.gravitino.exceptions.TableAlreadyExistsException; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.ColumnEntity; +import org.apache.gravitino.meta.TableEntity; import org.apache.gravitino.rel.Column; import org.apache.gravitino.rel.Table; import org.apache.gravitino.rel.TableChange; @@ -213,6 +218,43 @@ public class TestManagedTableOperations { NoSuchTableException.class, () -> tableOperations.loadTable(nonExistingTableIdent)); } + @Test + public void testLoadTableSortsColumnsByPosition() throws Exception { + // Columns can be returned unordered from the store; loadTable must order them + // by position. Store a table whose column list order differs from the column + // positions, then verify the loaded columns come back in position order. + AuditInfo auditInfo = + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build(); + Column col1 = createColumn("col1", Types.StringType.get(), null); + Column col2 = createColumn("col2", Types.IntegerType.get(), null); + Column col3 = createColumn("col3", Types.StringType.get(), null); + + // List order [col3(pos 2), col1(pos 0), col2(pos 1)] - intentionally not by position. + List<ColumnEntity> unordered = + Arrays.asList( + ColumnEntity.toColumnEntity(col3, 2, idGenerator.nextId(), auditInfo), + ColumnEntity.toColumnEntity(col1, 0, idGenerator.nextId(), auditInfo), + ColumnEntity.toColumnEntity(col2, 1, idGenerator.nextId(), auditInfo)); + + NameIdentifier tableIdent = + NameIdentifierUtil.ofTable(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, "table_order"); + TableEntity tableEntity = + TableEntity.builder() + .withId(idGenerator.nextId()) + .withName("table_order") + .withNamespace(NamespaceUtil.ofTable(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME)) + .withColumns(unordered) + .withProperties(Collections.emptyMap()) + .withAuditInfo(auditInfo) + .build(); + store.put(tableEntity, false /* overwrite */); + + Table loaded = tableOperations.loadTable(tableIdent); + Assertions.assertArrayEquals( + new String[] {"col1", "col2", "col3"}, + Arrays.stream(loaded.columns()).map(Column::name).toArray(String[]::new)); + } + @Test public void testCreateAndDropTable() { NameIdentifier table1Ident =
