This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 61ee16ae63 [#11980] fix(core): sort column ordinal positions while
fetching from entity store (#11979)
61ee16ae63 is described below
commit 61ee16ae6367576200e9b2ff13c30fd1a627602e
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 =