mchades commented on code in PR #14684:
URL: https://github.com/apache/gravitino/pull/14684#discussion_r4488341284
##########
server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelSourceValidator.java:
##########
@@ -91,12 +99,101 @@ void
testCrossCatalogSourceAndKeysWithDisabledAuthorization() {
}
@Test
- void testViewFallbackAndColumns() {
+ void testManagedTableUsesIdentityFromLoadedProperties() {
+ Table managed = mock(Table.class);
+ doReturn(columns("id")).when(managed).columns();
+ when(managed.properties())
+
.thenReturn(StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(303L),
Map.of()));
+ EntityCombinedTable combined =
EntityCombinedTable.of(managed).withImported(true);
+ when(tables.loadTable(FULL_SOURCE)).thenReturn(combined);
+ SemanticModelSourceUpdate update =
+ validator.validate("metalake", definition(dataset("orders", null,
null)));
+ assertEquals(303L, update.sources().get(0).getSourceId());
+ assertEquals(EntityType.TABLE, update.sources().get(0).getSourceType());
+ verify(tables).loadTable(FULL_SOURCE);
+ verifyNoInteractions(views);
+
+ // An unregistered external table's connector property is not a registered
identity.
+ combined.withImported(false);
Review Comment:
`withImported` mutates `this.imported` and returns `this`. The loader mock
returns that same instance, so the second validation sees `imported() == false`
and exercises the unregistered-source rejection. No reassignment is needed.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelSourceValidator.java:
##########
@@ -190,6 +210,55 @@ private Set<String> loadColumns(
throw new IllegalSemanticModelException("Semantic Model source %s does not
exist", ident);
}
+ private static long registeredTableId(NameIdentifier ident, Table table) {
+ if (table instanceof EntityCombinedTable) {
+ EntityCombinedTable combined = (EntityCombinedTable) table;
+ if (combined.tableFromGravitino() != null) {
+ return combined.tableFromGravitino().id();
+ }
+ // Managed tables carry their stored ID in the loaded catalog
properties, without a separate
+ // TableEntity. Read it before public-property masking, without
resolving the name again.
+ if (combined.imported()) {
+ StringIdentifier id =
+
StringIdentifier.fromProperties(combined.tableFromCatalog().properties());
+ if (id != null) {
+ return id.id();
+ }
+ }
+ }
+ throw new IllegalStateException("Source table has no registered Gravitino
identity: " + ident);
+ }
+
+ private static long registeredViewId(NameIdentifier ident, View view) {
+ if (view instanceof EntityCombinedView) {
+ EntityCombinedView combined = (EntityCombinedView) view;
+ if (combined.viewEntity() != null) {
+ return combined.viewEntity().id();
+ }
+ // Managed views use the same stored-identity property as managed tables.
+ if (combined.imported()) {
+ StringIdentifier id =
+
StringIdentifier.fromProperties(combined.viewFromCatalog().properties());
+ if (id != null) {
+ return id.id();
+ }
+ }
+ }
+ throw new IllegalStateException("Source View has no registered Gravitino
identity: " + ident);
Review Comment:
Changed `Source View` to `Source view` to match the table error message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
