From 220a730ba8e12afa617617d346921870358345b0 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Fri, 20 Mar 2026 00:42:45 +0000 Subject: [PATCH 01/26] Add metric view support to UC server - Add view_dependencies field to CreateTable and TableInfo in OpenAPI spec - Add dependent parameter to GenerateTemporaryTableCredential for view-mediated authorization (definer's rights model) - Create DependencyDAO and DependencyRepository for persisting view dependencies in uc_dependencies table - Update TableRepository to handle METRIC_VIEW table type with dependency storage/retrieval on create, get, and delete - Update TemporaryTableCredentialsService to support view-mediated credential vending via the dependent parameter - Make AuthorizeKey repeatable for multiple authorization parameters - Add BaseMetricViewCRUDTest and SdkMetricViewCRUDTest for metric view CRUD integration testing --- api/all.yaml | 19 ++- .../server/UnityCatalogServer.java | 2 +- .../server/auth/annotation/AuthorizeKey.java | 2 + .../server/auth/annotation/AuthorizeKeys.java | 13 ++ .../server/persist/DependencyRepository.java | 56 +++++++ .../server/persist/Repositories.java | 2 + .../server/persist/TableRepository.java | 75 +++++++-- .../server/persist/dao/DependencyDAO.java | 118 ++++++++++++++ .../server/persist/dao/TableInfoDAO.java | 15 +- .../persist/utils/HibernateConfigurator.java | 2 + .../TemporaryTableCredentialsService.java | 150 ++++++++++++++++-- .../base/table/BaseMetricViewCRUDTest.java | 130 +++++++++++++++ .../sdk/tables/SdkMetricViewCRUDTest.java | 28 ++++ 13 files changed, 583 insertions(+), 29 deletions(-) create mode 100644 server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java create mode 100644 server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java create mode 100644 server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java create mode 100644 server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/sdk/tables/SdkMetricViewCRUDTest.java diff --git a/api/all.yaml b/api/all.yaml index 9e4f95268d..8fe7e2dab5 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -1678,6 +1678,7 @@ components: - EXTERNAL - STREAMING_TABLE # Not yet fully implemented - MATERIALIZED_VIEW # Not yet fully implemented + - METRIC_VIEW DataSourceFormat: description: Data source format type: string @@ -1738,6 +1739,11 @@ components: table_id: description: Unique identifier for the table. type: string + view_definition: + description: SQL query that defines the metric view (for METRIC_VIEW table type). + type: string + view_dependencies: + "$ref": "#/components/schemas/DependencyList" CreateTable: type: object properties: @@ -1767,14 +1773,17 @@ components: type: string properties: $ref: '#/components/schemas/SecurablePropertiesMap' + view_definition: + description: SQL query that defines the metric view (for METRIC_VIEW table type). + type: string + view_dependencies: + "$ref": "#/components/schemas/DependencyList" required: - name - catalog_name - schema_name - table_type - - data_source_format - columns - - storage_location StagingTableInfo: type: object properties: @@ -2358,6 +2367,12 @@ components: type: string operation: $ref: '#/components/schemas/TableOperation' + dependent: + description: | + Optional. Table ID of the view or metric view through which + this table is being accessed. When provided, the server performs + view-mediated authorization instead of direct authorization. + type: string type: object required: - table_id diff --git a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java index 923c3f98b3..76dfc3fef4 100644 --- a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java +++ b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java @@ -177,7 +177,7 @@ private void addApiServices( MetastoreService metastoreService = new MetastoreService(repositories); // TODO: combine these into a single service in a follow-up PR TemporaryTableCredentialsService temporaryTableCredentialsService = - new TemporaryTableCredentialsService(storageCredentialVendor, repositories); + new TemporaryTableCredentialsService(storageCredentialVendor, authorizer, repositories); TemporaryVolumeCredentialsService temporaryVolumeCredentialsService = new TemporaryVolumeCredentialsService(storageCredentialVendor, repositories); TemporaryModelVersionCredentialsService temporaryModelVersionCredentialsService = diff --git a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java index 8bde310f85..74bd390eee 100644 --- a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java +++ b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java @@ -1,6 +1,7 @@ package io.unitycatalog.server.auth.annotation; import java.lang.annotation.ElementType; +import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @@ -56,6 +57,7 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.PARAMETER) +@Repeatable(AuthorizeKeys.class) public @interface AuthorizeKey { /** * The key path to extract from the request payload. Supports nested fields using dot notation diff --git a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java new file mode 100644 index 0000000000..3256cb526b --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java @@ -0,0 +1,13 @@ +package io.unitycatalog.server.auth.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Container annotation for repeatable {@link AuthorizeKey} annotations. */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.PARAMETER) +public @interface AuthorizeKeys { + AuthorizeKey[] value(); +} diff --git a/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java b/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java new file mode 100644 index 0000000000..0cb33b8b93 --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java @@ -0,0 +1,56 @@ +package io.unitycatalog.server.persist; + +import io.unitycatalog.server.persist.dao.DependencyDAO; +import java.util.List; +import java.util.UUID; +import org.hibernate.Session; +import org.hibernate.query.Query; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Repository for managing view/metric-view dependency records in uc_dependencies. Methods accept a + * Session so they can participate in the caller's transaction. + */ +public class DependencyRepository { + private static final Logger LOGGER = LoggerFactory.getLogger(DependencyRepository.class); + + public void createDependencies( + Session session, UUID dependentId, String dependentType, List dependencies) { + for (DependencyDAO dep : dependencies) { + dep.setDependentId(dependentId); + dep.setDependentType(dependentType); + session.persist(dep); + } + LOGGER.debug( + "Created {} dependencies for {}:{}", dependencies.size(), dependentType, dependentId); + } + + public List getDependencies( + Session session, UUID dependentId, String dependentType) { + String hql = + "FROM DependencyDAO d WHERE d.dependentId = :dependentId" + + " AND d.dependentType = :dependentType"; + Query query = session.createQuery(hql, DependencyDAO.class); + query.setParameter("dependentId", dependentId); + query.setParameter("dependentType", dependentType); + return query.list(); + } + + public void deleteDependencies(Session session, UUID dependentId, String dependentType) { + String hql = + "DELETE FROM DependencyDAO d WHERE d.dependentId = :dependentId" + + " AND d.dependentType = :dependentType"; + Query query = session.createQuery(hql); + query.setParameter("dependentId", dependentId); + query.setParameter("dependentType", dependentType); + int deleted = query.executeUpdate(); + LOGGER.debug("Deleted {} dependencies for {}:{}", deleted, dependentType, dependentId); + } + + public void updateDependencies( + Session session, UUID dependentId, String dependentType, List dependencies) { + deleteDependencies(session, dependentId, dependentType); + createDependencies(session, dependentId, dependentType, dependencies); + } +} diff --git a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java index 7222450e5b..ac4e6f4964 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java +++ b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java @@ -29,6 +29,7 @@ public class Repositories { private final CredentialRepository credentialRepository; private final ExternalLocationRepository externalLocationRepository; private final DeltaCommitRepository deltaCommitRepository; + private final DependencyRepository dependencyRepository; private final KeyMapper keyMapper; @@ -50,6 +51,7 @@ public Repositories(SessionFactory sessionFactory, ServerProperties serverProper this.credentialRepository = new CredentialRepository(this, sessionFactory, serverProperties); this.externalLocationRepository = new ExternalLocationRepository(this, sessionFactory); this.deltaCommitRepository = new DeltaCommitRepository(sessionFactory, serverProperties); + this.dependencyRepository = new DependencyRepository(); // KeyMapper uses all the repositories above. this.keyMapper = new KeyMapper(this); diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index cf30f20efb..6d79efa5cd 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -5,9 +5,11 @@ import io.unitycatalog.server.model.ColumnInfo; import io.unitycatalog.server.model.CreateTable; import io.unitycatalog.server.model.DataSourceFormat; +import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.model.TableType; +import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.PropertyDAO; import io.unitycatalog.server.persist.dao.SchemaInfoDAO; import io.unitycatalog.server.persist.dao.StagingTableDAO; @@ -151,6 +153,16 @@ public TableInfo getTable(String fullName) { TableInfo tableInfo = tableInfoDAO.toTableInfo(true, catalogName, schemaName); RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); + if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { + List deps = + repositories + .getDependencyRepository() + .getDependencies(session, tableInfoDAO.getId(), "TABLE"); + if (!deps.isEmpty()) { + tableInfo.setViewDependencies( + new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); + } + } return tableInfo; }, "Failed to get table", @@ -174,9 +186,11 @@ public TableInfo createTable(CreateTable createTable) { ValidationUtils.validateSqlObjectName(createTable.getName()); String callerId = IdentityUtils.findPrincipalEmailAddress(); List columnInfos = - createTable.getColumns().stream() - .map(c -> c.typeText(c.getTypeText().toLowerCase(Locale.ROOT))) - .collect(Collectors.toList()); + createTable.getColumns() != null + ? createTable.getColumns().stream() + .map(c -> c.typeText(c.getTypeText().toLowerCase(Locale.ROOT))) + .collect(Collectors.toList()) + : List.of(); Long createTime = System.currentTimeMillis(); String fullName = getTableFullName(createTable); LOGGER.debug("Creating table: {}", fullName); @@ -190,7 +204,6 @@ public TableInfo createTable(CreateTable createTable) { repositories .getSchemaRepository() .getSchemaIdOrThrow(session, catalogName, schemaName); - NormalizedURL storageLocation = NormalizedURL.from(createTable.getStorageLocation()); // Check if table already exists TableInfoDAO existingTable = @@ -200,13 +213,14 @@ public TableInfo createTable(CreateTable createTable) { ErrorCode.TABLE_ALREADY_EXISTS, "Table already exists: " + fullName); } TableType tableType = Objects.requireNonNull(createTable.getTableType()); - // The table ID will either be a new random one or the id of staging table, depending - // on the type of table to create. String tableID; + NormalizedURL storageLocation; if (tableType == TableType.EXTERNAL) { + storageLocation = NormalizedURL.from(createTable.getStorageLocation()); ExternalLocationUtils.validateNotOverlapWithManagedStorage(session, storageLocation); tableID = UUID.randomUUID().toString(); } else if (tableType == TableType.MANAGED) { + storageLocation = NormalizedURL.from(createTable.getStorageLocation()); serverProperties.checkManagedTableEnabled(); if (createTable.getDataSourceFormat() != DataSourceFormat.DELTA) { throw new BaseException( @@ -219,6 +233,26 @@ public TableInfo createTable(CreateTable createTable) { .getStagingTableRepository() .commitStagingTable(session, callerId, storageLocation); tableID = stagingTableDAO.getId().toString(); + } else if (tableType == TableType.METRIC_VIEW) { + if (createTable.getViewDefinition() == null + || createTable.getViewDefinition().isEmpty()) { + throw new BaseException( + ErrorCode.INVALID_ARGUMENT, "view_definition is required for metric view"); + } + storageLocation = null; + tableID = UUID.randomUUID().toString(); + // Persist view dependencies + DependencyList viewDeps = createTable.getViewDependencies(); + if (viewDeps != null && viewDeps.getDependencies() != null) { + UUID tableUUID = UUID.fromString(tableID); + List depDAOs = + viewDeps.getDependencies().stream() + .map(dep -> DependencyDAO.from(dep, tableUUID, "TABLE")) + .collect(Collectors.toList()); + repositories + .getDependencyRepository() + .createDependencies(session, tableUUID, "TABLE", depDAOs); + } } else if (tableType == TableType.STREAMING_TABLE) { throw new BaseException( ErrorCode.INVALID_ARGUMENT, "STREAMING TABLE creation is not supported yet."); @@ -245,18 +279,21 @@ public TableInfo createTable(CreateTable createTable) { .createdBy(callerId) .updatedAt(createTime) .updatedBy(callerId) - .storageLocation(storageLocation.toString()) + .storageLocation(storageLocation != null ? storageLocation.toString() : null) + .viewDefinition(createTable.getViewDefinition()) .tableId(tableID); TableInfoDAO tableInfoDAO = TableInfoDAO.from(tableInfo, schemaId); // create columns - tableInfoDAO - .getColumns() - .forEach( - c -> { - c.setId(UUID.randomUUID()); - c.setTable(tableInfoDAO); - }); + if (tableInfoDAO.getColumns() != null) { + tableInfoDAO + .getColumns() + .forEach( + c -> { + c.setId(UUID.randomUUID()); + c.setTable(tableInfoDAO); + }); + } // create properties PropertyDAO.from(tableInfo.getProperties(), tableInfoDAO.getId(), Constants.TABLE) .forEach(session::persist); @@ -267,6 +304,11 @@ public TableInfo createTable(CreateTable createTable) { /* readOnly = */ false); } + /** Retrieves a TableInfoDAO by its ID within an existing session/transaction. */ + public TableInfoDAO getTableById(Session session, UUID tableId) { + return session.get(TableInfoDAO.class, tableId); + } + public TableInfoDAO findBySchemaIdAndName(Session session, UUID schemaId, String name) { String hql = "FROM TableInfoDAO t WHERE t.schemaId = :schemaId AND t.name = :name"; Query query = session.createQuery(hql, TableInfoDAO.class); @@ -384,6 +426,11 @@ public void deleteTable(Session session, UUID schemaId, String tableName) { .getDeltaCommitRepository() .permanentlyDeleteTableCommits(session, tableInfoDAO.getId()); } + if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { + repositories + .getDependencyRepository() + .deleteDependencies(session, tableInfoDAO.getId(), "TABLE"); + } PropertyRepository.findProperties(session, tableInfoDAO.getId(), Constants.TABLE) .forEach(session::remove); session.remove(tableInfoDAO); diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java new file mode 100644 index 0000000000..b1872bad05 --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java @@ -0,0 +1,118 @@ +package io.unitycatalog.server.persist.dao; + +import io.unitycatalog.server.model.Dependency; +import io.unitycatalog.server.model.TableDependency; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; +import org.hibernate.annotations.UuidGenerator; + +@Entity +@Table( + name = "uc_dependencies", + indexes = { + @Index(name = "idx_dependent", columnList = "dependent_type,dependent_id"), + }) +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@EqualsAndHashCode +@ToString +@Builder +public class DependencyDAO { + @Id + @UuidGenerator + @Column(name = "id", updatable = false, nullable = false) + private UUID id; + + @Column(name = "dependent_type", nullable = false) + private String dependentType; + + @Column(name = "dependent_id", nullable = false) + private UUID dependentId; + + @Column(name = "dependency_type", nullable = false) + private String dependencyType; + + @Column(name = "dependency_catalog") + private String dependencyCatalog; + + @Column(name = "dependency_schema") + private String dependencySchema; + + @Column(name = "dependency_name") + private String dependencyName; + + @Column(name = "dependency_target_id") + private UUID dependencyTargetId; + + /** Converts a Dependency API model to a DependencyDAO for a given dependent. */ + public static DependencyDAO from(Dependency dependency, UUID dependentId, String dependentType) { + DependencyDAOBuilder builder = + DependencyDAO.builder().dependentId(dependentId).dependentType(dependentType); + + if (dependency.getTable() != null) { + builder.dependencyType("TABLE"); + String fullName = dependency.getTable().getTableFullName(); + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + builder.dependencyCatalog(parts[0]); + builder.dependencySchema(parts[1]); + builder.dependencyName(parts[2]); + } else { + builder.dependencyName(fullName); + } + } else if (dependency.getFunction() != null) { + builder.dependencyType("FUNCTION"); + String fullName = dependency.getFunction().getFunctionFullName(); + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + builder.dependencyCatalog(parts[0]); + builder.dependencySchema(parts[1]); + builder.dependencyName(parts[2]); + } else { + builder.dependencyName(fullName); + } + } + + return builder.build(); + } + + /** Converts this DAO to a Dependency API model. */ + public Dependency toDependency() { + Dependency dependency = new Dependency(); + if ("TABLE".equals(dependencyType)) { + String fullName = dependencyCatalog + "." + dependencySchema + "." + dependencyName; + dependency.setTable(new TableDependency().tableFullName(fullName)); + } + return dependency; + } + + public String getDependencyFullName() { + if (dependencyCatalog != null && dependencySchema != null && dependencyName != null) { + return dependencyCatalog + "." + dependencySchema + "." + dependencyName; + } + return dependencyName; + } + + public static List toDependencyList(List daos) { + if (daos == null) { + return new ArrayList<>(); + } + return daos.stream().map(DependencyDAO::toDependency).collect(Collectors.toList()); + } +} diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/TableInfoDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/TableInfoDAO.java index 7734f116e5..e630b26f7f 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/dao/TableInfoDAO.java +++ b/server/src/main/java/io/unitycatalog/server/persist/dao/TableInfoDAO.java @@ -76,6 +76,9 @@ public class TableInfoDAO extends IdentifiableDAO { fetch = FetchType.LAZY) private List columns; + @Column(name = "view_definition", length = 65535) + private String viewDefinition; + @Column(name = "uniform_iceberg_metadata_location", length = 65535) private String uniformIcebergMetadataLocation; @@ -98,8 +101,12 @@ public static TableInfoDAO from(TableInfo tableInfo, UUID schemaId) { .updatedBy(tableInfo.getUpdatedBy()) .columnCount(tableInfo.getColumns() != null ? tableInfo.getColumns().size() : 0) .type(tableInfo.getTableType().toString()) - .dataSourceFormat(tableInfo.getDataSourceFormat().toString()) + .dataSourceFormat( + tableInfo.getDataSourceFormat() != null + ? tableInfo.getDataSourceFormat().toString() + : null) .url(tableInfo.getStorageLocation()) + .viewDefinition(tableInfo.getViewDefinition()) .columns(ColumnInfoDAO.fromList(tableInfo.getColumns())) .schemaId(schemaId) .build(); @@ -113,8 +120,10 @@ public TableInfo toTableInfo(boolean fetchColumns, String catalogName, String sc .catalogName(catalogName) .schemaName(schemaName) .tableType(TableType.valueOf(type)) - .dataSourceFormat(DataSourceFormat.valueOf(dataSourceFormat)) - .storageLocation(NormalizedURL.normalize(url)) + .dataSourceFormat( + dataSourceFormat != null ? DataSourceFormat.valueOf(dataSourceFormat) : null) + .storageLocation(url != null ? NormalizedURL.normalize(url) : null) + .viewDefinition(viewDefinition) .comment(comment) .owner(owner) .createdAt(createdAt != null ? createdAt.getTime() : null) diff --git a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java index e3ee6672a3..486828bd5a 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java +++ b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java @@ -4,6 +4,7 @@ import io.unitycatalog.server.persist.dao.ColumnInfoDAO; import io.unitycatalog.server.persist.dao.CredentialDAO; import io.unitycatalog.server.persist.dao.DeltaCommitDAO; +import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.ExternalLocationDAO; import io.unitycatalog.server.persist.dao.FunctionInfoDAO; import io.unitycatalog.server.persist.dao.FunctionParameterInfoDAO; @@ -71,6 +72,7 @@ private static SessionFactory createSessionFactory(Properties hibernatePropertie configuration.addAnnotatedClass(CredentialDAO.class); configuration.addAnnotatedClass(ExternalLocationDAO.class); configuration.addAnnotatedClass(DeltaCommitDAO.class); + configuration.addAnnotatedClass(DependencyDAO.class); ServiceRegistry serviceRegistry = new StandardServiceRegistryBuilder().applySettings(configuration.getProperties()).build(); diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index 170321e7cc..1ec52f2cba 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -3,55 +3,187 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.server.annotation.ExceptionHandler; import com.linecorp.armeria.server.annotation.Post; +import io.unitycatalog.server.auth.UnityCatalogAuthorizer; import io.unitycatalog.server.auth.annotation.AuthorizeExpression; import io.unitycatalog.server.auth.annotation.AuthorizeResourceKey; import io.unitycatalog.server.auth.annotation.AuthorizeKey; +import io.unitycatalog.server.exception.BaseException; +import io.unitycatalog.server.exception.ErrorCode; import io.unitycatalog.server.exception.GlobalExceptionHandler; import io.unitycatalog.server.model.GenerateTemporaryTableCredential; import io.unitycatalog.server.model.TableOperation; +import io.unitycatalog.server.persist.DependencyRepository; import io.unitycatalog.server.persist.Repositories; +import io.unitycatalog.server.persist.SchemaRepository; import io.unitycatalog.server.persist.TableRepository; +import io.unitycatalog.server.persist.UserRepository; +import io.unitycatalog.server.persist.dao.DependencyDAO; +import io.unitycatalog.server.persist.dao.TableInfoDAO; +import io.unitycatalog.server.persist.model.Privileges; +import io.unitycatalog.server.persist.utils.TransactionManager; import io.unitycatalog.server.service.credential.CredentialContext; import io.unitycatalog.server.service.credential.StorageCredentialVendor; import io.unitycatalog.server.utils.NormalizedURL; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.UUID; +import org.hibernate.SessionFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import static io.unitycatalog.server.model.SecurableType.TABLE; import static io.unitycatalog.server.service.credential.CredentialContext.Privilege.SELECT; import static io.unitycatalog.server.service.credential.CredentialContext.Privilege.UPDATE; @ExceptionHandler(GlobalExceptionHandler.class) public class TemporaryTableCredentialsService { + private static final Logger LOGGER = + LoggerFactory.getLogger(TemporaryTableCredentialsService.class); + private final TableRepository tableRepository; + private final DependencyRepository dependencyRepository; + private final SchemaRepository schemaRepository; + private final UserRepository userRepository; private final StorageCredentialVendor storageCredentialVendor; + private final UnityCatalogAuthorizer authorizer; + private final SessionFactory sessionFactory; - public TemporaryTableCredentialsService(StorageCredentialVendor storageCredentialVendor, - Repositories repositories) { + public TemporaryTableCredentialsService( + StorageCredentialVendor storageCredentialVendor, + UnityCatalogAuthorizer authorizer, + Repositories repositories) { this.storageCredentialVendor = storageCredentialVendor; + this.authorizer = authorizer; this.tableRepository = repositories.getTableRepository(); + this.dependencyRepository = repositories.getDependencyRepository(); + this.schemaRepository = repositories.getSchemaRepository(); + this.userRepository = repositories.getUserRepository(); + this.sessionFactory = repositories.getSessionFactory(); } @Post("") @AuthorizeExpression(""" - #authorizeAny(#principal, #schema, OWNER, USE_SCHEMA) && - #authorizeAny(#principal, #catalog, OWNER, USE_CATALOG) && - (#operation == 'READ' - ? #authorizeAny(#principal, #table, OWNER, SELECT) - : (#authorize(#principal, #table, OWNER) || - #authorizeAll(#principal, #table, SELECT, MODIFY))) + #dependent != null || ( + #authorizeAny(#principal, #schema, OWNER, USE_SCHEMA) && + #authorizeAny(#principal, #catalog, OWNER, USE_CATALOG) && + (#operation == 'READ' + ? #authorizeAny(#principal, #table, OWNER, SELECT) + : (#authorize(#principal, #table, OWNER) || + #authorizeAll(#principal, #table, SELECT, MODIFY))) + ) """) public HttpResponse generateTemporaryTableCredential( @AuthorizeResourceKey(value = TABLE, key = "table_id") @AuthorizeKey(key = "operation") + @AuthorizeKey(key = "dependent") GenerateTemporaryTableCredential generateTemporaryTableCredential) { String tableId = generateTemporaryTableCredential.getTableId(); + String dependentId = generateTemporaryTableCredential.getDependent(); + + if (dependentId != null) { + return handleDependentCredentialRequest( + tableId, dependentId, generateTemporaryTableCredential.getOperation()); + } + NormalizedURL storageLocation = tableRepository.getStorageLocationForTableOrStagingTable( UUID.fromString(tableId)); return HttpResponse.ofJson(storageCredentialVendor.vendCredential(storageLocation, - tableOperationToPrivileges(generateTemporaryTableCredential.getOperation()))); + tableOperationToPrivileges(generateTemporaryTableCredential.getOperation()))); + } + + /** + * Handles credential requests that come through a view/metric-view (definer's-rights model). + * + * Authorization checks: + * 1. Current user has SELECT on the dependent (view/metric view) + * 2. The requested table_id is in the dependent's dependency list + * 3. The dependent's owner has SELECT on the source table + */ + private HttpResponse handleDependentCredentialRequest( + String sourceTableId, String dependentId, TableOperation operation) { + return TransactionManager.executeWithTransaction( + sessionFactory, + session -> { + UUID dependentUUID = UUID.fromString(dependentId); + UUID sourceTableUUID = UUID.fromString(sourceTableId); + + TableInfoDAO dependentTable = tableRepository.getTableById(session, dependentUUID); + if (dependentTable == null) { + throw new BaseException( + ErrorCode.NOT_FOUND, "Dependent table not found: " + dependentId); + } + + UUID principalId = userRepository.findPrincipalId(); + if (principalId != null) { + boolean userCanSelectView = authorizer.authorizeAny( + principalId, dependentUUID, Privileges.OWNER, Privileges.SELECT); + if (!userCanSelectView) { + throw new BaseException( + ErrorCode.PERMISSION_DENIED, + "User does not have SELECT permission on the metric view"); + } + } + + List deps = + dependencyRepository.getDependencies(session, dependentUUID, "TABLE"); + boolean sourceInDeps = deps.stream().anyMatch(dep -> { + if (dep.getDependencyTargetId() != null) { + return dep.getDependencyTargetId().equals(sourceTableUUID); + } + String fullName = dep.getDependencyFullName(); + if (fullName != null) { + try { + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + UUID schemaId = schemaRepository.getSchemaIdOrThrow( + session, parts[0], parts[1]); + TableInfoDAO resolved = tableRepository.findBySchemaIdAndName( + session, schemaId, parts[2]); + return resolved != null && resolved.getId().equals(sourceTableUUID); + } + } catch (Exception e) { + LOGGER.warn("Failed to resolve dependency: {}", fullName, e); + } + } + return false; + }); + + if (!sourceInDeps) { + throw new BaseException( + ErrorCode.PERMISSION_DENIED, + "Source table is not a dependency of the specified metric view"); + } + + String viewOwner = dependentTable.getOwner(); + if (viewOwner != null && principalId != null) { + try { + UUID ownerPrincipalId = UUID.fromString( + userRepository.getUserByEmail(viewOwner).getId()); + boolean ownerCanSelectSource = authorizer.authorizeAny( + ownerPrincipalId, sourceTableUUID, Privileges.OWNER, Privileges.SELECT); + if (!ownerCanSelectSource) { + throw new BaseException( + ErrorCode.PERMISSION_DENIED, + "Metric view owner does not have SELECT permission on the source table"); + } + } catch (BaseException e) { + if (e.getErrorCode() == ErrorCode.PERMISSION_DENIED) { + throw e; + } + LOGGER.warn("Could not resolve view owner for auth check: {}", viewOwner); + } + } + + NormalizedURL storageLocation = tableRepository + .getStorageLocationForTableOrStagingTable(sourceTableUUID); + return HttpResponse.ofJson(storageCredentialVendor.vendCredential( + storageLocation, tableOperationToPrivileges(operation))); + }, + "Failed to handle dependent credential request", + /* readOnly = */ true); } private Set tableOperationToPrivileges( diff --git a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java new file mode 100644 index 0000000000..4c4b4f5b32 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java @@ -0,0 +1,130 @@ +package io.unitycatalog.server.base.table; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.unitycatalog.client.model.CreateTable; +import io.unitycatalog.client.model.Dependency; +import io.unitycatalog.client.model.DependencyList; +import io.unitycatalog.client.model.TableDependency; +import io.unitycatalog.client.model.TableInfo; +import io.unitycatalog.client.model.TableType; +import io.unitycatalog.server.utils.TestUtils; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +public abstract class BaseMetricViewCRUDTest extends BaseTableCRUDTestEnv { + + protected static final String METRIC_VIEW_NAME = "uc_test_metric_view"; + protected static final String METRIC_VIEW_FULL_NAME = + TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + METRIC_VIEW_NAME; + protected static final String VIEW_DEFINITION = + "SELECT date_trunc('day', event_time) AS day, count(*) AS event_count FROM events GROUP BY 1"; + protected static final Map PROPERTIES = + Map.of("team", "analytics", "refresh", "daily"); + + @Test + public void testMetricViewCRUD() throws Exception { + assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) + .isInstanceOf(Exception.class); + + // --- Create --- + CreateTable createRequest = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDefinition(VIEW_DEFINITION) + .comment("Daily event counts by day") + .properties(PROPERTIES); + + TableInfo created = tableOperations.createTable(createRequest); + assertThat(created.getName()).isEqualTo(METRIC_VIEW_NAME); + assertThat(created.getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); + assertThat(created.getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); + assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); + assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(created.getTableId()).isNotNull(); + assertThat(created.getStorageLocation()) + .as("Metric views should have no storage location") + .isNull(); + + // --- Get --- + TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); + assertThat(fetched.getName()).isEqualTo(METRIC_VIEW_NAME); + assertThat(fetched.getTableType()).isEqualTo(TableType.METRIC_VIEW); + assertThat(fetched.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(fetched.getComment()).isEqualTo("Daily event counts by day"); + assertThat(fetched.getCreatedAt()).isNotNull(); + assertThat(fetched.getTableId()).isNotNull(); + + // Verify properties round-trip + assertThat(fetched.getProperties()).isNotNull(); + assertThat(fetched.getProperties().get("team")).isEqualTo("analytics"); + assertThat(fetched.getProperties().get("refresh")).isEqualTo("daily"); + + // --- List --- + List tables = + tableOperations.listTables(TestUtils.CATALOG_NAME, TestUtils.SCHEMA_NAME, Optional.empty()); + assertThat(tables) + .as("Metric view should appear in listTables") + .anyMatch( + t -> + METRIC_VIEW_NAME.equals(t.getName()) + && TableType.METRIC_VIEW.equals(t.getTableType())); + + // --- Create without view_definition should fail --- + CreateTable badRequest = + new CreateTable() + .name("bad_metric_view") + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW); + assertThatThrownBy(() -> tableOperations.createTable(badRequest)).isInstanceOf(Exception.class); + + // --- Delete --- + tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); + assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) + .isInstanceOf(Exception.class); + } + + @Test + public void testMetricViewWithDependencies() throws Exception { + String sourceTableFullName = + TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".source_events"; + + Dependency dep = new Dependency(); + dep.setTable(new TableDependency().tableFullName(sourceTableFullName)); + DependencyList depList = new DependencyList(); + depList.setDependencies(List.of(dep)); + + CreateTable createRequest = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDefinition(VIEW_DEFINITION) + .viewDependencies(depList) + .comment("Metric view with dependencies"); + + TableInfo created = tableOperations.createTable(createRequest); + assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); + assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + + // Verify dependencies are returned on GET + TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); + assertThat(fetched.getViewDependencies()).isNotNull(); + assertThat(fetched.getViewDependencies().getDependencies()).hasSize(1); + assertThat(fetched.getViewDependencies().getDependencies().get(0).getTable().getTableFullName()) + .isEqualTo(sourceTableFullName); + + // Delete and verify dependencies are cleaned up + tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); + assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) + .isInstanceOf(Exception.class); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/sdk/tables/SdkMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/sdk/tables/SdkMetricViewCRUDTest.java new file mode 100644 index 0000000000..728611b9d9 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/sdk/tables/SdkMetricViewCRUDTest.java @@ -0,0 +1,28 @@ +package io.unitycatalog.server.sdk.tables; + +import io.unitycatalog.server.base.ServerConfig; +import io.unitycatalog.server.base.catalog.CatalogOperations; +import io.unitycatalog.server.base.schema.SchemaOperations; +import io.unitycatalog.server.base.table.BaseMetricViewCRUDTest; +import io.unitycatalog.server.base.table.TableOperations; +import io.unitycatalog.server.sdk.catalog.SdkCatalogOperations; +import io.unitycatalog.server.sdk.schema.SdkSchemaOperations; +import io.unitycatalog.server.utils.TestUtils; + +public class SdkMetricViewCRUDTest extends BaseMetricViewCRUDTest { + + @Override + protected CatalogOperations createCatalogOperations(ServerConfig config) { + return new SdkCatalogOperations(TestUtils.createApiClient(config)); + } + + @Override + protected SchemaOperations createSchemaOperations(ServerConfig config) { + return new SdkSchemaOperations(TestUtils.createApiClient(config)); + } + + @Override + protected TableOperations createTableOperations(ServerConfig config) { + return new SdkTableOperations(TestUtils.createApiClient(config)); + } +} From 665fb83440978e98680d8f1720032e388eb76c5d Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Fri, 20 Mar 2026 00:43:07 +0000 Subject: [PATCH 02/26] Add metric view support to UC Spark connector - Add MetricViewContext thread-local for passing metric view identity during credential vending (definer's rights model) - Handle METRIC_VIEW table type in UCSingleCatalog.createTable to route metric view creation through UCProxy with proper metadata - Add createMetricView to UCProxy for constructing METRIC_VIEW CreateTable requests with view_definition, schema, and dependency properties - Update build.sbt to support building against Spark 4.2.0-SNAPSHOT with unmanaged JARs from assembly directory - Fix Guava import references: replace org.sparkproject.guava with com.google.common across connector Java files to support Spark 4.2 where Guava is no longer shaded --- build.sbt | 4 + .../spark/auth/CredPropsUtil.java | 4 +- .../auth/storage/AwsVendedTokenProvider.java | 2 +- .../auth/storage/GcsVendedTokenProvider.java | 2 +- .../storage/GenericCredentialProvider.java | 6 +- .../spark/fs/CredScopedFileSystem.java | 4 +- .../spark/MetricViewContext.scala | 19 +++ .../unitycatalog/spark/UCSingleCatalog.scala | 122 +++++++++++++++--- .../spark/AzureCredentialTestFileSystem.java | 2 +- .../auth/storage/BaseCredRenewITTest.java | 4 +- 10 files changed, 140 insertions(+), 29 deletions(-) create mode 100644 connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala diff --git a/build.sbt b/build.sbt index 9cafdf9ecf..9c12409aaa 100644 --- a/build.sbt +++ b/build.sbt @@ -594,6 +594,10 @@ lazy val spark = (project in file("connectors/spark")) lombokPath ) }, + Compile / unmanagedJars ++= { + val sparkAssemblyDir = sys.props.get("sparkAssemblyDir").map(file).filter(_.exists) + sparkAssemblyDir.toSeq.flatMap(d => (d ** "*.jar").get.classpath) + }, libraryDependencies ++= Seq( "org.apache.spark" %% "spark-sql" % sparkVersion % Provided, "com.fasterxml.jackson.core" % "jackson-databind" % "2.15.0", diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java index 20dbdd0e3d..f707d0daff 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java @@ -4,6 +4,8 @@ import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_ACCOUNT_IS_HNS_ENABLED; import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; import io.unitycatalog.client.auth.TokenProvider; import io.unitycatalog.client.model.AwsCredentials; import io.unitycatalog.client.model.AzureUserDelegationSAS; @@ -19,8 +21,6 @@ import io.unitycatalog.spark.fs.CredScopedFs; import java.util.Map; import java.util.UUID; -import org.sparkproject.guava.base.Preconditions; -import org.sparkproject.guava.collect.ImmutableMap; public class CredPropsUtil { private CredPropsUtil() {} diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java index 4b461527b9..b0e3481b88 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java @@ -1,8 +1,8 @@ package io.unitycatalog.spark.auth.storage; +import com.google.common.base.Preconditions; import io.unitycatalog.spark.UCHadoopConf; import org.apache.hadoop.conf.Configuration; -import org.sparkproject.guava.base.Preconditions; import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java index 99ea6621a3..357f45628f 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java @@ -1,12 +1,12 @@ package io.unitycatalog.spark.auth.storage; import com.google.cloud.hadoop.util.AccessTokenProvider; +import com.google.common.base.Preconditions; import io.unitycatalog.client.model.GcpOauthToken; import io.unitycatalog.spark.UCHadoopConf; import java.io.IOException; import java.time.Instant; import org.apache.hadoop.conf.Configuration; -import org.sparkproject.guava.base.Preconditions; public class GcsVendedTokenProvider extends GenericCredentialProvider implements AccessTokenProvider { diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java index 2ac23ca98d..28dd643aed 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java @@ -1,5 +1,8 @@ package io.unitycatalog.spark.auth.storage; +import com.google.common.base.Preconditions; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import io.unitycatalog.client.ApiException; import io.unitycatalog.client.api.TemporaryCredentialsApi; import io.unitycatalog.client.auth.TokenProvider; @@ -14,9 +17,6 @@ import io.unitycatalog.spark.UCHadoopConf; import java.net.URI; import org.apache.hadoop.conf.Configuration; -import org.sparkproject.guava.base.Preconditions; -import org.sparkproject.guava.cache.Cache; -import org.sparkproject.guava.cache.CacheBuilder; public abstract class GenericCredentialProvider { // The credential cache, for saving QPS to unity catalog server. diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java b/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java index 481f08cd79..c5240c33d4 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java @@ -1,13 +1,13 @@ package io.unitycatalog.spark.fs; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import java.io.IOException; import java.net.URI; import java.util.concurrent.ExecutionException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FilterFileSystem; -import org.sparkproject.guava.cache.Cache; -import org.sparkproject.guava.cache.CacheBuilder; /** * A Hadoop {@link FileSystem} wrapper that enables multiple credential scopes to coexist within a diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala new file mode 100644 index 0000000000..c24f4aac13 --- /dev/null +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala @@ -0,0 +1,19 @@ +package io.unitycatalog.spark + +/** + * Thread-local holder for metric view context during source table resolution. + * + * When UCProxy.loadTable() encounters a METRIC_VIEW, it sets the view's table ID here. + * When the Spark analyzer subsequently resolves the metric view's source table (another + * loadTable() call), UCProxy reads this context and passes the view ID as the `dependent` + * parameter in the credential vending request, enabling view-mediated authorization. + */ +object MetricViewContext { + private val viewId = new ThreadLocal[String]() + + def set(id: String): Unit = viewId.set(id) + + def get(): Option[String] = Option(viewId.get()) + + def clear(): Unit = viewId.remove() +} diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 1d3daca7f3..c34a86d7f4 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -18,7 +18,7 @@ import org.apache.spark.sql.connector.catalog._ import org.apache.spark.sql.connector.expressions.Transform import org.apache.spark.sql.types._ import org.apache.spark.sql.util.CaseInsensitiveStringMap -import org.sparkproject.guava.base.Preconditions +import com.google.common.base.Preconditions import java.net.URI import java.util @@ -105,6 +105,14 @@ class UCSingleCatalog partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) + + if ("METRIC_VIEW".equals(properties.get("table_type"))) { + val schema = StructType(columns.map { col => + StructField(col.name(), col.dataType(), col.nullable()) + }) + return delegate.createTable(ident, schema, Array.empty[Transform], properties) + } + val hasExternalClause = properties.containsKey(TableCatalog.PROP_EXTERNAL) val hasLocationClause = properties.containsKey(TableCatalog.PROP_LOCATION) if (hasExternalClause && !hasLocationClause) { @@ -582,6 +590,11 @@ private class UCProxy( case e: ApiException if e.getCode == 404 => throw new NoSuchTableException(ident) } + + if (t.getTableType == TableType.METRIC_VIEW) { + return loadMetricView(t, ident) + } + val identifier = TableIdentifier(t.getName, Some(t.getSchemaName), Some(t.getCatalogName)) val partitionCols = scala.collection.mutable.ArrayBuffer.empty[(String, Int)] val fields = t.getColumns.asScala.map { col => @@ -594,25 +607,23 @@ private class UCProxy( val locationUri = CatalogUtils.stringToURI(t.getStorageLocation) val tableId = t.getTableId var tableOp = TableOperation.READ_WRITE + val credRequest = new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) + // If resolving a source table through a metric view, pass the view as dependent + MetricViewContext.get().foreach { viewId => + credRequest.setDependent(viewId) + MetricViewContext.clear() + } val temporaryCredentials = { try { - temporaryCredentialsApi - .generateTemporaryTableCredentials( - // TODO: at this time, we don't know if the table will be read or written. For now we always - // request READ_WRITE credentials as the server doesn't distinguish between READ and - // READ_WRITE credentials as of today. When loading a table, Spark should tell if it's - // for read or write, we can request the proper credential after fixing Spark. - new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - ) - } catch { + temporaryCredentialsApi.generateTemporaryTableCredentials(credRequest) + } catch { case e: ApiException => logWarning(s"READ_WRITE credential generation failed for table $identifier: ${e.getMessage}") try { tableOp = TableOperation.READ - temporaryCredentialsApi - .generateTemporaryTableCredentials( - new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - ) + val readCredRequest = new GenerateTemporaryTableCredential() + .tableId(tableId).operation(tableOp) + temporaryCredentialsApi.generateTemporaryTableCredentials(readCredRequest) } catch { case e: ApiException => logWarning(s"READ credential generation failed for table $identifier: ${e.getMessage}") @@ -658,17 +669,94 @@ private class UCProxy( tracksPartitionsInCatalog = false, partitionColumnNames = partitionCols.sortBy(_._2).map(_._1).toSeq ) - // Spark separates table lookup and data source resolution. To support Spark native data - // sources, here we return the `V1Table` which only contains the table metadata. Spark will - // resolve the data source and create scan node later. Class.forName("org.apache.spark.sql.connector.catalog.V1Table") .getDeclaredConstructor(classOf[CatalogTable]) .newInstance(sparkTable) .asInstanceOf[Table] } + private def loadMetricView(t: TableInfo, ident: Identifier): Table = { + val identifier = TableIdentifier(t.getName, Some(t.getSchemaName), Some(t.getCatalogName)) + val fields = if (t.getColumns != null) { + t.getColumns.asScala.map { col => + StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) + .withComment(col.getComment) + }.toArray + } else { + Array.empty[StructField] + } + + MetricViewContext.set(t.getTableId) + + val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) + + ("view.viewWithMetrics" -> "true") + + val sparkTable = CatalogTable( + identifier, + tableType = CatalogTableType.VIEW, + storage = CatalogStorageFormat.empty, + schema = StructType(fields), + viewText = Some(t.getViewDefinition), + viewOriginalText = Some(t.getViewDefinition), + properties = props, + tracksPartitionsInCatalog = false + ) + Class.forName("org.apache.spark.sql.connector.catalog.V1Table") + .getDeclaredConstructor(classOf[CatalogTable]) + .newInstance(sparkTable) + .asInstanceOf[Table] + } + + private def createMetricView( + ident: Identifier, schema: StructType, + properties: util.Map[String, String]): Table = { + val ct = new CreateTable() + ct.setName(ident.name()) + ct.setSchemaName(ident.namespace().head) + ct.setCatalogName(this.name) + ct.setTableType(TableType.METRIC_VIEW) + ct.setViewDefinition(properties.get("view_definition")) + Option(properties.get("comment")).foreach(ct.setComment(_)) + + val columns: Seq[ColumnInfo] = schema.fields.toSeq.zipWithIndex.map { case (field, i) => + val column = new ColumnInfo() + column.setName(field.name) + field.getComment().foreach(column.setComment(_)) + column.setNullable(field.nullable) + column.setTypeText(field.dataType.catalogString) + column.setTypeName(convertDataTypeToTypeName(field.dataType)) + column.setTypeJson(field.dataType.json) + column.setPosition(i) + column + } + ct.setColumns(columns.asJava) + + // Dependencies are extracted by Spark and passed as the view.dependency property + Option(properties.get("view.dependency")).foreach { depFullName => + val dep = new Dependency() + dep.setTable(new TableDependency().tableFullName(depFullName)) + val depList = new DependencyList() + depList.setDependencies(java.util.List.of(dep)) + ct.setViewDependencies(depList) + } + + val serverProps = properties.asScala + .filterKeys(!Set("table_type", "view_definition", "comment", "view.dependency", + "provider").contains(_)) + .toMap.asJava + ct.setProperties(serverProps) + + tablesApi.createTable(ct) + loadTable(ident) + } + override def createTable(ident: Identifier, schema: StructType, partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) + + if ("METRIC_VIEW".equals(properties.get("table_type"))) { + return createMetricView(ident, schema, properties) + } + UCSingleCatalog.requireProviderSpecified("CREATE TABLE", properties) val createTable = new CreateTable() diff --git a/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java b/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java index a812dfd5e1..e4df6a7fca 100644 --- a/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java +++ b/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java @@ -3,10 +3,10 @@ import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE; import static org.assertj.core.api.Assertions.assertThat; +import com.google.common.base.Objects; import io.unitycatalog.spark.auth.storage.AbfsVendedTokenProvider; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; -import org.sparkproject.guava.base.Objects; public class AzureCredentialTestFileSystem extends CredentialTestFileSystem { private volatile AbfsVendedTokenProvider provider; diff --git a/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java b/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java index daedb31da7..3b750b1a43 100644 --- a/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java +++ b/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java @@ -3,6 +3,8 @@ import static io.unitycatalog.server.utils.TestUtils.createApiClient; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterators; import io.delta.tables.DeltaTable; import io.unitycatalog.client.internal.Clock; import io.unitycatalog.client.model.CreateCatalog; @@ -41,8 +43,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.sparkproject.guava.collect.ImmutableList; -import org.sparkproject.guava.collect.Iterators; /** * Integration test to verify that cloud vendor credential renewal works as expected. From 35321aaead50f2b80aefc4a4f8c5ce4381af5164 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 30 Mar 2026 17:32:55 +0000 Subject: [PATCH 03/26] build sbt --- build.sbt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 9c12409aaa..efddadbf25 100644 --- a/build.sbt +++ b/build.sbt @@ -623,8 +623,9 @@ lazy val spark = (project in file("connectors/spark")) "org.apache.hadoop" % "hadoop-aws" % hadoopVersion % Test, "org.projectlombok" % "lombok" % "1.18.32" % Test, "com.google.cloud.bigdataoss" % "gcs-connector" % "3.0.2" % Test classifier "shaded", + ) ++ (if (sparkMajorMinorVersion == "4.0") Seq( "io.delta" %% s"delta-spark_$sparkMajorMinorVersion" % deltaVersion % Test, - ), + ) else Nil), dependencyOverrides ++= Seq( "com.fasterxml.jackson.core" % "jackson-databind" % "2.15.0", "com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.15.0", @@ -680,7 +681,9 @@ lazy val integrationTests = (project in file("integration-tests")) "org.assertj" % "assertj-core" % "3.26.3" % Test, "org.projectlombok" % "lombok" % "1.18.32" % Provided, "org.apache.spark" %% "spark-sql" % sparkVersion % Test, + ) ++ (if (sparkMajorMinorVersion == "4.0") Seq( "io.delta" %% s"delta-spark_$sparkMajorMinorVersion" % deltaVersion % Test, + ) else Nil) ++ Seq( "org.apache.hadoop" % "hadoop-aws" % hadoopVersion % Test, "org.apache.hadoop" % "hadoop-azure" % hadoopVersion % Test, "com.google.cloud.bigdataoss" % "gcs-connector" % "3.0.2" % Test classifier "shaded", From 3596f4909a6718e92a5079088caa083721580ab8 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 31 Mar 2026 01:10:06 +0000 Subject: [PATCH 04/26] Use AnalysisContext for metric view credential vending and adopt structured TableInfo - Replace MetricViewContext thread-local with Spark's AnalysisContext.metricViewId - Use CatalogTableType.METRIC_VIEW instead of view.viewWithMetrics property - Handle createTable(ident, TableInfo) for metric views with structured dependencies - Populate view dependencies in listTables for METRIC_VIEW type --- .../spark/MetricViewContext.scala | 19 ---- .../unitycatalog/spark/UCSingleCatalog.scala | 97 +++++++++++-------- .../server/persist/TableRepository.java | 10 ++ .../TemporaryTableCredentialsService.java | 2 +- 4 files changed, 66 insertions(+), 62 deletions(-) delete mode 100644 connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala deleted file mode 100644 index c24f4aac13..0000000000 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/MetricViewContext.scala +++ /dev/null @@ -1,19 +0,0 @@ -package io.unitycatalog.spark - -/** - * Thread-local holder for metric view context during source table resolution. - * - * When UCProxy.loadTable() encounters a METRIC_VIEW, it sets the view's table ID here. - * When the Spark analyzer subsequently resolves the metric view's source table (another - * loadTable() call), UCProxy reads this context and passes the view ID as the `dependent` - * parameter in the credential vending request, enabling view-mediated authorization. - */ -object MetricViewContext { - private val viewId = new ThreadLocal[String]() - - def set(id: String): Unit = viewId.set(id) - - def get(): Option[String] = Option(viewId.get()) - - def clear(): Unit = viewId.remove() -} diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index c34a86d7f4..1a4033bee6 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -99,6 +99,18 @@ class UCSingleCatalog delegate.tableExists(ident) } + override def createTable( + ident: Identifier, + tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { + UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) + val tableType = tableInfo.tableType() + if (TableSummary.METRIC_VIEW_TABLE_TYPE.equals(tableType)) { + return delegate.asInstanceOf[UCProxy].createMetricViewFromTableInfo( + ident, this.name, tableInfo) + } + super.createTable(ident, tableInfo) + } + override def createTable( ident: Identifier, columns: Array[Column], @@ -106,13 +118,6 @@ class UCSingleCatalog properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - if ("METRIC_VIEW".equals(properties.get("table_type"))) { - val schema = StructType(columns.map { col => - StructField(col.name(), col.dataType(), col.nullable()) - }) - return delegate.createTable(ident, schema, Array.empty[Transform], properties) - } - val hasExternalClause = properties.containsKey(TableCatalog.PROP_EXTERNAL) val hasLocationClause = properties.containsKey(TableCatalog.PROP_LOCATION) if (hasExternalClause && !hasLocationClause) { @@ -127,8 +132,6 @@ class UCSingleCatalog val newProps = prepareExternalTableProperties(properties) delegate.createTable(ident, columns, partitions, newProps) } else { - // TODO: for path-based tables, Spark should generate a location property using the qualified - // path string. delegate.createTable(ident, columns, partitions, properties) } } @@ -609,9 +612,8 @@ private class UCProxy( var tableOp = TableOperation.READ_WRITE val credRequest = new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) // If resolving a source table through a metric view, pass the view as dependent - MetricViewContext.get().foreach { viewId => + org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => credRequest.setDependent(viewId) - MetricViewContext.clear() } val temporaryCredentials = { try { @@ -686,14 +688,13 @@ private class UCProxy( Array.empty[StructField] } - MetricViewContext.set(t.getTableId) + org.apache.spark.sql.catalyst.analysis.AnalysisContext.setMetricViewId(t.getTableId) - val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) + - ("view.viewWithMetrics" -> "true") + val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) val sparkTable = CatalogTable( identifier, - tableType = CatalogTableType.VIEW, + tableType = CatalogTableType.METRIC_VIEW, storage = CatalogStorageFormat.empty, schema = StructType(fields), viewText = Some(t.getViewDefinition), @@ -707,42 +708,58 @@ private class UCProxy( .asInstanceOf[Table] } - private def createMetricView( - ident: Identifier, schema: StructType, - properties: util.Map[String, String]): Table = { + /** + * Creates a metric view using structured fields from Spark's TableInfo. + * Called by UCSingleCatalog when tableInfo.tableType() is METRIC_VIEW. + */ + def createMetricViewFromTableInfo( + ident: Identifier, + catalogName: String, + tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { val ct = new CreateTable() ct.setName(ident.name()) ct.setSchemaName(ident.namespace().head) - ct.setCatalogName(this.name) + ct.setCatalogName(catalogName) ct.setTableType(TableType.METRIC_VIEW) - ct.setViewDefinition(properties.get("view_definition")) - Option(properties.get("comment")).foreach(ct.setComment(_)) + ct.setViewDefinition(tableInfo.viewDefinition()) + Option(tableInfo.properties().get("comment")).foreach(ct.setComment(_)) - val columns: Seq[ColumnInfo] = schema.fields.toSeq.zipWithIndex.map { case (field, i) => + val columns: Seq[ColumnInfo] = tableInfo.columns().toSeq.zipWithIndex.map { case (col, i) => val column = new ColumnInfo() - column.setName(field.name) - field.getComment().foreach(column.setComment(_)) - column.setNullable(field.nullable) - column.setTypeText(field.dataType.catalogString) - column.setTypeName(convertDataTypeToTypeName(field.dataType)) - column.setTypeJson(field.dataType.json) + column.setName(col.name()) + column.setNullable(col.nullable()) + column.setTypeText(col.dataType().catalogString) + column.setTypeName(convertDataTypeToTypeName(col.dataType())) + column.setTypeJson(col.dataType().json) column.setPosition(i) column } ct.setColumns(columns.asJava) - // Dependencies are extracted by Spark and passed as the view.dependency property - Option(properties.get("view.dependency")).foreach { depFullName => - val dep = new Dependency() - dep.setTable(new TableDependency().tableFullName(depFullName)) - val depList = new DependencyList() - depList.setDependencies(java.util.List.of(dep)) - ct.setViewDependencies(depList) + Option(tableInfo.viewDependencies()).foreach { sparkDepList => + val ucDepList = new io.unitycatalog.client.model.DependencyList() + val ucDeps = sparkDepList.dependencies().map { dep => + val ucDep = new io.unitycatalog.client.model.Dependency() + dep match { + case td: org.apache.spark.sql.connector.catalog.TableDependency => + ucDep.setTable( + new io.unitycatalog.client.model.TableDependency() + .tableFullName(td.tableFullName())) + case fd: org.apache.spark.sql.connector.catalog.FunctionDependency => + ucDep.setFunction( + new io.unitycatalog.client.model.FunctionDependency() + .functionFullName(fd.functionFullName())) + case _ => + } + ucDep + } + ucDepList.setDependencies(java.util.Arrays.asList(ucDeps: _*)) + ct.setViewDependencies(ucDepList) } - val serverProps = properties.asScala - .filterKeys(!Set("table_type", "view_definition", "comment", "view.dependency", - "provider").contains(_)) + val reservedKeys = Set("comment", "provider") + val serverProps = tableInfo.properties().asScala + .filterKeys(!reservedKeys.contains(_)) .toMap.asJava ct.setProperties(serverProps) @@ -753,10 +770,6 @@ private class UCProxy( override def createTable(ident: Identifier, schema: StructType, partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - if ("METRIC_VIEW".equals(properties.get("table_type"))) { - return createMetricView(ident, schema, properties) - } - UCSingleCatalog.requireProviderSpecified("CREATE TABLE", properties) val createTable = new CreateTable() diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index 6d79efa5cd..6bba8567e7 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -384,6 +384,16 @@ public ListTablesResponse listTables( RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); } + if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { + List deps = + repositories + .getDependencyRepository() + .getDependencies(session, tableInfoDAO.getId(), "TABLE"); + if (!deps.isEmpty()) { + tableInfo.setViewDependencies( + new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); + } + } result.add(tableInfo); } return new ListTablesResponse().tables(result).nextPageToken(nextPageToken); diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index 1ec52f2cba..02fa78a218 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -102,7 +102,7 @@ public HttpResponse generateTemporaryTableCredential( * 2. The requested table_id is in the dependent's dependency list * 3. The dependent's owner has SELECT on the source table */ - private HttpResponse handleDependentCredentialRequest( + private HttpResponse `handleDependentCredentialRequest( String sourceTableId, String dependentId, TableOperation operation) { return TransactionManager.executeWithTransaction( sessionFactory, From 7030b20285f8d39b3d391d59ba1cdc9e3b59112c Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 31 Mar 2026 15:55:51 +0000 Subject: [PATCH 05/26] Add metadata snapshot endpoint for metric view definer's rights Implement GET /tables/{full_name}/metadata-snapshot that returns the metric view metadata plus resolved source table metadata in a single call. This allows the Spark connector to resolve source table metadata without requiring direct SELECT on source tables (definer's rights for metadata access). Server changes: - Add MetadataSnapshot model and endpoint to OpenAPI spec - Implement getMetadataSnapshot in TableRepository with dependency resolution - Add authorized endpoint in TableService (SELECT on metric view only) - Fix backtick typo in handleDependentCredentialRequest method name Connector changes: - Add metadataSnapshotCache to UCSingleCatalog companion object - Call metadata snapshot API in loadMetricView and cache source table info - Check cache in loadTable before calling getTable (consume-once semantics) Tests: - Add SdkMetricViewAccessControlTest with 8 test cases covering both credential vending (4 tests) and metadata snapshot (4 tests) permissions --- api/all.yaml | 45 ++ .../unitycatalog/spark/UCSingleCatalog.scala | 41 +- .../server/persist/TableRepository.java | 58 +++ .../server/service/TableService.java | 18 + .../TemporaryTableCredentialsService.java | 2 +- .../SdkMetricViewAccessControlTest.java | 440 ++++++++++++++++++ 6 files changed, 595 insertions(+), 9 deletions(-) create mode 100644 server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java diff --git a/api/all.yaml b/api/all.yaml index 8fe7e2dab5..015a3468b7 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -433,6 +433,37 @@ paths: content: application/json: schema: {} + /tables/{full_name}/metadata-snapshot: + parameters: + - name: full_name + in: path + description: Full name of the metric view (catalog.schema.view_name). + required: true + schema: + type: string + get: + tags: + - Tables + operationId: getMetadataSnapshot + summary: Get metadata snapshot for a metric view + description: | + Returns the metric view's metadata along with resolved metadata for all + dependency tables. This implements definer's rights for metadata access -- + the caller only needs SELECT on the metric view, not on the source tables. + The server resolves source table metadata server-side. + responses: + '200': + description: Metadata snapshot successfully retrieved. + content: + application/json: + schema: + $ref: '#/components/schemas/MetadataSnapshot' + '400': + description: Request is invalid (e.g., table is not a METRIC_VIEW). + '403': + description: Caller does not have SELECT on the metric view. + '404': + description: Metric view not found. /volumes: post: tags: @@ -3174,6 +3205,20 @@ components: required: - commits - latest_table_version + MetadataSnapshot: + type: object + description: | + A snapshot of metadata for a metric view and its dependency tables. + Used for definer's rights resolution -- the server resolves source + table metadata so the client does not need direct access to sources. + properties: + table_info: + $ref: '#/components/schemas/TableInfo' + dependency_table_infos: + description: Resolved metadata for each dependency table. + type: array + items: + $ref: '#/components/schemas/TableInfo' info: title: Unity Catalog API version: '0.1' diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 1a4033bee6..e9ba251b5b 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -449,6 +449,9 @@ object UCSingleCatalog { val LOAD_DELTA_CATALOG = ThreadLocal.withInitial[Boolean](() => true) val DELTA_CATALOG_LOADED = ThreadLocal.withInitial[Boolean](() => false) + private[spark] val metadataSnapshotCache = + new java.util.concurrent.ConcurrentHashMap[String, TableInfo]() + /** * Returns any user-configured {@code fs..impl} values from the current Spark session. * @@ -584,14 +587,17 @@ private class UCProxy( } override def loadTable(ident: Identifier): Table = { - val t = try { - tablesApi.getTable( - UCSingleCatalog.fullTableNameForApi(this.name, ident), - /* readStreamingTableAsManaged = */ true, - /* readMaterializedViewAsManaged = */ true) - } catch { - case e: ApiException if e.getCode == 404 => - throw new NoSuchTableException(ident) + val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) + val t = Option(UCSingleCatalog.metadataSnapshotCache.remove(fullName)).getOrElse { + try { + tablesApi.getTable( + fullName, + /* readStreamingTableAsManaged = */ true, + /* readMaterializedViewAsManaged = */ true) + } catch { + case e: ApiException if e.getCode == 404 => + throw new NoSuchTableException(ident) + } } if (t.getTableType == TableType.METRIC_VIEW) { @@ -625,6 +631,9 @@ private class UCProxy( tableOp = TableOperation.READ val readCredRequest = new GenerateTemporaryTableCredential() .tableId(tableId).operation(tableOp) + org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => + readCredRequest.setDependent(viewId) + } temporaryCredentialsApi.generateTemporaryTableCredentials(readCredRequest) } catch { case e: ApiException => @@ -688,6 +697,22 @@ private class UCProxy( Array.empty[StructField] } + try { + val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) + val snapshot = tablesApi.getMetadataSnapshot(fullName) + if (snapshot.getDependencyTableInfos != null) { + snapshot.getDependencyTableInfos.asScala.foreach { depTable => + val depFullName = + s"${depTable.getCatalogName}.${depTable.getSchemaName}.${depTable.getName}" + UCSingleCatalog.metadataSnapshotCache.put(depFullName, depTable) + } + } + } catch { + case e: Exception => + logWarning(s"Failed to get metadata snapshot for ${t.getName}, " + + s"source table resolution will fall back to direct getTable calls", e) + } + org.apache.spark.sql.catalyst.analysis.AnalysisContext.setMetricViewId(t.getTableId) val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index 6bba8567e7..c7d406102a 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -7,6 +7,7 @@ import io.unitycatalog.server.model.DataSourceFormat; import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; +import io.unitycatalog.server.model.MetadataSnapshot; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.model.TableType; import io.unitycatalog.server.persist.dao.DependencyDAO; @@ -169,6 +170,63 @@ public TableInfo getTable(String fullName) { /* readOnly = */ true); } + public MetadataSnapshot getMetadataSnapshot(String fullName) { + LOGGER.debug("Getting metadata snapshot: {}", fullName); + return TransactionManager.executeWithTransaction( + sessionFactory, + session -> { + String[] parts = fullName.split("\\."); + if (parts.length != 3) { + throw new BaseException(ErrorCode.INVALID_ARGUMENT, "Invalid table name: " + fullName); + } + String catalogName = parts[0]; + String schemaName = parts[1]; + String tableName = parts[2]; + TableInfoDAO viewDAO = findTable(session, catalogName, schemaName, tableName); + if (viewDAO == null) { + throw new BaseException(ErrorCode.NOT_FOUND, "Table not found: " + fullName); + } + if (!"METRIC_VIEW".equals(viewDAO.getType())) { + throw new BaseException( + ErrorCode.INVALID_ARGUMENT, + "metadata-snapshot is only supported for METRIC_VIEW tables"); + } + TableInfo viewInfo = viewDAO.toTableInfo(true, catalogName, schemaName); + RepositoryUtils.attachProperties( + viewInfo, viewInfo.getTableId(), Constants.TABLE, session); + + List deps = + repositories + .getDependencyRepository() + .getDependencies(session, viewDAO.getId(), "TABLE"); + if (!deps.isEmpty()) { + viewInfo.setViewDependencies( + new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); + } + + List depTableInfos = new ArrayList<>(); + for (DependencyDAO dep : deps) { + TableInfoDAO depDAO = + findTable( + session, + dep.getDependencyCatalog(), + dep.getDependencySchema(), + dep.getDependencyName()); + if (depDAO != null) { + TableInfo depInfo = + depDAO.toTableInfo(true, dep.getDependencyCatalog(), dep.getDependencySchema()); + RepositoryUtils.attachProperties( + depInfo, depInfo.getTableId(), Constants.TABLE, session); + depTableInfos.add(depInfo); + } + } + + return new MetadataSnapshot().tableInfo(viewInfo).dependencyTableInfos(depTableInfos); + }, + "Failed to get metadata snapshot", + /* readOnly = */ true); + } + public String getTableUniformMetadataLocation( Session session, String catalogName, String schemaName, String tableName) { TableInfoDAO dao = findTable(session, catalogName, schemaName, tableName); diff --git a/server/src/main/java/io/unitycatalog/server/service/TableService.java b/server/src/main/java/io/unitycatalog/server/service/TableService.java index fe82504d60..3384732841 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TableService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TableService.java @@ -15,6 +15,7 @@ import io.unitycatalog.server.model.CatalogInfo; import io.unitycatalog.server.model.CreateTable; import io.unitycatalog.server.model.ListTablesResponse; +import io.unitycatalog.server.model.MetadataSnapshot; import io.unitycatalog.server.model.SchemaInfo; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.persist.CatalogRepository; @@ -123,6 +124,23 @@ public HttpResponse getTable(@Param("full_name") @AuthorizeResourceKey(TABLE) St return HttpResponse.ofJson(tableInfo); } + @Get("/{full_name}/metadata-snapshot") + @AuthorizeExpression(""" + #authorize(#principal, #metastore, OWNER) || + #authorize(#principal, #catalog, OWNER) || + (#authorize(#principal, #schema, OWNER) && #authorize(#principal, #catalog, USE_CATALOG)) || + (#authorize(#principal, #schema, USE_SCHEMA) && + #authorize(#principal, #catalog, USE_CATALOG) && + #authorizeAny(#principal, #table, OWNER, SELECT)) + """) + @AuthorizeResourceKey(METASTORE) + public HttpResponse getMetadataSnapshot( + @Param("full_name") @AuthorizeResourceKey(TABLE) String fullName) { + assert fullName != null; + MetadataSnapshot snapshot = tableRepository.getMetadataSnapshot(fullName); + return HttpResponse.ofJson(snapshot); + } + @Get("") @AuthorizeExpression("#defer") public HttpResponse listTables( diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index 02fa78a218..1ec52f2cba 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -102,7 +102,7 @@ public HttpResponse generateTemporaryTableCredential( * 2. The requested table_id is in the dependent's dependency list * 3. The dependent's owner has SELECT on the source table */ - private HttpResponse `handleDependentCredentialRequest( + private HttpResponse handleDependentCredentialRequest( String sourceTableId, String dependentId, TableOperation operation) { return TransactionManager.executeWithTransaction( sessionFactory, diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java new file mode 100644 index 0000000000..fd63dc8906 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java @@ -0,0 +1,440 @@ +package io.unitycatalog.server.sdk.access; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +import io.unitycatalog.client.ApiException; +import io.unitycatalog.client.api.TablesApi; +import io.unitycatalog.client.api.TemporaryCredentialsApi; +import io.unitycatalog.client.model.ColumnInfo; +import io.unitycatalog.client.model.ColumnTypeName; +import io.unitycatalog.client.model.CreateTable; +import io.unitycatalog.client.model.DataSourceFormat; +import io.unitycatalog.client.model.Dependency; +import io.unitycatalog.client.model.DependencyList; +import io.unitycatalog.client.model.GenerateTemporaryTableCredential; +import io.unitycatalog.client.model.MetadataSnapshot; +import io.unitycatalog.client.model.SecurableType; +import io.unitycatalog.client.model.TableDependency; +import io.unitycatalog.client.model.TableInfo; +import io.unitycatalog.client.model.TableOperation; +import io.unitycatalog.client.model.TableType; +import io.unitycatalog.server.base.ServerConfig; +import io.unitycatalog.server.exception.ErrorCode; +import io.unitycatalog.server.persist.model.Privileges; +import io.unitycatalog.server.utils.TestUtils; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Tests the definer's-rights permission model for metric view credential vending. + * + *

When a user queries a metric view, the Spark connector requests temporary credentials for the + * source table with the metric view ID as the {@code dependent} parameter. The server's {@code + * handleDependentCredentialRequest} performs three authorization checks: + * + *

    + *
  1. The requesting user has SELECT on the metric view (dependent) + *
  2. The source table is in the metric view's dependency list + *
  3. The metric view's owner has SELECT on the source table (definer's rights) + *
+ * + *

This test validates all three checks: one positive case and three negative cases (one for each + * check failing). + */ +public class SdkMetricViewAccessControlTest extends SdkAccessControlBaseCRUDTest { + + private static final String VIEW_OWNER_EMAIL = "view_owner@test.com"; + private static final String READER_EMAIL = "reader@test.com"; + + private static final String SOURCE_TABLE_NAME = "source_data"; + private static final String SOURCE_TABLE_FULL_NAME = + TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + SOURCE_TABLE_NAME; + + private static final String METRIC_VIEW_NAME = "test_metric_view"; + private static final String METRIC_VIEW_FULL_NAME = + TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + METRIC_VIEW_NAME; + + private static final String VIEW_DEFINITION = + "version: \"0.1\"\nsource: " + + SOURCE_TABLE_FULL_NAME + + "\nmeasures:\n - name: total\n expr: SUM(amount)"; + + private static final List SOURCE_COLUMNS = + List.of( + new ColumnInfo() + .name("amount") + .typeText("DOUBLE") + .typeJson("{\"type\": \"double\"}") + .typeName(ColumnTypeName.DOUBLE) + .position(0) + .nullable(true)); + + /** + * Positive test: view-mediated credential vending succeeds when all three checks pass. + * + *

The view owner has SELECT on the source table, the reader has SELECT on the metric view, and + * the source table is in the metric view's dependency list. + */ + @Test + public void testDefinerRightsPositive() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + // Create source table as admin first (must exist before granting permissions) + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + // Grant view owner: catalog/schema access + CREATE_TABLE + SELECT on source table + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); + + // Create metric view as view owner (so owner field is set correctly) + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); + + // Grant reader: catalog/schema access + SELECT on metric view + grantReaderBasePermissions(); + grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); + + // Reader requests credentials for source table through the metric view + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TemporaryCredentialsApi readerTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + + GenerateTemporaryTableCredential credRequest = + new GenerateTemporaryTableCredential() + .tableId(sourceTable.getTableId()) + .operation(TableOperation.READ) + .dependent(metricView.getTableId()); + + // Should succeed: all three checks pass + readerTempCredsApi.generateTemporaryTableCredentials(credRequest); + } + + /** + * Negative test: credential vending fails when the user lacks SELECT on the metric view. + * + *

This validates authorization check #1. + */ + @Test + public void testDeniedWhenUserLacksSelectOnView() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); + + // Grant reader catalog/schema access but NOT SELECT on metric view + grantReaderBasePermissions(); + + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TemporaryCredentialsApi readerTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + + GenerateTemporaryTableCredential credRequest = + new GenerateTemporaryTableCredential() + .tableId(sourceTable.getTableId()) + .operation(TableOperation.READ) + .dependent(metricView.getTableId()); + + assertThatExceptionOfType(ApiException.class) + .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) + .satisfies( + ex -> + assertThat(ex.getCode()) + .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + } + + /** + * Negative test: credential vending fails when the source table is not in the metric view's + * dependency list. + * + *

This validates authorization check #2. + */ + @Test + public void testDeniedWhenSourceNotInDependencies() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + // Create a metric view WITHOUT dependencies + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + + CreateTable createMetricView = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDefinition(VIEW_DEFINITION); + TableInfo metricView = viewOwnerTablesApi.createTable(createMetricView); + + // Grant reader SELECT on metric view + grantReaderBasePermissions(); + grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); + + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TemporaryCredentialsApi readerTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + + GenerateTemporaryTableCredential credRequest = + new GenerateTemporaryTableCredential() + .tableId(sourceTable.getTableId()) + .operation(TableOperation.READ) + .dependent(metricView.getTableId()); + + assertThatExceptionOfType(ApiException.class) + .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) + .satisfies( + ex -> + assertThat(ex.getCode()) + .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + } + + /** + * Negative test: credential vending fails when the metric view owner lacks SELECT on the source + * table. + * + *

This validates authorization check #3 (definer's rights). + */ + @Test + public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + // Grant view owner catalog/schema access but NOT SELECT on source table + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); + + // Grant reader SELECT on metric view + grantReaderBasePermissions(); + grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); + + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TemporaryCredentialsApi readerTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + + GenerateTemporaryTableCredential credRequest = + new GenerateTemporaryTableCredential() + .tableId(sourceTable.getTableId()) + .operation(TableOperation.READ) + .dependent(metricView.getTableId()); + + assertThatExceptionOfType(ApiException.class) + .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) + .satisfies( + ex -> + assertThat(ex.getCode()) + .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + } + + /** + * Positive test: metadata snapshot succeeds when the reader has SELECT on the metric view. The + * response should include the source table's full metadata even though the reader has no direct + * SELECT on it (definer's rights for metadata access). + */ + @Test + public void testMetadataSnapshotPositive() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); + + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); + + grantReaderBasePermissions(); + grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); + + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); + + MetadataSnapshot snapshot = readerTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME); + + assertThat(snapshot.getTableInfo()).isNotNull(); + assertThat(snapshot.getTableInfo().getName()).isEqualTo(METRIC_VIEW_NAME); + assertThat(snapshot.getTableInfo().getTableType()).isEqualTo(TableType.METRIC_VIEW); + + assertThat(snapshot.getDependencyTableInfos()).isNotNull(); + assertThat(snapshot.getDependencyTableInfos()).hasSize(1); + + TableInfo resolvedSource = snapshot.getDependencyTableInfos().get(0); + assertThat(resolvedSource.getName()).isEqualTo(SOURCE_TABLE_NAME); + assertThat(resolvedSource.getStorageLocation()).isNotNull(); + assertThat(resolvedSource.getColumns()).isNotNull(); + assertThat(resolvedSource.getColumns()).hasSizeGreaterThan(0); + } + + /** Negative test: metadata snapshot fails when the reader lacks SELECT on the metric view. */ + @Test + public void testMetadataSnapshotDeniedNoSelectOnView() throws Exception { + createTestUser(VIEW_OWNER_EMAIL, "View Owner"); + createTestUser(READER_EMAIL, "Reader"); + + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + + grantViewOwnerBasePermissions(); + grantPermissions( + VIEW_OWNER_EMAIL, + SecurableType.SCHEMA, + TestUtils.SCHEMA_FULL_NAME, + Privileges.CREATE_TABLE); + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); + + ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); + TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); + createMetricView(viewOwnerTablesApi, sourceTable); + + // Grant reader catalog/schema access but NOT SELECT on metric view + grantReaderBasePermissions(); + + ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); + TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); + + assertThatExceptionOfType(ApiException.class) + .isThrownBy(() -> readerTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME)) + .satisfies( + ex -> + assertThat(ex.getCode()) + .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + } + + /** Negative test: metadata snapshot fails when called on a non-METRIC_VIEW table. */ + @Test + public void testMetadataSnapshotNonMetricViewFails() throws Exception { + TablesApi adminTablesApi = new TablesApi(adminApiClient); + createSourceTable(adminTablesApi); + + assertThatExceptionOfType(ApiException.class) + .isThrownBy(() -> adminTablesApi.getMetadataSnapshot(SOURCE_TABLE_FULL_NAME)) + .satisfies(ex -> assertThat(ex.getCode()).isEqualTo(400)); + } + + /** + * Positive test: metadata snapshot resolves full source table metadata including columns and + * storage location, verifying the server-side resolution is complete. + */ + @Test + public void testMetadataSnapshotResolvesSourceMetadata() throws Exception { + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + TableInfo metricView = createMetricView(adminTablesApi, sourceTable); + + MetadataSnapshot snapshot = adminTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME); + + assertThat(snapshot.getTableInfo().getTableId()).isEqualTo(metricView.getTableId()); + assertThat(snapshot.getTableInfo().getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(snapshot.getTableInfo().getViewDependencies()).isNotNull(); + + assertThat(snapshot.getDependencyTableInfos()).hasSize(1); + TableInfo resolvedSource = snapshot.getDependencyTableInfos().get(0); + assertThat(resolvedSource.getTableId()).isEqualTo(sourceTable.getTableId()); + assertThat(resolvedSource.getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); + assertThat(resolvedSource.getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); + assertThat(resolvedSource.getStorageLocation()) + .contains("uc-test-metric-view/" + SOURCE_TABLE_NAME); + assertThat(resolvedSource.getDataSourceFormat()).isEqualTo(DataSourceFormat.PARQUET); + assertThat(resolvedSource.getColumns()).hasSize(1); + assertThat(resolvedSource.getColumns().get(0).getName()).isEqualTo("amount"); + } + + private TableInfo createSourceTable(TablesApi tablesApi) throws ApiException { + CreateTable createTable = + new CreateTable() + .name(SOURCE_TABLE_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .columns(SOURCE_COLUMNS) + .tableType(TableType.EXTERNAL) + .dataSourceFormat(DataSourceFormat.PARQUET) + .storageLocation("file:///tmp/uc-test-metric-view/" + SOURCE_TABLE_NAME); + return tablesApi.createTable(createTable); + } + + private TableInfo createMetricView(TablesApi tablesApi, TableInfo sourceTable) + throws ApiException { + Dependency dep = new Dependency(); + dep.setTable(new TableDependency().tableFullName(SOURCE_TABLE_FULL_NAME)); + DependencyList depList = new DependencyList(); + depList.setDependencies(List.of(dep)); + + CreateTable createMetricView = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDefinition(VIEW_DEFINITION) + .viewDependencies(depList); + return tablesApi.createTable(createMetricView); + } + + private void grantViewOwnerBasePermissions() throws Exception { + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); + grantPermissions( + VIEW_OWNER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); + } + + private void grantReaderBasePermissions() throws Exception { + grantPermissions( + READER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); + grantPermissions( + READER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); + } +} From 214979cb92935af5f3727114a6bb526faf8e7349 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 31 Mar 2026 20:28:30 +0000 Subject: [PATCH 06/26] Switch to MAPS endpoint and structured Dependent for Databricks UC compatibility Replace the custom GET /metadata-snapshot endpoint with the standard POST /metadata-and-permissions-snapshot (MAPS) endpoint to align with Databricks UC wire format. This enables the OSS connector to work against both OSS UC and Databricks UC without code changes. Key changes: - OpenAPI: new MAPS endpoint with nested response shape (MetadataAndPermissionsSnapshotResponse wrapping MetadataSnapshotResponse), rename TableResult.missing_reason to reason, add Dependent/TableDependent schemas for structured credential dependent field - Server: new MetadataSnapshotService handler for MAPS, remove old per-table GET endpoint from TableService, update credential handler to parse nested Dependent structure - Connector: call getMetadataAndPermissionsSnapshot, unwrap nested response, construct structured Dependent for credential vending, convert metadataSnapshotCache to ThreadLocal for thread safety - Tests: update all 9 integration tests in SdkMetricViewAccessControlTest --- api/all.yaml | 166 ++++++++++++--- .../unitycatalog/spark/UCSingleCatalog.scala | 39 ++-- .../server/UnityCatalogServer.java | 7 + .../server/persist/TableRepository.java | 82 +++++--- .../service/MetadataSnapshotService.java | 123 +++++++++++ .../server/service/TableService.java | 18 -- .../TemporaryTableCredentialsService.java | 10 +- .../SdkMetricViewAccessControlTest.java | 198 ++++++++++++------ 8 files changed, 487 insertions(+), 156 deletions(-) create mode 100644 server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java diff --git a/api/all.yaml b/api/all.yaml index 015a3468b7..403db5dd8d 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -433,37 +433,36 @@ paths: content: application/json: schema: {} - /tables/{full_name}/metadata-snapshot: - parameters: - - name: full_name - in: path - description: Full name of the metric view (catalog.schema.view_name). - required: true - schema: - type: string - get: + /metadata-and-permissions-snapshot: + post: tags: - Tables - operationId: getMetadataSnapshot - summary: Get metadata snapshot for a metric view + operationId: getMetadataAndPermissionsSnapshot + summary: Get metadata and permissions snapshot for securables description: | - Returns the metric view's metadata along with resolved metadata for all - dependency tables. This implements definer's rights for metadata access -- - the caller only needs SELECT on the metric view, not on the source tables. - The server resolves source table metadata server-side. + Returns a batch metadata snapshot for the requested securables, with optional + view dependency expansion. When a securable is a view or metric view and + include_view_dependency_expansion is true, the server resolves all transitive + dependency tables and includes their metadata in the response. This implements + definer's rights for metadata access -- the caller only needs SELECT on the + view, not on the source tables. Compatible with Databricks UC MAPS endpoint. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/MetadataAndPermissionsSnapshotRequest' responses: '200': - description: Metadata snapshot successfully retrieved. + description: Metadata and permissions snapshot successfully retrieved. content: application/json: schema: - $ref: '#/components/schemas/MetadataSnapshot' + $ref: '#/components/schemas/MetadataAndPermissionsSnapshotResponse' '400': - description: Request is invalid (e.g., table is not a METRIC_VIEW). + description: Request is invalid (e.g., missing securables). '403': - description: Caller does not have SELECT on the metric view. - '404': - description: Metric view not found. + description: Caller does not have required permissions. /volumes: post: tags: @@ -2400,10 +2399,12 @@ components: $ref: '#/components/schemas/TableOperation' dependent: description: | - Optional. Table ID of the view or metric view through which - this table is being accessed. When provided, the server performs - view-mediated authorization instead of direct authorization. - type: string + Optional. The entity that depends on the table, such as a view + or metric view through which this table is being accessed. + When provided, the server performs view-mediated authorization + instead of direct authorization. Compatible with Databricks UC + Dependent proto wire format. + $ref: '#/components/schemas/Dependent' type: object required: - table_id @@ -3205,20 +3206,117 @@ components: required: - commits - latest_table_version - MetadataSnapshot: + Securable: type: object - description: | - A snapshot of metadata for a metric view and its dependency tables. - Used for definer's rights resolution -- the server resolves source - table metadata so the client does not need direct access to sources. + description: A reference to a securable object for batch metadata requests. properties: - table_info: + type: + $ref: '#/components/schemas/SecurableType' + full_name: + description: Full name of the securable (e.g. catalog.schema.table). + type: string + required: + - type + - full_name + MissingReason: + type: object + description: Reason why a requested securable could not be resolved. + properties: + name: + description: Full name of the missing object. + type: string + reason: + description: Error message explaining why the object is missing. + type: string + TableResult: + type: object + description: Result wrapper for a table in a metadata snapshot response. + properties: + table: $ref: '#/components/schemas/TableInfo' - dependency_table_infos: - description: Resolved metadata for each dependency table. + reason: + $ref: '#/components/schemas/MissingReason' + SchemaResult: + type: object + description: Result wrapper for a schema in a metadata snapshot response. + properties: + schema: + $ref: '#/components/schemas/SchemaInfo' + reason: + $ref: '#/components/schemas/MissingReason' + CatalogResult: + type: object + description: Result wrapper for a catalog in a metadata snapshot response. + properties: + catalog: + $ref: '#/components/schemas/CatalogInfo' + reason: + $ref: '#/components/schemas/MissingReason' + MetadataAndPermissionsSnapshotRequest: + type: object + description: | + Request for a batch metadata and permissions snapshot. Compatible with the + Databricks UC MetadataAndPermissionsSnapshot RPC. + properties: + securables: + description: List of securables for which metadata is requested. type: array items: - $ref: '#/components/schemas/TableInfo' + $ref: '#/components/schemas/Securable' + include_view_dependency_expansion: + description: | + If true, view and metric view dependencies are resolved server-side + and included in the response. The full dependency DAG is traversed + and authorized using definer's rights. + type: boolean + default: false + required: + - securables + MetadataSnapshotResponse: + type: object + description: | + Inner metadata snapshot containing resolved metadata for the requested + securables and optionally their transitive dependencies. + properties: + catalogs: + description: Catalog metadata for the requested securables. + type: array + items: + $ref: '#/components/schemas/CatalogResult' + schemas: + description: Schema metadata for the requested securables. + type: array + items: + $ref: '#/components/schemas/SchemaResult' + tables: + description: Table metadata for the requested securables and dependencies. + type: array + items: + $ref: '#/components/schemas/TableResult' + MetadataAndPermissionsSnapshotResponse: + type: object + description: | + Batch metadata and permissions snapshot response. The metadata field wraps + the resolved metadata for the requested securables. Compatible with the + Databricks UC MetadataAndPermissionsSnapshot.Response wire format. + properties: + metadata: + $ref: '#/components/schemas/MetadataSnapshotResponse' + Dependent: + type: object + description: | + The entity that depends on a table, such as a view or metric view. + Compatible with the Databricks UC Dependent proto wire format. + properties: + table: + $ref: '#/components/schemas/TableDependent' + TableDependent: + type: object + description: A table that is the dependent entity (e.g. a view or metric view). + properties: + table_id: + description: The table ID of the dependent entity. + type: string info: title: Unity Catalog API version: '0.1' diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index e9ba251b5b..5227419b9f 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -449,8 +449,10 @@ object UCSingleCatalog { val LOAD_DELTA_CATALOG = ThreadLocal.withInitial[Boolean](() => true) val DELTA_CATALOG_LOADED = ThreadLocal.withInitial[Boolean](() => false) - private[spark] val metadataSnapshotCache = - new java.util.concurrent.ConcurrentHashMap[String, TableInfo]() + private[spark] val metadataSnapshotCache: ThreadLocal[java.util.HashMap[String, TableInfo]] = + ThreadLocal.withInitial[java.util.HashMap[String, TableInfo]]( + () => new java.util.HashMap[String, TableInfo]() + ) /** * Returns any user-configured {@code fs..impl} values from the current Spark session. @@ -588,7 +590,7 @@ private class UCProxy( override def loadTable(ident: Identifier): Table = { val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) - val t = Option(UCSingleCatalog.metadataSnapshotCache.remove(fullName)).getOrElse { + val t = Option(UCSingleCatalog.metadataSnapshotCache.get().remove(fullName)).getOrElse { try { tablesApi.getTable( fullName, @@ -617,9 +619,8 @@ private class UCProxy( val tableId = t.getTableId var tableOp = TableOperation.READ_WRITE val credRequest = new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - // If resolving a source table through a metric view, pass the view as dependent org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => - credRequest.setDependent(viewId) + credRequest.setDependent(new Dependent().table(new TableDependent().tableId(viewId))) } val temporaryCredentials = { try { @@ -632,7 +633,8 @@ private class UCProxy( val readCredRequest = new GenerateTemporaryTableCredential() .tableId(tableId).operation(tableOp) org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => - readCredRequest.setDependent(viewId) + readCredRequest.setDependent( + new Dependent().table(new TableDependent().tableId(viewId))) } temporaryCredentialsApi.generateTemporaryTableCredentials(readCredRequest) } catch { @@ -699,12 +701,25 @@ private class UCProxy( try { val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) - val snapshot = tablesApi.getMetadataSnapshot(fullName) - if (snapshot.getDependencyTableInfos != null) { - snapshot.getDependencyTableInfos.asScala.foreach { depTable => - val depFullName = - s"${depTable.getCatalogName}.${depTable.getSchemaName}.${depTable.getName}" - UCSingleCatalog.metadataSnapshotCache.put(depFullName, depTable) + val securable = new Securable() + .`type`(SecurableType.TABLE) + .fullName(fullName) + val request = new MetadataAndPermissionsSnapshotRequest() + .securables(java.util.List.of(securable)) + .includeViewDependencyExpansion(true) + val response = tablesApi.getMetadataAndPermissionsSnapshot(request) + val metadata = response.getMetadata + if (metadata != null && metadata.getTables != null) { + metadata.getTables.asScala.foreach { tableResult => + val depTable = tableResult.getTable + if (depTable != null) { + val depFullName = + s"${depTable.getCatalogName}.${depTable.getSchemaName}.${depTable.getName}" + val requestedFullName = fullName + if (depFullName != requestedFullName) { + UCSingleCatalog.metadataSnapshotCache.get().put(depFullName, depTable) + } + } } } } catch { diff --git a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java index 76dfc3fef4..23f2d6370a 100644 --- a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java +++ b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java @@ -34,6 +34,7 @@ import io.unitycatalog.server.service.ExternalLocationService; import io.unitycatalog.server.service.FunctionService; import io.unitycatalog.server.service.IcebergRestCatalogService; +import io.unitycatalog.server.service.MetadataSnapshotService; import io.unitycatalog.server.service.MetastoreService; import io.unitycatalog.server.service.ModelService; import io.unitycatalog.server.service.PermissionService; @@ -167,6 +168,8 @@ private void addApiServices( SchemaService schemaService = new SchemaService(authorizer, repositories); VolumeService volumeService = new VolumeService(authorizer, repositories); TableService tableService = new TableService(authorizer, repositories); + MetadataSnapshotService metadataSnapshotService = + new MetadataSnapshotService(authorizer, repositories); StagingTableService stagingTableService = new StagingTableService(authorizer, repositories); FunctionService functionService = new FunctionService(authorizer, repositories); ModelService modelService = new ModelService(authorizer, repositories); @@ -214,6 +217,10 @@ private void addApiServices( .annotatedService(BASE_PATH + "schemas", schemaService, requestConverterFunction) .annotatedService(BASE_PATH + "volumes", volumeService, requestConverterFunction) .annotatedService(BASE_PATH + "tables", tableService, requestConverterFunction) + .annotatedService( + BASE_PATH + "metadata-and-permissions-snapshot", + metadataSnapshotService, + requestConverterFunction) .annotatedService( BASE_PATH + "staging-tables", stagingTableService, requestConverterFunction) .annotatedService(BASE_PATH + "functions", functionService, requestConverterFunction) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index c7d406102a..eb71ed069f 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -7,8 +7,10 @@ import io.unitycatalog.server.model.DataSourceFormat; import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; -import io.unitycatalog.server.model.MetadataSnapshot; +import io.unitycatalog.server.model.MetadataSnapshotResponse; +import io.unitycatalog.server.model.MissingReason; import io.unitycatalog.server.model.TableInfo; +import io.unitycatalog.server.model.TableResult; import io.unitycatalog.server.model.TableType; import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.PropertyDAO; @@ -170,7 +172,8 @@ public TableInfo getTable(String fullName) { /* readOnly = */ true); } - public MetadataSnapshot getMetadataSnapshot(String fullName) { + public MetadataSnapshotResponse getMetadataSnapshot( + String fullName, boolean includeViewDependencyExpansion) { LOGGER.debug("Getting metadata snapshot: {}", fullName); return TransactionManager.executeWithTransaction( sessionFactory, @@ -182,46 +185,67 @@ public MetadataSnapshot getMetadataSnapshot(String fullName) { String catalogName = parts[0]; String schemaName = parts[1]; String tableName = parts[2]; - TableInfoDAO viewDAO = findTable(session, catalogName, schemaName, tableName); - if (viewDAO == null) { - throw new BaseException(ErrorCode.NOT_FOUND, "Table not found: " + fullName); - } - if (!"METRIC_VIEW".equals(viewDAO.getType())) { - throw new BaseException( - ErrorCode.INVALID_ARGUMENT, - "metadata-snapshot is only supported for METRIC_VIEW tables"); + TableInfoDAO tableDAO = findTable(session, catalogName, schemaName, tableName); + + MetadataSnapshotResponse response = new MetadataSnapshotResponse(); + List tableResults = new ArrayList<>(); + + if (tableDAO == null) { + tableResults.add( + new TableResult() + .reason( + new MissingReason().name(fullName).reason("Table not found: " + fullName))); + return response.tables(tableResults); } - TableInfo viewInfo = viewDAO.toTableInfo(true, catalogName, schemaName); + + TableInfo tableInfo = tableDAO.toTableInfo(true, catalogName, schemaName); RepositoryUtils.attachProperties( - viewInfo, viewInfo.getTableId(), Constants.TABLE, session); + tableInfo, tableInfo.getTableId(), Constants.TABLE, session); List deps = repositories .getDependencyRepository() - .getDependencies(session, viewDAO.getId(), "TABLE"); + .getDependencies(session, tableDAO.getId(), "TABLE"); if (!deps.isEmpty()) { - viewInfo.setViewDependencies( + tableInfo.setViewDependencies( new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); } + tableResults.add(new TableResult().table(tableInfo)); - List depTableInfos = new ArrayList<>(); - for (DependencyDAO dep : deps) { - TableInfoDAO depDAO = - findTable( - session, - dep.getDependencyCatalog(), - dep.getDependencySchema(), - dep.getDependencyName()); - if (depDAO != null) { - TableInfo depInfo = - depDAO.toTableInfo(true, dep.getDependencyCatalog(), dep.getDependencySchema()); - RepositoryUtils.attachProperties( - depInfo, depInfo.getTableId(), Constants.TABLE, session); - depTableInfos.add(depInfo); + if (includeViewDependencyExpansion + && "METRIC_VIEW".equals(tableDAO.getType()) + && !deps.isEmpty()) { + for (DependencyDAO dep : deps) { + TableInfoDAO depDAO = + findTable( + session, + dep.getDependencyCatalog(), + dep.getDependencySchema(), + dep.getDependencyName()); + if (depDAO != null) { + TableInfo depInfo = + depDAO.toTableInfo(true, dep.getDependencyCatalog(), dep.getDependencySchema()); + RepositoryUtils.attachProperties( + depInfo, depInfo.getTableId(), Constants.TABLE, session); + tableResults.add(new TableResult().table(depInfo)); + } else { + String depFullName = + dep.getDependencyCatalog() + + "." + + dep.getDependencySchema() + + "." + + dep.getDependencyName(); + tableResults.add( + new TableResult() + .reason( + new MissingReason() + .name(depFullName) + .reason("Dependency table not found"))); + } } } - return new MetadataSnapshot().tableInfo(viewInfo).dependencyTableInfos(depTableInfos); + return response.tables(tableResults); }, "Failed to get metadata snapshot", /* readOnly = */ true); diff --git a/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java b/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java new file mode 100644 index 0000000000..3192224e60 --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java @@ -0,0 +1,123 @@ +package io.unitycatalog.server.service; + +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.server.annotation.ExceptionHandler; +import com.linecorp.armeria.server.annotation.Post; +import io.unitycatalog.server.auth.UnityCatalogAuthorizer; +import io.unitycatalog.server.auth.annotation.AuthorizeExpression; +import io.unitycatalog.server.exception.BaseException; +import io.unitycatalog.server.exception.ErrorCode; +import io.unitycatalog.server.exception.GlobalExceptionHandler; +import io.unitycatalog.server.model.CatalogInfo; +import io.unitycatalog.server.model.MetadataAndPermissionsSnapshotRequest; +import io.unitycatalog.server.model.MetadataAndPermissionsSnapshotResponse; +import io.unitycatalog.server.model.MetadataSnapshotResponse; +import io.unitycatalog.server.model.MissingReason; +import io.unitycatalog.server.model.SchemaInfo; +import io.unitycatalog.server.model.Securable; +import io.unitycatalog.server.model.SecurableType; +import io.unitycatalog.server.model.TableInfo; +import io.unitycatalog.server.model.TableResult; +import io.unitycatalog.server.persist.CatalogRepository; +import io.unitycatalog.server.persist.MetastoreRepository; +import io.unitycatalog.server.persist.Repositories; +import io.unitycatalog.server.persist.SchemaRepository; +import io.unitycatalog.server.persist.TableRepository; +import io.unitycatalog.server.persist.model.Privileges; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import lombok.SneakyThrows; + +@ExceptionHandler(GlobalExceptionHandler.class) +public class MetadataSnapshotService extends AuthorizedService { + + private final TableRepository tableRepository; + private final CatalogRepository catalogRepository; + private final SchemaRepository schemaRepository; + private final MetastoreRepository metastoreRepository; + + @SneakyThrows + public MetadataSnapshotService(UnityCatalogAuthorizer authorizer, Repositories repositories) { + super(authorizer, repositories); + this.tableRepository = repositories.getTableRepository(); + this.catalogRepository = repositories.getCatalogRepository(); + this.schemaRepository = repositories.getSchemaRepository(); + this.metastoreRepository = repositories.getMetastoreRepository(); + } + + @Post("") + @AuthorizeExpression("#defer") + public HttpResponse getMetadataAndPermissionsSnapshot( + MetadataAndPermissionsSnapshotRequest request) { + if (request == null || request.getSecurables() == null || request.getSecurables().isEmpty()) { + throw new BaseException(ErrorCode.INVALID_ARGUMENT, "At least one securable is required"); + } + + boolean expandDeps = + request.getIncludeViewDependencyExpansion() != null + && request.getIncludeViewDependencyExpansion(); + + MetadataSnapshotResponse innerResponse = new MetadataSnapshotResponse(); + List allTableResults = new ArrayList<>(); + + for (Securable securable : request.getSecurables()) { + if (securable.getType() == SecurableType.TABLE) { + MetadataSnapshotResponse partialResponse = + tableRepository.getMetadataSnapshot(securable.getFullName(), expandDeps); + if (partialResponse.getTables() != null && !partialResponse.getTables().isEmpty()) { + TableResult primaryResult = partialResponse.getTables().get(0); + if (primaryResult.getTable() != null && isAuthorizedForTable(primaryResult.getTable())) { + allTableResults.addAll(partialResponse.getTables()); + } + } + } else { + allTableResults.add( + new TableResult() + .reason( + new MissingReason() + .name(securable.getFullName()) + .reason( + "Unsupported securable type for metadata snapshot: " + + securable.getType()))); + } + } + + innerResponse.setTables(allTableResults); + + MetadataAndPermissionsSnapshotResponse response = + new MetadataAndPermissionsSnapshotResponse().metadata(innerResponse); + return HttpResponse.ofJson(response); + } + + private boolean isAuthorizedForTable(TableInfo tableInfo) { + UUID principalId = userRepository.findPrincipalId(); + UUID metastoreId = metastoreRepository.getMetastoreId(); + + if (authorizer.authorize(principalId, metastoreId, Privileges.OWNER)) { + return true; + } + + CatalogInfo catalogInfo = catalogRepository.getCatalog(tableInfo.getCatalogName()); + UUID catalogId = UUID.fromString(catalogInfo.getId()); + + if (authorizer.authorize(principalId, catalogId, Privileges.OWNER)) { + return true; + } + + SchemaInfo schemaInfo = + schemaRepository.getSchema(tableInfo.getCatalogName() + "." + tableInfo.getSchemaName()); + UUID schemaId = UUID.fromString(schemaInfo.getSchemaId()); + UUID tableId = UUID.fromString(tableInfo.getTableId()); + + if (authorizer.authorize(principalId, schemaId, Privileges.OWNER) + && authorizer.authorize(principalId, catalogId, Privileges.USE_CATALOG)) { + return true; + } + + return authorizer.authorize(principalId, schemaId, Privileges.USE_SCHEMA) + && authorizer.authorize(principalId, catalogId, Privileges.USE_CATALOG) + && (authorizer.authorize(principalId, tableId, Privileges.OWNER) + || authorizer.authorize(principalId, tableId, Privileges.SELECT)); + } +} diff --git a/server/src/main/java/io/unitycatalog/server/service/TableService.java b/server/src/main/java/io/unitycatalog/server/service/TableService.java index 3384732841..fe82504d60 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TableService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TableService.java @@ -15,7 +15,6 @@ import io.unitycatalog.server.model.CatalogInfo; import io.unitycatalog.server.model.CreateTable; import io.unitycatalog.server.model.ListTablesResponse; -import io.unitycatalog.server.model.MetadataSnapshot; import io.unitycatalog.server.model.SchemaInfo; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.persist.CatalogRepository; @@ -124,23 +123,6 @@ public HttpResponse getTable(@Param("full_name") @AuthorizeResourceKey(TABLE) St return HttpResponse.ofJson(tableInfo); } - @Get("/{full_name}/metadata-snapshot") - @AuthorizeExpression(""" - #authorize(#principal, #metastore, OWNER) || - #authorize(#principal, #catalog, OWNER) || - (#authorize(#principal, #schema, OWNER) && #authorize(#principal, #catalog, USE_CATALOG)) || - (#authorize(#principal, #schema, USE_SCHEMA) && - #authorize(#principal, #catalog, USE_CATALOG) && - #authorizeAny(#principal, #table, OWNER, SELECT)) - """) - @AuthorizeResourceKey(METASTORE) - public HttpResponse getMetadataSnapshot( - @Param("full_name") @AuthorizeResourceKey(TABLE) String fullName) { - assert fullName != null; - MetadataSnapshot snapshot = tableRepository.getMetadataSnapshot(fullName); - return HttpResponse.ofJson(snapshot); - } - @Get("") @AuthorizeExpression("#defer") public HttpResponse listTables( diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index 1ec52f2cba..c6210cc2d0 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -10,6 +10,7 @@ import io.unitycatalog.server.exception.BaseException; import io.unitycatalog.server.exception.ErrorCode; import io.unitycatalog.server.exception.GlobalExceptionHandler; +import io.unitycatalog.server.model.Dependent; import io.unitycatalog.server.model.GenerateTemporaryTableCredential; import io.unitycatalog.server.model.TableOperation; import io.unitycatalog.server.persist.DependencyRepository; @@ -81,11 +82,14 @@ public HttpResponse generateTemporaryTableCredential( @AuthorizeKey(key = "dependent") GenerateTemporaryTableCredential generateTemporaryTableCredential) { String tableId = generateTemporaryTableCredential.getTableId(); - String dependentId = generateTemporaryTableCredential.getDependent(); + Dependent dependent = generateTemporaryTableCredential.getDependent(); - if (dependentId != null) { + if (dependent != null && dependent.getTable() != null + && dependent.getTable().getTableId() != null) { return handleDependentCredentialRequest( - tableId, dependentId, generateTemporaryTableCredential.getOperation()); + tableId, + dependent.getTable().getTableId(), + generateTemporaryTableCredential.getOperation()); } NormalizedURL storageLocation = tableRepository.getStorageLocationForTableOrStagingTable( diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java index fd63dc8906..a0d3ecb91d 100644 --- a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java @@ -12,12 +12,18 @@ import io.unitycatalog.client.model.DataSourceFormat; import io.unitycatalog.client.model.Dependency; import io.unitycatalog.client.model.DependencyList; +import io.unitycatalog.client.model.Dependent; import io.unitycatalog.client.model.GenerateTemporaryTableCredential; -import io.unitycatalog.client.model.MetadataSnapshot; +import io.unitycatalog.client.model.MetadataAndPermissionsSnapshotRequest; +import io.unitycatalog.client.model.MetadataAndPermissionsSnapshotResponse; +import io.unitycatalog.client.model.MetadataSnapshotResponse; +import io.unitycatalog.client.model.Securable; import io.unitycatalog.client.model.SecurableType; import io.unitycatalog.client.model.TableDependency; +import io.unitycatalog.client.model.TableDependent; import io.unitycatalog.client.model.TableInfo; import io.unitycatalog.client.model.TableOperation; +import io.unitycatalog.client.model.TableResult; import io.unitycatalog.client.model.TableType; import io.unitycatalog.server.base.ServerConfig; import io.unitycatalog.server.exception.ErrorCode; @@ -70,6 +76,10 @@ public class SdkMetricViewAccessControlTest extends SdkAccessControlBaseCRUDTest .position(0) .nullable(true)); + private static Dependent makeDependentFromView(TableInfo metricView) { + return new Dependent().table(new TableDependent().tableId(metricView.getTableId())); + } + /** * Positive test: view-mediated credential vending succeeds when all three checks pass. * @@ -81,11 +91,9 @@ public void testDefinerRightsPositive() throws Exception { createTestUser(VIEW_OWNER_EMAIL, "View Owner"); createTestUser(READER_EMAIL, "Reader"); - // Create source table as admin first (must exist before granting permissions) TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); - // Grant view owner: catalog/schema access + CREATE_TABLE + SELECT on source table grantViewOwnerBasePermissions(); grantPermissions( VIEW_OWNER_EMAIL, @@ -95,16 +103,13 @@ public void testDefinerRightsPositive() throws Exception { grantPermissions( VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - // Create metric view as view owner (so owner field is set correctly) ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - // Grant reader: catalog/schema access + SELECT on metric view grantReaderBasePermissions(); grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - // Reader requests credentials for source table through the metric view ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); TemporaryCredentialsApi readerTempCredsApi = new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); @@ -113,9 +118,8 @@ public void testDefinerRightsPositive() throws Exception { new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) .operation(TableOperation.READ) - .dependent(metricView.getTableId()); + .dependent(makeDependentFromView(metricView)); - // Should succeed: all three checks pass readerTempCredsApi.generateTemporaryTableCredentials(credRequest); } @@ -145,7 +149,6 @@ public void testDeniedWhenUserLacksSelectOnView() throws Exception { TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - // Grant reader catalog/schema access but NOT SELECT on metric view grantReaderBasePermissions(); ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); @@ -156,7 +159,7 @@ public void testDeniedWhenUserLacksSelectOnView() throws Exception { new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) .operation(TableOperation.READ) - .dependent(metricView.getTableId()); + .dependent(makeDependentFromView(metricView)); assertThatExceptionOfType(ApiException.class) .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) @@ -187,7 +190,6 @@ public void testDeniedWhenSourceNotInDependencies() throws Exception { TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); - // Create a metric view WITHOUT dependencies ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); @@ -200,7 +202,6 @@ public void testDeniedWhenSourceNotInDependencies() throws Exception { .viewDefinition(VIEW_DEFINITION); TableInfo metricView = viewOwnerTablesApi.createTable(createMetricView); - // Grant reader SELECT on metric view grantReaderBasePermissions(); grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); @@ -212,7 +213,7 @@ public void testDeniedWhenSourceNotInDependencies() throws Exception { new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) .operation(TableOperation.READ) - .dependent(metricView.getTableId()); + .dependent(makeDependentFromView(metricView)); assertThatExceptionOfType(ApiException.class) .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) @@ -233,7 +234,6 @@ public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { createTestUser(VIEW_OWNER_EMAIL, "View Owner"); createTestUser(READER_EMAIL, "Reader"); - // Grant view owner catalog/schema access but NOT SELECT on source table grantViewOwnerBasePermissions(); grantPermissions( VIEW_OWNER_EMAIL, @@ -248,7 +248,6 @@ public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - // Grant reader SELECT on metric view grantReaderBasePermissions(); grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); @@ -260,7 +259,7 @@ public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) .operation(TableOperation.READ) - .dependent(metricView.getTableId()); + .dependent(makeDependentFromView(metricView)); assertThatExceptionOfType(ApiException.class) .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) @@ -271,7 +270,7 @@ public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { } /** - * Positive test: metadata snapshot succeeds when the reader has SELECT on the metric view. The + * Positive test: MAPS snapshot succeeds when the reader has SELECT on the metric view. The * response should include the source table's full metadata even though the reader has no direct * SELECT on it (definer's rights for metadata access). */ @@ -302,23 +301,36 @@ public void testMetadataSnapshotPositive() throws Exception { ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); - MetadataSnapshot snapshot = readerTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME); - - assertThat(snapshot.getTableInfo()).isNotNull(); - assertThat(snapshot.getTableInfo().getName()).isEqualTo(METRIC_VIEW_NAME); - assertThat(snapshot.getTableInfo().getTableType()).isEqualTo(TableType.METRIC_VIEW); - - assertThat(snapshot.getDependencyTableInfos()).isNotNull(); - assertThat(snapshot.getDependencyTableInfos()).hasSize(1); - - TableInfo resolvedSource = snapshot.getDependencyTableInfos().get(0); - assertThat(resolvedSource.getName()).isEqualTo(SOURCE_TABLE_NAME); - assertThat(resolvedSource.getStorageLocation()).isNotNull(); - assertThat(resolvedSource.getColumns()).isNotNull(); - assertThat(resolvedSource.getColumns()).hasSizeGreaterThan(0); + MetadataAndPermissionsSnapshotRequest request = + new MetadataAndPermissionsSnapshotRequest() + .securables( + List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) + .includeViewDependencyExpansion(true); + MetadataAndPermissionsSnapshotResponse response = + readerTablesApi.getMetadataAndPermissionsSnapshot(request); + + MetadataSnapshotResponse metadata = response.getMetadata(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getTables()).isNotNull(); + assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(2); + + TableResult viewResult = metadata.getTables().get(0); + assertThat(viewResult.getTable()).isNotNull(); + assertThat(viewResult.getTable().getName()).isEqualTo(METRIC_VIEW_NAME); + assertThat(viewResult.getTable().getTableType()).isEqualTo(TableType.METRIC_VIEW); + + TableResult sourceResult = metadata.getTables().get(1); + assertThat(sourceResult.getTable()).isNotNull(); + assertThat(sourceResult.getTable().getName()).isEqualTo(SOURCE_TABLE_NAME); + assertThat(sourceResult.getTable().getStorageLocation()).isNotNull(); + assertThat(sourceResult.getTable().getColumns()).isNotNull(); + assertThat(sourceResult.getTable().getColumns()).hasSizeGreaterThan(0); } - /** Negative test: metadata snapshot fails when the reader lacks SELECT on the metric view. */ + /** + * Negative test: MAPS snapshot filters out results when the reader lacks SELECT on the metric + * view. The batch endpoint returns an empty tables list rather than a 403. + */ @Test public void testMetadataSnapshotDeniedNoSelectOnView() throws Exception { createTestUser(VIEW_OWNER_EMAIL, "View Owner"); @@ -340,34 +352,49 @@ public void testMetadataSnapshotDeniedNoSelectOnView() throws Exception { TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); createMetricView(viewOwnerTablesApi, sourceTable); - // Grant reader catalog/schema access but NOT SELECT on metric view grantReaderBasePermissions(); ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); - assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> readerTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME)) - .satisfies( - ex -> - assertThat(ex.getCode()) - .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + MetadataAndPermissionsSnapshotRequest request = + new MetadataAndPermissionsSnapshotRequest() + .securables( + List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) + .includeViewDependencyExpansion(true); + MetadataAndPermissionsSnapshotResponse response = + readerTablesApi.getMetadataAndPermissionsSnapshot(request); + + assertThat(response.getMetadata()).isNotNull(); + assertThat(response.getMetadata().getTables()).isEmpty(); } - /** Negative test: metadata snapshot fails when called on a non-METRIC_VIEW table. */ + /** Test: MAPS snapshot on a non-METRIC_VIEW returns the table without dependency expansion. */ @Test - public void testMetadataSnapshotNonMetricViewFails() throws Exception { + public void testMetadataSnapshotNonMetricViewReturnsSingleTable() throws Exception { TablesApi adminTablesApi = new TablesApi(adminApiClient); createSourceTable(adminTablesApi); - assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> adminTablesApi.getMetadataSnapshot(SOURCE_TABLE_FULL_NAME)) - .satisfies(ex -> assertThat(ex.getCode()).isEqualTo(400)); + MetadataAndPermissionsSnapshotRequest request = + new MetadataAndPermissionsSnapshotRequest() + .securables( + List.of(new Securable().type(SecurableType.TABLE).fullName(SOURCE_TABLE_FULL_NAME))) + .includeViewDependencyExpansion(true); + MetadataAndPermissionsSnapshotResponse response = + adminTablesApi.getMetadataAndPermissionsSnapshot(request); + + assertThat(response.getMetadata()).isNotNull(); + assertThat(response.getMetadata().getTables()).hasSize(1); + assertThat(response.getMetadata().getTables().get(0).getTable()).isNotNull(); + assertThat(response.getMetadata().getTables().get(0).getTable().getName()) + .isEqualTo(SOURCE_TABLE_NAME); + assertThat(response.getMetadata().getTables().get(0).getTable().getTableType()) + .isEqualTo(TableType.EXTERNAL); } /** - * Positive test: metadata snapshot resolves full source table metadata including columns and - * storage location, verifying the server-side resolution is complete. + * Positive test: MAPS snapshot resolves full source table metadata including columns and storage + * location, verifying the server-side resolution is complete. */ @Test public void testMetadataSnapshotResolvesSourceMetadata() throws Exception { @@ -375,22 +402,73 @@ public void testMetadataSnapshotResolvesSourceMetadata() throws Exception { TableInfo sourceTable = createSourceTable(adminTablesApi); TableInfo metricView = createMetricView(adminTablesApi, sourceTable); - MetadataSnapshot snapshot = adminTablesApi.getMetadataSnapshot(METRIC_VIEW_FULL_NAME); + MetadataAndPermissionsSnapshotRequest request = + new MetadataAndPermissionsSnapshotRequest() + .securables( + List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) + .includeViewDependencyExpansion(true); + MetadataAndPermissionsSnapshotResponse response = + adminTablesApi.getMetadataAndPermissionsSnapshot(request); + + MetadataSnapshotResponse metadata = response.getMetadata(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(2); + + TableResult viewResult = metadata.getTables().get(0); + assertThat(viewResult.getTable().getTableId()).isEqualTo(metricView.getTableId()); + assertThat(viewResult.getTable().getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(viewResult.getTable().getViewDependencies()).isNotNull(); + + TableResult sourceResult = metadata.getTables().get(1); + assertThat(sourceResult.getTable().getTableId()).isEqualTo(sourceTable.getTableId()); + assertThat(sourceResult.getTable().getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); + assertThat(sourceResult.getTable().getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); + assertThat(sourceResult.getTable().getStorageLocation()) + .contains("uc-test-metric-view/" + SOURCE_TABLE_NAME); + assertThat(sourceResult.getTable().getDataSourceFormat()).isEqualTo(DataSourceFormat.PARQUET); + assertThat(sourceResult.getTable().getColumns()).hasSize(1); + assertThat(sourceResult.getTable().getColumns().get(0).getName()).isEqualTo("amount"); + } - assertThat(snapshot.getTableInfo().getTableId()).isEqualTo(metricView.getTableId()); - assertThat(snapshot.getTableInfo().getViewDefinition()).isEqualTo(VIEW_DEFINITION); - assertThat(snapshot.getTableInfo().getViewDependencies()).isNotNull(); + /** + * Positive test: batch request with multiple securables (metric view + regular table) returns + * correct results for each. + */ + @Test + public void testMetadataSnapshotBatchMultipleSecurables() throws Exception { + TablesApi adminTablesApi = new TablesApi(adminApiClient); + TableInfo sourceTable = createSourceTable(adminTablesApi); + TableInfo metricView = createMetricView(adminTablesApi, sourceTable); - assertThat(snapshot.getDependencyTableInfos()).hasSize(1); - TableInfo resolvedSource = snapshot.getDependencyTableInfos().get(0); - assertThat(resolvedSource.getTableId()).isEqualTo(sourceTable.getTableId()); - assertThat(resolvedSource.getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); - assertThat(resolvedSource.getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); - assertThat(resolvedSource.getStorageLocation()) - .contains("uc-test-metric-view/" + SOURCE_TABLE_NAME); - assertThat(resolvedSource.getDataSourceFormat()).isEqualTo(DataSourceFormat.PARQUET); - assertThat(resolvedSource.getColumns()).hasSize(1); - assertThat(resolvedSource.getColumns().get(0).getName()).isEqualTo("amount"); + MetadataAndPermissionsSnapshotRequest request = + new MetadataAndPermissionsSnapshotRequest() + .securables( + List.of( + new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME), + new Securable().type(SecurableType.TABLE).fullName(SOURCE_TABLE_FULL_NAME))) + .includeViewDependencyExpansion(true); + MetadataAndPermissionsSnapshotResponse response = + adminTablesApi.getMetadataAndPermissionsSnapshot(request); + + MetadataSnapshotResponse metadata = response.getMetadata(); + assertThat(metadata).isNotNull(); + assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(3); + + assertThat(metadata.getTables().get(0).getTable().getName()).isEqualTo(METRIC_VIEW_NAME); + assertThat(metadata.getTables().get(0).getTable().getTableType()) + .isEqualTo(TableType.METRIC_VIEW); + + boolean foundStandaloneSource = false; + for (TableResult tr : metadata.getTables()) { + if (tr.getTable() != null + && SOURCE_TABLE_NAME.equals(tr.getTable().getName()) + && tr.getTable().getTableType() == TableType.EXTERNAL) { + foundStandaloneSource = true; + } + } + assertThat(foundStandaloneSource) + .as("Response should include the standalone source table from the second securable") + .isTrue(); } private TableInfo createSourceTable(TablesApi tablesApi) throws ApiException { From 69e79afd18b4f73b0ab88c2a158cedc48aae28d9 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 13 Apr 2026 15:11:43 +0000 Subject: [PATCH 07/26] Remove definer's rights, adopt invoker's rights per UC TLG decision Per UC TLG decision, external/untrusted engines cannot be trusted to enforce definer's rights. OSS Spark uses invoker's rights: users must have SELECT on both the metric view and all source tables. Removed: - MAPS endpoint (MetadataSnapshotService, route, OpenAPI schemas) - Structured Dependent/TableDependent types from OpenAPI spec - dependent parameter from GenerateTemporaryTableCredential - Definer's rights logic in TemporaryTableCredentialsService - ThreadLocal metadataSnapshotCache in connector - MAPS call and AnalysisContext.setMetricViewId in loadMetricView - Dependent construction in credential vending Kept: - view_definition and view_dependencies on CreateTable/TableInfo - uc_dependencies table and DependencyDAO - DependencyList/Dependency/TableDependency/FunctionDependency schemas - Metric view CRUD and backward compatibility Tests: Rewrote SdkMetricViewAccessControlTest from 9 definer's rights tests to 4 invoker's rights tests (all passing). --- api/all.yaml | 149 ------ .../unitycatalog/spark/UCSingleCatalog.scala | 61 +-- .../server/UnityCatalogServer.java | 7 - .../server/persist/TableRepository.java | 82 ---- .../service/MetadataSnapshotService.java | 123 ----- .../TemporaryTableCredentialsService.java | 145 +----- .../SdkMetricViewAccessControlTest.java | 423 +++--------------- 7 files changed, 72 insertions(+), 918 deletions(-) delete mode 100644 server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java diff --git a/api/all.yaml b/api/all.yaml index 403db5dd8d..547aed5989 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -433,36 +433,6 @@ paths: content: application/json: schema: {} - /metadata-and-permissions-snapshot: - post: - tags: - - Tables - operationId: getMetadataAndPermissionsSnapshot - summary: Get metadata and permissions snapshot for securables - description: | - Returns a batch metadata snapshot for the requested securables, with optional - view dependency expansion. When a securable is a view or metric view and - include_view_dependency_expansion is true, the server resolves all transitive - dependency tables and includes their metadata in the response. This implements - definer's rights for metadata access -- the caller only needs SELECT on the - view, not on the source tables. Compatible with Databricks UC MAPS endpoint. - requestBody: - required: true - content: - application/json: - schema: - $ref: '#/components/schemas/MetadataAndPermissionsSnapshotRequest' - responses: - '200': - description: Metadata and permissions snapshot successfully retrieved. - content: - application/json: - schema: - $ref: '#/components/schemas/MetadataAndPermissionsSnapshotResponse' - '400': - description: Request is invalid (e.g., missing securables). - '403': - description: Caller does not have required permissions. /volumes: post: tags: @@ -2397,14 +2367,6 @@ components: type: string operation: $ref: '#/components/schemas/TableOperation' - dependent: - description: | - Optional. The entity that depends on the table, such as a view - or metric view through which this table is being accessed. - When provided, the server performs view-mediated authorization - instead of direct authorization. Compatible with Databricks UC - Dependent proto wire format. - $ref: '#/components/schemas/Dependent' type: object required: - table_id @@ -3206,117 +3168,6 @@ components: required: - commits - latest_table_version - Securable: - type: object - description: A reference to a securable object for batch metadata requests. - properties: - type: - $ref: '#/components/schemas/SecurableType' - full_name: - description: Full name of the securable (e.g. catalog.schema.table). - type: string - required: - - type - - full_name - MissingReason: - type: object - description: Reason why a requested securable could not be resolved. - properties: - name: - description: Full name of the missing object. - type: string - reason: - description: Error message explaining why the object is missing. - type: string - TableResult: - type: object - description: Result wrapper for a table in a metadata snapshot response. - properties: - table: - $ref: '#/components/schemas/TableInfo' - reason: - $ref: '#/components/schemas/MissingReason' - SchemaResult: - type: object - description: Result wrapper for a schema in a metadata snapshot response. - properties: - schema: - $ref: '#/components/schemas/SchemaInfo' - reason: - $ref: '#/components/schemas/MissingReason' - CatalogResult: - type: object - description: Result wrapper for a catalog in a metadata snapshot response. - properties: - catalog: - $ref: '#/components/schemas/CatalogInfo' - reason: - $ref: '#/components/schemas/MissingReason' - MetadataAndPermissionsSnapshotRequest: - type: object - description: | - Request for a batch metadata and permissions snapshot. Compatible with the - Databricks UC MetadataAndPermissionsSnapshot RPC. - properties: - securables: - description: List of securables for which metadata is requested. - type: array - items: - $ref: '#/components/schemas/Securable' - include_view_dependency_expansion: - description: | - If true, view and metric view dependencies are resolved server-side - and included in the response. The full dependency DAG is traversed - and authorized using definer's rights. - type: boolean - default: false - required: - - securables - MetadataSnapshotResponse: - type: object - description: | - Inner metadata snapshot containing resolved metadata for the requested - securables and optionally their transitive dependencies. - properties: - catalogs: - description: Catalog metadata for the requested securables. - type: array - items: - $ref: '#/components/schemas/CatalogResult' - schemas: - description: Schema metadata for the requested securables. - type: array - items: - $ref: '#/components/schemas/SchemaResult' - tables: - description: Table metadata for the requested securables and dependencies. - type: array - items: - $ref: '#/components/schemas/TableResult' - MetadataAndPermissionsSnapshotResponse: - type: object - description: | - Batch metadata and permissions snapshot response. The metadata field wraps - the resolved metadata for the requested securables. Compatible with the - Databricks UC MetadataAndPermissionsSnapshot.Response wire format. - properties: - metadata: - $ref: '#/components/schemas/MetadataSnapshotResponse' - Dependent: - type: object - description: | - The entity that depends on a table, such as a view or metric view. - Compatible with the Databricks UC Dependent proto wire format. - properties: - table: - $ref: '#/components/schemas/TableDependent' - TableDependent: - type: object - description: A table that is the dependent entity (e.g. a view or metric view). - properties: - table_id: - description: The table ID of the dependent entity. - type: string info: title: Unity Catalog API version: '0.1' diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 5227419b9f..add5540314 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -449,11 +449,6 @@ object UCSingleCatalog { val LOAD_DELTA_CATALOG = ThreadLocal.withInitial[Boolean](() => true) val DELTA_CATALOG_LOADED = ThreadLocal.withInitial[Boolean](() => false) - private[spark] val metadataSnapshotCache: ThreadLocal[java.util.HashMap[String, TableInfo]] = - ThreadLocal.withInitial[java.util.HashMap[String, TableInfo]]( - () => new java.util.HashMap[String, TableInfo]() - ) - /** * Returns any user-configured {@code fs..impl} values from the current Spark session. * @@ -590,16 +585,14 @@ private class UCProxy( override def loadTable(ident: Identifier): Table = { val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) - val t = Option(UCSingleCatalog.metadataSnapshotCache.get().remove(fullName)).getOrElse { - try { - tablesApi.getTable( - fullName, - /* readStreamingTableAsManaged = */ true, - /* readMaterializedViewAsManaged = */ true) - } catch { - case e: ApiException if e.getCode == 404 => - throw new NoSuchTableException(ident) - } + val t = try { + tablesApi.getTable( + fullName, + /* readStreamingTableAsManaged = */ true, + /* readMaterializedViewAsManaged = */ true) + } catch { + case e: ApiException if e.getCode == 404 => + throw new NoSuchTableException(ident) } if (t.getTableType == TableType.METRIC_VIEW) { @@ -619,9 +612,6 @@ private class UCProxy( val tableId = t.getTableId var tableOp = TableOperation.READ_WRITE val credRequest = new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => - credRequest.setDependent(new Dependent().table(new TableDependent().tableId(viewId))) - } val temporaryCredentials = { try { temporaryCredentialsApi.generateTemporaryTableCredentials(credRequest) @@ -632,10 +622,6 @@ private class UCProxy( tableOp = TableOperation.READ val readCredRequest = new GenerateTemporaryTableCredential() .tableId(tableId).operation(tableOp) - org.apache.spark.sql.catalyst.analysis.AnalysisContext.get.metricViewId.foreach { viewId => - readCredRequest.setDependent( - new Dependent().table(new TableDependent().tableId(viewId))) - } temporaryCredentialsApi.generateTemporaryTableCredentials(readCredRequest) } catch { case e: ApiException => @@ -699,37 +685,6 @@ private class UCProxy( Array.empty[StructField] } - try { - val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) - val securable = new Securable() - .`type`(SecurableType.TABLE) - .fullName(fullName) - val request = new MetadataAndPermissionsSnapshotRequest() - .securables(java.util.List.of(securable)) - .includeViewDependencyExpansion(true) - val response = tablesApi.getMetadataAndPermissionsSnapshot(request) - val metadata = response.getMetadata - if (metadata != null && metadata.getTables != null) { - metadata.getTables.asScala.foreach { tableResult => - val depTable = tableResult.getTable - if (depTable != null) { - val depFullName = - s"${depTable.getCatalogName}.${depTable.getSchemaName}.${depTable.getName}" - val requestedFullName = fullName - if (depFullName != requestedFullName) { - UCSingleCatalog.metadataSnapshotCache.get().put(depFullName, depTable) - } - } - } - } - } catch { - case e: Exception => - logWarning(s"Failed to get metadata snapshot for ${t.getName}, " + - s"source table resolution will fall back to direct getTable calls", e) - } - - org.apache.spark.sql.catalyst.analysis.AnalysisContext.setMetricViewId(t.getTableId) - val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) val sparkTable = CatalogTable( diff --git a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java index 23f2d6370a..76dfc3fef4 100644 --- a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java +++ b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java @@ -34,7 +34,6 @@ import io.unitycatalog.server.service.ExternalLocationService; import io.unitycatalog.server.service.FunctionService; import io.unitycatalog.server.service.IcebergRestCatalogService; -import io.unitycatalog.server.service.MetadataSnapshotService; import io.unitycatalog.server.service.MetastoreService; import io.unitycatalog.server.service.ModelService; import io.unitycatalog.server.service.PermissionService; @@ -168,8 +167,6 @@ private void addApiServices( SchemaService schemaService = new SchemaService(authorizer, repositories); VolumeService volumeService = new VolumeService(authorizer, repositories); TableService tableService = new TableService(authorizer, repositories); - MetadataSnapshotService metadataSnapshotService = - new MetadataSnapshotService(authorizer, repositories); StagingTableService stagingTableService = new StagingTableService(authorizer, repositories); FunctionService functionService = new FunctionService(authorizer, repositories); ModelService modelService = new ModelService(authorizer, repositories); @@ -217,10 +214,6 @@ private void addApiServices( .annotatedService(BASE_PATH + "schemas", schemaService, requestConverterFunction) .annotatedService(BASE_PATH + "volumes", volumeService, requestConverterFunction) .annotatedService(BASE_PATH + "tables", tableService, requestConverterFunction) - .annotatedService( - BASE_PATH + "metadata-and-permissions-snapshot", - metadataSnapshotService, - requestConverterFunction) .annotatedService( BASE_PATH + "staging-tables", stagingTableService, requestConverterFunction) .annotatedService(BASE_PATH + "functions", functionService, requestConverterFunction) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index eb71ed069f..6bba8567e7 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -7,10 +7,7 @@ import io.unitycatalog.server.model.DataSourceFormat; import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; -import io.unitycatalog.server.model.MetadataSnapshotResponse; -import io.unitycatalog.server.model.MissingReason; import io.unitycatalog.server.model.TableInfo; -import io.unitycatalog.server.model.TableResult; import io.unitycatalog.server.model.TableType; import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.PropertyDAO; @@ -172,85 +169,6 @@ public TableInfo getTable(String fullName) { /* readOnly = */ true); } - public MetadataSnapshotResponse getMetadataSnapshot( - String fullName, boolean includeViewDependencyExpansion) { - LOGGER.debug("Getting metadata snapshot: {}", fullName); - return TransactionManager.executeWithTransaction( - sessionFactory, - session -> { - String[] parts = fullName.split("\\."); - if (parts.length != 3) { - throw new BaseException(ErrorCode.INVALID_ARGUMENT, "Invalid table name: " + fullName); - } - String catalogName = parts[0]; - String schemaName = parts[1]; - String tableName = parts[2]; - TableInfoDAO tableDAO = findTable(session, catalogName, schemaName, tableName); - - MetadataSnapshotResponse response = new MetadataSnapshotResponse(); - List tableResults = new ArrayList<>(); - - if (tableDAO == null) { - tableResults.add( - new TableResult() - .reason( - new MissingReason().name(fullName).reason("Table not found: " + fullName))); - return response.tables(tableResults); - } - - TableInfo tableInfo = tableDAO.toTableInfo(true, catalogName, schemaName); - RepositoryUtils.attachProperties( - tableInfo, tableInfo.getTableId(), Constants.TABLE, session); - - List deps = - repositories - .getDependencyRepository() - .getDependencies(session, tableDAO.getId(), "TABLE"); - if (!deps.isEmpty()) { - tableInfo.setViewDependencies( - new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); - } - tableResults.add(new TableResult().table(tableInfo)); - - if (includeViewDependencyExpansion - && "METRIC_VIEW".equals(tableDAO.getType()) - && !deps.isEmpty()) { - for (DependencyDAO dep : deps) { - TableInfoDAO depDAO = - findTable( - session, - dep.getDependencyCatalog(), - dep.getDependencySchema(), - dep.getDependencyName()); - if (depDAO != null) { - TableInfo depInfo = - depDAO.toTableInfo(true, dep.getDependencyCatalog(), dep.getDependencySchema()); - RepositoryUtils.attachProperties( - depInfo, depInfo.getTableId(), Constants.TABLE, session); - tableResults.add(new TableResult().table(depInfo)); - } else { - String depFullName = - dep.getDependencyCatalog() - + "." - + dep.getDependencySchema() - + "." - + dep.getDependencyName(); - tableResults.add( - new TableResult() - .reason( - new MissingReason() - .name(depFullName) - .reason("Dependency table not found"))); - } - } - } - - return response.tables(tableResults); - }, - "Failed to get metadata snapshot", - /* readOnly = */ true); - } - public String getTableUniformMetadataLocation( Session session, String catalogName, String schemaName, String tableName) { TableInfoDAO dao = findTable(session, catalogName, schemaName, tableName); diff --git a/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java b/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java deleted file mode 100644 index 3192224e60..0000000000 --- a/server/src/main/java/io/unitycatalog/server/service/MetadataSnapshotService.java +++ /dev/null @@ -1,123 +0,0 @@ -package io.unitycatalog.server.service; - -import com.linecorp.armeria.common.HttpResponse; -import com.linecorp.armeria.server.annotation.ExceptionHandler; -import com.linecorp.armeria.server.annotation.Post; -import io.unitycatalog.server.auth.UnityCatalogAuthorizer; -import io.unitycatalog.server.auth.annotation.AuthorizeExpression; -import io.unitycatalog.server.exception.BaseException; -import io.unitycatalog.server.exception.ErrorCode; -import io.unitycatalog.server.exception.GlobalExceptionHandler; -import io.unitycatalog.server.model.CatalogInfo; -import io.unitycatalog.server.model.MetadataAndPermissionsSnapshotRequest; -import io.unitycatalog.server.model.MetadataAndPermissionsSnapshotResponse; -import io.unitycatalog.server.model.MetadataSnapshotResponse; -import io.unitycatalog.server.model.MissingReason; -import io.unitycatalog.server.model.SchemaInfo; -import io.unitycatalog.server.model.Securable; -import io.unitycatalog.server.model.SecurableType; -import io.unitycatalog.server.model.TableInfo; -import io.unitycatalog.server.model.TableResult; -import io.unitycatalog.server.persist.CatalogRepository; -import io.unitycatalog.server.persist.MetastoreRepository; -import io.unitycatalog.server.persist.Repositories; -import io.unitycatalog.server.persist.SchemaRepository; -import io.unitycatalog.server.persist.TableRepository; -import io.unitycatalog.server.persist.model.Privileges; -import java.util.ArrayList; -import java.util.List; -import java.util.UUID; -import lombok.SneakyThrows; - -@ExceptionHandler(GlobalExceptionHandler.class) -public class MetadataSnapshotService extends AuthorizedService { - - private final TableRepository tableRepository; - private final CatalogRepository catalogRepository; - private final SchemaRepository schemaRepository; - private final MetastoreRepository metastoreRepository; - - @SneakyThrows - public MetadataSnapshotService(UnityCatalogAuthorizer authorizer, Repositories repositories) { - super(authorizer, repositories); - this.tableRepository = repositories.getTableRepository(); - this.catalogRepository = repositories.getCatalogRepository(); - this.schemaRepository = repositories.getSchemaRepository(); - this.metastoreRepository = repositories.getMetastoreRepository(); - } - - @Post("") - @AuthorizeExpression("#defer") - public HttpResponse getMetadataAndPermissionsSnapshot( - MetadataAndPermissionsSnapshotRequest request) { - if (request == null || request.getSecurables() == null || request.getSecurables().isEmpty()) { - throw new BaseException(ErrorCode.INVALID_ARGUMENT, "At least one securable is required"); - } - - boolean expandDeps = - request.getIncludeViewDependencyExpansion() != null - && request.getIncludeViewDependencyExpansion(); - - MetadataSnapshotResponse innerResponse = new MetadataSnapshotResponse(); - List allTableResults = new ArrayList<>(); - - for (Securable securable : request.getSecurables()) { - if (securable.getType() == SecurableType.TABLE) { - MetadataSnapshotResponse partialResponse = - tableRepository.getMetadataSnapshot(securable.getFullName(), expandDeps); - if (partialResponse.getTables() != null && !partialResponse.getTables().isEmpty()) { - TableResult primaryResult = partialResponse.getTables().get(0); - if (primaryResult.getTable() != null && isAuthorizedForTable(primaryResult.getTable())) { - allTableResults.addAll(partialResponse.getTables()); - } - } - } else { - allTableResults.add( - new TableResult() - .reason( - new MissingReason() - .name(securable.getFullName()) - .reason( - "Unsupported securable type for metadata snapshot: " - + securable.getType()))); - } - } - - innerResponse.setTables(allTableResults); - - MetadataAndPermissionsSnapshotResponse response = - new MetadataAndPermissionsSnapshotResponse().metadata(innerResponse); - return HttpResponse.ofJson(response); - } - - private boolean isAuthorizedForTable(TableInfo tableInfo) { - UUID principalId = userRepository.findPrincipalId(); - UUID metastoreId = metastoreRepository.getMetastoreId(); - - if (authorizer.authorize(principalId, metastoreId, Privileges.OWNER)) { - return true; - } - - CatalogInfo catalogInfo = catalogRepository.getCatalog(tableInfo.getCatalogName()); - UUID catalogId = UUID.fromString(catalogInfo.getId()); - - if (authorizer.authorize(principalId, catalogId, Privileges.OWNER)) { - return true; - } - - SchemaInfo schemaInfo = - schemaRepository.getSchema(tableInfo.getCatalogName() + "." + tableInfo.getSchemaName()); - UUID schemaId = UUID.fromString(schemaInfo.getSchemaId()); - UUID tableId = UUID.fromString(tableInfo.getTableId()); - - if (authorizer.authorize(principalId, schemaId, Privileges.OWNER) - && authorizer.authorize(principalId, catalogId, Privileges.USE_CATALOG)) { - return true; - } - - return authorizer.authorize(principalId, schemaId, Privileges.USE_SCHEMA) - && authorizer.authorize(principalId, catalogId, Privileges.USE_CATALOG) - && (authorizer.authorize(principalId, tableId, Privileges.OWNER) - || authorizer.authorize(principalId, tableId, Privileges.SELECT)); - } -} diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index c6210cc2d0..16f50c63d9 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -5,92 +5,53 @@ import com.linecorp.armeria.server.annotation.Post; import io.unitycatalog.server.auth.UnityCatalogAuthorizer; import io.unitycatalog.server.auth.annotation.AuthorizeExpression; -import io.unitycatalog.server.auth.annotation.AuthorizeResourceKey; import io.unitycatalog.server.auth.annotation.AuthorizeKey; -import io.unitycatalog.server.exception.BaseException; -import io.unitycatalog.server.exception.ErrorCode; +import io.unitycatalog.server.auth.annotation.AuthorizeResourceKey; import io.unitycatalog.server.exception.GlobalExceptionHandler; -import io.unitycatalog.server.model.Dependent; import io.unitycatalog.server.model.GenerateTemporaryTableCredential; import io.unitycatalog.server.model.TableOperation; -import io.unitycatalog.server.persist.DependencyRepository; import io.unitycatalog.server.persist.Repositories; -import io.unitycatalog.server.persist.SchemaRepository; import io.unitycatalog.server.persist.TableRepository; -import io.unitycatalog.server.persist.UserRepository; -import io.unitycatalog.server.persist.dao.DependencyDAO; -import io.unitycatalog.server.persist.dao.TableInfoDAO; -import io.unitycatalog.server.persist.model.Privileges; -import io.unitycatalog.server.persist.utils.TransactionManager; import io.unitycatalog.server.service.credential.CredentialContext; import io.unitycatalog.server.service.credential.StorageCredentialVendor; import io.unitycatalog.server.utils.NormalizedURL; import java.util.Collections; -import java.util.List; import java.util.Set; import java.util.UUID; -import org.hibernate.SessionFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import static io.unitycatalog.server.model.SecurableType.TABLE; import static io.unitycatalog.server.service.credential.CredentialContext.Privilege.SELECT; import static io.unitycatalog.server.service.credential.CredentialContext.Privilege.UPDATE; @ExceptionHandler(GlobalExceptionHandler.class) public class TemporaryTableCredentialsService { - private static final Logger LOGGER = - LoggerFactory.getLogger(TemporaryTableCredentialsService.class); private final TableRepository tableRepository; - private final DependencyRepository dependencyRepository; - private final SchemaRepository schemaRepository; - private final UserRepository userRepository; private final StorageCredentialVendor storageCredentialVendor; - private final UnityCatalogAuthorizer authorizer; - private final SessionFactory sessionFactory; public TemporaryTableCredentialsService( StorageCredentialVendor storageCredentialVendor, UnityCatalogAuthorizer authorizer, Repositories repositories) { this.storageCredentialVendor = storageCredentialVendor; - this.authorizer = authorizer; this.tableRepository = repositories.getTableRepository(); - this.dependencyRepository = repositories.getDependencyRepository(); - this.schemaRepository = repositories.getSchemaRepository(); - this.userRepository = repositories.getUserRepository(); - this.sessionFactory = repositories.getSessionFactory(); } @Post("") @AuthorizeExpression(""" - #dependent != null || ( - #authorizeAny(#principal, #schema, OWNER, USE_SCHEMA) && - #authorizeAny(#principal, #catalog, OWNER, USE_CATALOG) && - (#operation == 'READ' - ? #authorizeAny(#principal, #table, OWNER, SELECT) - : (#authorize(#principal, #table, OWNER) || - #authorizeAll(#principal, #table, SELECT, MODIFY))) - ) + #authorizeAny(#principal, #schema, OWNER, USE_SCHEMA) && + #authorizeAny(#principal, #catalog, OWNER, USE_CATALOG) && + (#operation == 'READ' + ? #authorizeAny(#principal, #table, OWNER, SELECT) + : (#authorize(#principal, #table, OWNER) || + #authorizeAll(#principal, #table, SELECT, MODIFY))) """) public HttpResponse generateTemporaryTableCredential( @AuthorizeResourceKey(value = TABLE, key = "table_id") @AuthorizeKey(key = "operation") - @AuthorizeKey(key = "dependent") GenerateTemporaryTableCredential generateTemporaryTableCredential) { String tableId = generateTemporaryTableCredential.getTableId(); - Dependent dependent = generateTemporaryTableCredential.getDependent(); - - if (dependent != null && dependent.getTable() != null - && dependent.getTable().getTableId() != null) { - return handleDependentCredentialRequest( - tableId, - dependent.getTable().getTableId(), - generateTemporaryTableCredential.getOperation()); - } NormalizedURL storageLocation = tableRepository.getStorageLocationForTableOrStagingTable( UUID.fromString(tableId)); @@ -98,98 +59,6 @@ public HttpResponse generateTemporaryTableCredential( tableOperationToPrivileges(generateTemporaryTableCredential.getOperation()))); } - /** - * Handles credential requests that come through a view/metric-view (definer's-rights model). - * - * Authorization checks: - * 1. Current user has SELECT on the dependent (view/metric view) - * 2. The requested table_id is in the dependent's dependency list - * 3. The dependent's owner has SELECT on the source table - */ - private HttpResponse handleDependentCredentialRequest( - String sourceTableId, String dependentId, TableOperation operation) { - return TransactionManager.executeWithTransaction( - sessionFactory, - session -> { - UUID dependentUUID = UUID.fromString(dependentId); - UUID sourceTableUUID = UUID.fromString(sourceTableId); - - TableInfoDAO dependentTable = tableRepository.getTableById(session, dependentUUID); - if (dependentTable == null) { - throw new BaseException( - ErrorCode.NOT_FOUND, "Dependent table not found: " + dependentId); - } - - UUID principalId = userRepository.findPrincipalId(); - if (principalId != null) { - boolean userCanSelectView = authorizer.authorizeAny( - principalId, dependentUUID, Privileges.OWNER, Privileges.SELECT); - if (!userCanSelectView) { - throw new BaseException( - ErrorCode.PERMISSION_DENIED, - "User does not have SELECT permission on the metric view"); - } - } - - List deps = - dependencyRepository.getDependencies(session, dependentUUID, "TABLE"); - boolean sourceInDeps = deps.stream().anyMatch(dep -> { - if (dep.getDependencyTargetId() != null) { - return dep.getDependencyTargetId().equals(sourceTableUUID); - } - String fullName = dep.getDependencyFullName(); - if (fullName != null) { - try { - String[] parts = fullName.split("\\."); - if (parts.length == 3) { - UUID schemaId = schemaRepository.getSchemaIdOrThrow( - session, parts[0], parts[1]); - TableInfoDAO resolved = tableRepository.findBySchemaIdAndName( - session, schemaId, parts[2]); - return resolved != null && resolved.getId().equals(sourceTableUUID); - } - } catch (Exception e) { - LOGGER.warn("Failed to resolve dependency: {}", fullName, e); - } - } - return false; - }); - - if (!sourceInDeps) { - throw new BaseException( - ErrorCode.PERMISSION_DENIED, - "Source table is not a dependency of the specified metric view"); - } - - String viewOwner = dependentTable.getOwner(); - if (viewOwner != null && principalId != null) { - try { - UUID ownerPrincipalId = UUID.fromString( - userRepository.getUserByEmail(viewOwner).getId()); - boolean ownerCanSelectSource = authorizer.authorizeAny( - ownerPrincipalId, sourceTableUUID, Privileges.OWNER, Privileges.SELECT); - if (!ownerCanSelectSource) { - throw new BaseException( - ErrorCode.PERMISSION_DENIED, - "Metric view owner does not have SELECT permission on the source table"); - } - } catch (BaseException e) { - if (e.getErrorCode() == ErrorCode.PERMISSION_DENIED) { - throw e; - } - LOGGER.warn("Could not resolve view owner for auth check: {}", viewOwner); - } - } - - NormalizedURL storageLocation = tableRepository - .getStorageLocationForTableOrStagingTable(sourceTableUUID); - return HttpResponse.ofJson(storageCredentialVendor.vendCredential( - storageLocation, tableOperationToPrivileges(operation))); - }, - "Failed to handle dependent credential request", - /* readOnly = */ true); - } - private Set tableOperationToPrivileges( TableOperation tableOperation) { return switch (tableOperation) { diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java index a0d3ecb91d..d77a832c48 100644 --- a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java +++ b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java @@ -12,18 +12,11 @@ import io.unitycatalog.client.model.DataSourceFormat; import io.unitycatalog.client.model.Dependency; import io.unitycatalog.client.model.DependencyList; -import io.unitycatalog.client.model.Dependent; import io.unitycatalog.client.model.GenerateTemporaryTableCredential; -import io.unitycatalog.client.model.MetadataAndPermissionsSnapshotRequest; -import io.unitycatalog.client.model.MetadataAndPermissionsSnapshotResponse; -import io.unitycatalog.client.model.MetadataSnapshotResponse; -import io.unitycatalog.client.model.Securable; import io.unitycatalog.client.model.SecurableType; import io.unitycatalog.client.model.TableDependency; -import io.unitycatalog.client.model.TableDependent; import io.unitycatalog.client.model.TableInfo; import io.unitycatalog.client.model.TableOperation; -import io.unitycatalog.client.model.TableResult; import io.unitycatalog.client.model.TableType; import io.unitycatalog.server.base.ServerConfig; import io.unitycatalog.server.exception.ErrorCode; @@ -33,25 +26,18 @@ import org.junit.jupiter.api.Test; /** - * Tests the definer's-rights permission model for metric view credential vending. + * Tests the invoker's-rights permission model for metric view access. * - *

When a user queries a metric view, the Spark connector requests temporary credentials for the - * source table with the metric view ID as the {@code dependent} parameter. The server's {@code - * handleDependentCredentialRequest} performs three authorization checks: + *

Under invoker's rights, the user querying a metric view must have SELECT on both the metric + * view AND all source tables. There is no definer's rights bypass -- the user's own permissions are + * checked directly on each table. * - *

    - *
  1. The requesting user has SELECT on the metric view (dependent) - *
  2. The source table is in the metric view's dependency list - *
  3. The metric view's owner has SELECT on the source table (definer's rights) - *
- * - *

This test validates all three checks: one positive case and three negative cases (one for each - * check failing). + *

This matches the behavior of Databricks UC on non-PE (Single User) clusters, where untrusted + * engines cannot be relied upon to enforce definer's rights. */ public class SdkMetricViewAccessControlTest extends SdkAccessControlBaseCRUDTest { - private static final String VIEW_OWNER_EMAIL = "view_owner@test.com"; - private static final String READER_EMAIL = "reader@test.com"; + private static final String USER_EMAIL = "metric_view_user@test.com"; private static final String SOURCE_TABLE_NAME = "source_data"; private static final String SOURCE_TABLE_FULL_NAME = @@ -76,93 +62,60 @@ public class SdkMetricViewAccessControlTest extends SdkAccessControlBaseCRUDTest .position(0) .nullable(true)); - private static Dependent makeDependentFromView(TableInfo metricView) { - return new Dependent().table(new TableDependent().tableId(metricView.getTableId())); - } - /** - * Positive test: view-mediated credential vending succeeds when all three checks pass. - * - *

The view owner has SELECT on the source table, the reader has SELECT on the metric view, and - * the source table is in the metric view's dependency list. + * Positive test: user with SELECT on both the metric view and source table can get credentials + * for the source table (invoker's rights). */ @Test - public void testDefinerRightsPositive() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); + public void testInvokerRightsPositive() throws Exception { + createTestUser(USER_EMAIL, "Test User"); TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); + createMetricView(adminTablesApi, sourceTable); - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); + grantUserBasePermissions(); + grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); + grantPermissions(USER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - grantReaderBasePermissions(); - grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TemporaryCredentialsApi readerTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); + TemporaryCredentialsApi userTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(userConfig)); GenerateTemporaryTableCredential credRequest = new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ) - .dependent(makeDependentFromView(metricView)); + .operation(TableOperation.READ); - readerTempCredsApi.generateTemporaryTableCredentials(credRequest); + userTempCredsApi.generateTemporaryTableCredentials(credRequest); } /** - * Negative test: credential vending fails when the user lacks SELECT on the metric view. - * - *

This validates authorization check #1. + * Negative test: user with SELECT on the metric view but NOT on the source table is denied + * credentials for the source table (invoker's rights -- no definer bypass). */ @Test - public void testDeniedWhenUserLacksSelectOnView() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); + public void testInvokerRightsDeniedWithoutSourceSelect() throws Exception { + createTestUser(USER_EMAIL, "Test User"); TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); + createMetricView(adminTablesApi, sourceTable); - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); - - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - - grantReaderBasePermissions(); + grantUserBasePermissions(); + grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TemporaryCredentialsApi readerTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); + TemporaryCredentialsApi userTempCredsApi = + new TemporaryCredentialsApi(TestUtils.createApiClient(userConfig)); GenerateTemporaryTableCredential credRequest = new GenerateTemporaryTableCredential() .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ) - .dependent(makeDependentFromView(metricView)); + .operation(TableOperation.READ); assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) + .isThrownBy(() -> userTempCredsApi.generateTemporaryTableCredentials(credRequest)) .satisfies( ex -> assertThat(ex.getCode()) @@ -170,307 +123,52 @@ public void testDeniedWhenUserLacksSelectOnView() throws Exception { } /** - * Negative test: credential vending fails when the source table is not in the metric view's - * dependency list. - * - *

This validates authorization check #2. + * Test: user can read metric view metadata (GET /tables) with SELECT on the metric view, even + * without SELECT on the source table. Metadata access is allowed. */ @Test - public void testDeniedWhenSourceNotInDependencies() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); - - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); + public void testMetricViewMetadataAccessible() throws Exception { + createTestUser(USER_EMAIL, "Test User"); TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); + createMetricView(adminTablesApi, sourceTable); - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - - CreateTable createMetricView = - new CreateTable() - .name(METRIC_VIEW_NAME) - .catalogName(TestUtils.CATALOG_NAME) - .schemaName(TestUtils.SCHEMA_NAME) - .tableType(TableType.METRIC_VIEW) - .viewDefinition(VIEW_DEFINITION); - TableInfo metricView = viewOwnerTablesApi.createTable(createMetricView); - - grantReaderBasePermissions(); - grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TemporaryCredentialsApi readerTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + grantUserBasePermissions(); + grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - GenerateTemporaryTableCredential credRequest = - new GenerateTemporaryTableCredential() - .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ) - .dependent(makeDependentFromView(metricView)); + ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); + TablesApi userTablesApi = new TablesApi(TestUtils.createApiClient(userConfig)); - assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) - .satisfies( - ex -> - assertThat(ex.getCode()) - .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); + TableInfo metricViewInfo = userTablesApi.getTable(METRIC_VIEW_FULL_NAME, false, false); + assertThat(metricViewInfo).isNotNull(); + assertThat(metricViewInfo.getTableType()).isEqualTo(TableType.METRIC_VIEW); + assertThat(metricViewInfo.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(metricViewInfo.getViewDependencies()).isNotNull(); } - /** - * Negative test: credential vending fails when the metric view owner lacks SELECT on the source - * table. - * - *

This validates authorization check #3 (definer's rights). - */ + /** Negative test: user without SELECT on the metric view cannot access its metadata. */ @Test - public void testDeniedWhenOwnerLacksSelectOnSource() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); - - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); + public void testMetricViewMetadataDeniedWithoutSelect() throws Exception { + createTestUser(USER_EMAIL, "Test User"); TablesApi adminTablesApi = new TablesApi(adminApiClient); TableInfo sourceTable = createSourceTable(adminTablesApi); + createMetricView(adminTablesApi, sourceTable); - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - - grantReaderBasePermissions(); - grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TemporaryCredentialsApi readerTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(readerConfig)); + grantUserBasePermissions(); - GenerateTemporaryTableCredential credRequest = - new GenerateTemporaryTableCredential() - .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ) - .dependent(makeDependentFromView(metricView)); + ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); + TablesApi userTablesApi = new TablesApi(TestUtils.createApiClient(userConfig)); assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> readerTempCredsApi.generateTemporaryTableCredentials(credRequest)) + .isThrownBy(() -> userTablesApi.getTable(METRIC_VIEW_FULL_NAME, false, false)) .satisfies( ex -> assertThat(ex.getCode()) .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); } - /** - * Positive test: MAPS snapshot succeeds when the reader has SELECT on the metric view. The - * response should include the source table's full metadata even though the reader has no direct - * SELECT on it (definer's rights for metadata access). - */ - @Test - public void testMetadataSnapshotPositive() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - TableInfo metricView = createMetricView(viewOwnerTablesApi, sourceTable); - - grantReaderBasePermissions(); - grantPermissions(READER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); - - MetadataAndPermissionsSnapshotRequest request = - new MetadataAndPermissionsSnapshotRequest() - .securables( - List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) - .includeViewDependencyExpansion(true); - MetadataAndPermissionsSnapshotResponse response = - readerTablesApi.getMetadataAndPermissionsSnapshot(request); - - MetadataSnapshotResponse metadata = response.getMetadata(); - assertThat(metadata).isNotNull(); - assertThat(metadata.getTables()).isNotNull(); - assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(2); - - TableResult viewResult = metadata.getTables().get(0); - assertThat(viewResult.getTable()).isNotNull(); - assertThat(viewResult.getTable().getName()).isEqualTo(METRIC_VIEW_NAME); - assertThat(viewResult.getTable().getTableType()).isEqualTo(TableType.METRIC_VIEW); - - TableResult sourceResult = metadata.getTables().get(1); - assertThat(sourceResult.getTable()).isNotNull(); - assertThat(sourceResult.getTable().getName()).isEqualTo(SOURCE_TABLE_NAME); - assertThat(sourceResult.getTable().getStorageLocation()).isNotNull(); - assertThat(sourceResult.getTable().getColumns()).isNotNull(); - assertThat(sourceResult.getTable().getColumns()).hasSizeGreaterThan(0); - } - - /** - * Negative test: MAPS snapshot filters out results when the reader lacks SELECT on the metric - * view. The batch endpoint returns an empty tables list rather than a 403. - */ - @Test - public void testMetadataSnapshotDeniedNoSelectOnView() throws Exception { - createTestUser(VIEW_OWNER_EMAIL, "View Owner"); - createTestUser(READER_EMAIL, "Reader"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - - grantViewOwnerBasePermissions(); - grantPermissions( - VIEW_OWNER_EMAIL, - SecurableType.SCHEMA, - TestUtils.SCHEMA_FULL_NAME, - Privileges.CREATE_TABLE); - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - - ServerConfig viewOwnerConfig = createTestUserServerConfig(VIEW_OWNER_EMAIL); - TablesApi viewOwnerTablesApi = new TablesApi(TestUtils.createApiClient(viewOwnerConfig)); - createMetricView(viewOwnerTablesApi, sourceTable); - - grantReaderBasePermissions(); - - ServerConfig readerConfig = createTestUserServerConfig(READER_EMAIL); - TablesApi readerTablesApi = new TablesApi(TestUtils.createApiClient(readerConfig)); - - MetadataAndPermissionsSnapshotRequest request = - new MetadataAndPermissionsSnapshotRequest() - .securables( - List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) - .includeViewDependencyExpansion(true); - MetadataAndPermissionsSnapshotResponse response = - readerTablesApi.getMetadataAndPermissionsSnapshot(request); - - assertThat(response.getMetadata()).isNotNull(); - assertThat(response.getMetadata().getTables()).isEmpty(); - } - - /** Test: MAPS snapshot on a non-METRIC_VIEW returns the table without dependency expansion. */ - @Test - public void testMetadataSnapshotNonMetricViewReturnsSingleTable() throws Exception { - TablesApi adminTablesApi = new TablesApi(adminApiClient); - createSourceTable(adminTablesApi); - - MetadataAndPermissionsSnapshotRequest request = - new MetadataAndPermissionsSnapshotRequest() - .securables( - List.of(new Securable().type(SecurableType.TABLE).fullName(SOURCE_TABLE_FULL_NAME))) - .includeViewDependencyExpansion(true); - MetadataAndPermissionsSnapshotResponse response = - adminTablesApi.getMetadataAndPermissionsSnapshot(request); - - assertThat(response.getMetadata()).isNotNull(); - assertThat(response.getMetadata().getTables()).hasSize(1); - assertThat(response.getMetadata().getTables().get(0).getTable()).isNotNull(); - assertThat(response.getMetadata().getTables().get(0).getTable().getName()) - .isEqualTo(SOURCE_TABLE_NAME); - assertThat(response.getMetadata().getTables().get(0).getTable().getTableType()) - .isEqualTo(TableType.EXTERNAL); - } - - /** - * Positive test: MAPS snapshot resolves full source table metadata including columns and storage - * location, verifying the server-side resolution is complete. - */ - @Test - public void testMetadataSnapshotResolvesSourceMetadata() throws Exception { - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - TableInfo metricView = createMetricView(adminTablesApi, sourceTable); - - MetadataAndPermissionsSnapshotRequest request = - new MetadataAndPermissionsSnapshotRequest() - .securables( - List.of(new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME))) - .includeViewDependencyExpansion(true); - MetadataAndPermissionsSnapshotResponse response = - adminTablesApi.getMetadataAndPermissionsSnapshot(request); - - MetadataSnapshotResponse metadata = response.getMetadata(); - assertThat(metadata).isNotNull(); - assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(2); - - TableResult viewResult = metadata.getTables().get(0); - assertThat(viewResult.getTable().getTableId()).isEqualTo(metricView.getTableId()); - assertThat(viewResult.getTable().getViewDefinition()).isEqualTo(VIEW_DEFINITION); - assertThat(viewResult.getTable().getViewDependencies()).isNotNull(); - - TableResult sourceResult = metadata.getTables().get(1); - assertThat(sourceResult.getTable().getTableId()).isEqualTo(sourceTable.getTableId()); - assertThat(sourceResult.getTable().getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); - assertThat(sourceResult.getTable().getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); - assertThat(sourceResult.getTable().getStorageLocation()) - .contains("uc-test-metric-view/" + SOURCE_TABLE_NAME); - assertThat(sourceResult.getTable().getDataSourceFormat()).isEqualTo(DataSourceFormat.PARQUET); - assertThat(sourceResult.getTable().getColumns()).hasSize(1); - assertThat(sourceResult.getTable().getColumns().get(0).getName()).isEqualTo("amount"); - } - - /** - * Positive test: batch request with multiple securables (metric view + regular table) returns - * correct results for each. - */ - @Test - public void testMetadataSnapshotBatchMultipleSecurables() throws Exception { - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - TableInfo metricView = createMetricView(adminTablesApi, sourceTable); - - MetadataAndPermissionsSnapshotRequest request = - new MetadataAndPermissionsSnapshotRequest() - .securables( - List.of( - new Securable().type(SecurableType.TABLE).fullName(METRIC_VIEW_FULL_NAME), - new Securable().type(SecurableType.TABLE).fullName(SOURCE_TABLE_FULL_NAME))) - .includeViewDependencyExpansion(true); - MetadataAndPermissionsSnapshotResponse response = - adminTablesApi.getMetadataAndPermissionsSnapshot(request); - - MetadataSnapshotResponse metadata = response.getMetadata(); - assertThat(metadata).isNotNull(); - assertThat(metadata.getTables()).hasSizeGreaterThanOrEqualTo(3); - - assertThat(metadata.getTables().get(0).getTable().getName()).isEqualTo(METRIC_VIEW_NAME); - assertThat(metadata.getTables().get(0).getTable().getTableType()) - .isEqualTo(TableType.METRIC_VIEW); - - boolean foundStandaloneSource = false; - for (TableResult tr : metadata.getTables()) { - if (tr.getTable() != null - && SOURCE_TABLE_NAME.equals(tr.getTable().getName()) - && tr.getTable().getTableType() == TableType.EXTERNAL) { - foundStandaloneSource = true; - } - } - assertThat(foundStandaloneSource) - .as("Response should include the standalone source table from the second securable") - .isTrue(); - } - private TableInfo createSourceTable(TablesApi tablesApi) throws ApiException { CreateTable createTable = new CreateTable() @@ -502,17 +200,10 @@ private TableInfo createMetricView(TablesApi tablesApi, TableInfo sourceTable) return tablesApi.createTable(createMetricView); } - private void grantViewOwnerBasePermissions() throws Exception { - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); - grantPermissions( - VIEW_OWNER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); - } - - private void grantReaderBasePermissions() throws Exception { + private void grantUserBasePermissions() throws Exception { grantPermissions( - READER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); + USER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); grantPermissions( - READER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); + USER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); } } From 25aed48a99592b82fadbd8595f6939d28504db27 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 13 Apr 2026 15:44:43 +0000 Subject: [PATCH 08/26] Remove unnecessary @Repeatable AuthorizeKey changes The @Repeatable annotation and AuthorizeKeys container were only needed for the definer's rights implementation (multiple @AuthorizeKey on the credential vending parameter). No longer needed with invoker's rights. --- .../server/auth/annotation/AuthorizeKey.java | 2 -- .../server/auth/annotation/AuthorizeKeys.java | 13 ------------- 2 files changed, 15 deletions(-) delete mode 100644 server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java diff --git a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java index 74bd390eee..8bde310f85 100644 --- a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java +++ b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKey.java @@ -1,7 +1,6 @@ package io.unitycatalog.server.auth.annotation; import java.lang.annotation.ElementType; -import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @@ -57,7 +56,6 @@ */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.PARAMETER) -@Repeatable(AuthorizeKeys.class) public @interface AuthorizeKey { /** * The key path to extract from the request payload. Supports nested fields using dot notation diff --git a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java b/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java deleted file mode 100644 index 3256cb526b..0000000000 --- a/server/src/main/java/io/unitycatalog/server/auth/annotation/AuthorizeKeys.java +++ /dev/null @@ -1,13 +0,0 @@ -package io.unitycatalog.server.auth.annotation; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** Container annotation for repeatable {@link AuthorizeKey} annotations. */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.PARAMETER) -public @interface AuthorizeKeys { - AuthorizeKey[] value(); -} From f59b9c1fe2aa0349ff5d5bda4c6cbefb3d537071 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 13 Apr 2026 15:47:48 +0000 Subject: [PATCH 09/26] Revert unnecessary TemporaryTableCredentialsService changes The original file had no functional changes from definer's rights removal -- only cosmetic diffs (import reordering, whitespace, unused authorizer parameter). Revert to main version to keep the diff clean. --- .../io/unitycatalog/server/UnityCatalogServer.java | 2 +- .../service/TemporaryTableCredentialsService.java | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java index 76dfc3fef4..923c3f98b3 100644 --- a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java +++ b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java @@ -177,7 +177,7 @@ private void addApiServices( MetastoreService metastoreService = new MetastoreService(repositories); // TODO: combine these into a single service in a follow-up PR TemporaryTableCredentialsService temporaryTableCredentialsService = - new TemporaryTableCredentialsService(storageCredentialVendor, authorizer, repositories); + new TemporaryTableCredentialsService(storageCredentialVendor, repositories); TemporaryVolumeCredentialsService temporaryVolumeCredentialsService = new TemporaryVolumeCredentialsService(storageCredentialVendor, repositories); TemporaryModelVersionCredentialsService temporaryModelVersionCredentialsService = diff --git a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java index 16f50c63d9..170321e7cc 100644 --- a/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java +++ b/server/src/main/java/io/unitycatalog/server/service/TemporaryTableCredentialsService.java @@ -3,10 +3,9 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.server.annotation.ExceptionHandler; import com.linecorp.armeria.server.annotation.Post; -import io.unitycatalog.server.auth.UnityCatalogAuthorizer; import io.unitycatalog.server.auth.annotation.AuthorizeExpression; -import io.unitycatalog.server.auth.annotation.AuthorizeKey; import io.unitycatalog.server.auth.annotation.AuthorizeResourceKey; +import io.unitycatalog.server.auth.annotation.AuthorizeKey; import io.unitycatalog.server.exception.GlobalExceptionHandler; import io.unitycatalog.server.model.GenerateTemporaryTableCredential; import io.unitycatalog.server.model.TableOperation; @@ -26,14 +25,11 @@ @ExceptionHandler(GlobalExceptionHandler.class) public class TemporaryTableCredentialsService { - private final TableRepository tableRepository; private final StorageCredentialVendor storageCredentialVendor; - public TemporaryTableCredentialsService( - StorageCredentialVendor storageCredentialVendor, - UnityCatalogAuthorizer authorizer, - Repositories repositories) { + public TemporaryTableCredentialsService(StorageCredentialVendor storageCredentialVendor, + Repositories repositories) { this.storageCredentialVendor = storageCredentialVendor; this.tableRepository = repositories.getTableRepository(); } @@ -52,11 +48,10 @@ public HttpResponse generateTemporaryTableCredential( @AuthorizeKey(key = "operation") GenerateTemporaryTableCredential generateTemporaryTableCredential) { String tableId = generateTemporaryTableCredential.getTableId(); - NormalizedURL storageLocation = tableRepository.getStorageLocationForTableOrStagingTable( UUID.fromString(tableId)); return HttpResponse.ofJson(storageCredentialVendor.vendCredential(storageLocation, - tableOperationToPrivileges(generateTemporaryTableCredential.getOperation()))); + tableOperationToPrivileges(generateTemporaryTableCredential.getOperation()))); } private Set tableOperationToPrivileges( From a75eb9640b3317ad38dcc77491628620e3d7cb52 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 13 Apr 2026 15:54:14 +0000 Subject: [PATCH 10/26] Remove SdkMetricViewAccessControlTest With invoker's rights, metric views use the same standard permission checks as regular tables. There is no metric-view-specific permission logic to test -- the existing table access control tests already cover this behavior. --- .../SdkMetricViewAccessControlTest.java | 209 ------------------ 1 file changed, 209 deletions(-) delete mode 100644 server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java diff --git a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java b/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java deleted file mode 100644 index d77a832c48..0000000000 --- a/server/src/test/java/io/unitycatalog/server/sdk/access/SdkMetricViewAccessControlTest.java +++ /dev/null @@ -1,209 +0,0 @@ -package io.unitycatalog.server.sdk.access; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; - -import io.unitycatalog.client.ApiException; -import io.unitycatalog.client.api.TablesApi; -import io.unitycatalog.client.api.TemporaryCredentialsApi; -import io.unitycatalog.client.model.ColumnInfo; -import io.unitycatalog.client.model.ColumnTypeName; -import io.unitycatalog.client.model.CreateTable; -import io.unitycatalog.client.model.DataSourceFormat; -import io.unitycatalog.client.model.Dependency; -import io.unitycatalog.client.model.DependencyList; -import io.unitycatalog.client.model.GenerateTemporaryTableCredential; -import io.unitycatalog.client.model.SecurableType; -import io.unitycatalog.client.model.TableDependency; -import io.unitycatalog.client.model.TableInfo; -import io.unitycatalog.client.model.TableOperation; -import io.unitycatalog.client.model.TableType; -import io.unitycatalog.server.base.ServerConfig; -import io.unitycatalog.server.exception.ErrorCode; -import io.unitycatalog.server.persist.model.Privileges; -import io.unitycatalog.server.utils.TestUtils; -import java.util.List; -import org.junit.jupiter.api.Test; - -/** - * Tests the invoker's-rights permission model for metric view access. - * - *

Under invoker's rights, the user querying a metric view must have SELECT on both the metric - * view AND all source tables. There is no definer's rights bypass -- the user's own permissions are - * checked directly on each table. - * - *

This matches the behavior of Databricks UC on non-PE (Single User) clusters, where untrusted - * engines cannot be relied upon to enforce definer's rights. - */ -public class SdkMetricViewAccessControlTest extends SdkAccessControlBaseCRUDTest { - - private static final String USER_EMAIL = "metric_view_user@test.com"; - - private static final String SOURCE_TABLE_NAME = "source_data"; - private static final String SOURCE_TABLE_FULL_NAME = - TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + SOURCE_TABLE_NAME; - - private static final String METRIC_VIEW_NAME = "test_metric_view"; - private static final String METRIC_VIEW_FULL_NAME = - TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + METRIC_VIEW_NAME; - - private static final String VIEW_DEFINITION = - "version: \"0.1\"\nsource: " - + SOURCE_TABLE_FULL_NAME - + "\nmeasures:\n - name: total\n expr: SUM(amount)"; - - private static final List SOURCE_COLUMNS = - List.of( - new ColumnInfo() - .name("amount") - .typeText("DOUBLE") - .typeJson("{\"type\": \"double\"}") - .typeName(ColumnTypeName.DOUBLE) - .position(0) - .nullable(true)); - - /** - * Positive test: user with SELECT on both the metric view and source table can get credentials - * for the source table (invoker's rights). - */ - @Test - public void testInvokerRightsPositive() throws Exception { - createTestUser(USER_EMAIL, "Test User"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - createMetricView(adminTablesApi, sourceTable); - - grantUserBasePermissions(); - grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - grantPermissions(USER_EMAIL, SecurableType.TABLE, SOURCE_TABLE_FULL_NAME, Privileges.SELECT); - - ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); - TemporaryCredentialsApi userTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(userConfig)); - - GenerateTemporaryTableCredential credRequest = - new GenerateTemporaryTableCredential() - .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ); - - userTempCredsApi.generateTemporaryTableCredentials(credRequest); - } - - /** - * Negative test: user with SELECT on the metric view but NOT on the source table is denied - * credentials for the source table (invoker's rights -- no definer bypass). - */ - @Test - public void testInvokerRightsDeniedWithoutSourceSelect() throws Exception { - createTestUser(USER_EMAIL, "Test User"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - createMetricView(adminTablesApi, sourceTable); - - grantUserBasePermissions(); - grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); - TemporaryCredentialsApi userTempCredsApi = - new TemporaryCredentialsApi(TestUtils.createApiClient(userConfig)); - - GenerateTemporaryTableCredential credRequest = - new GenerateTemporaryTableCredential() - .tableId(sourceTable.getTableId()) - .operation(TableOperation.READ); - - assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> userTempCredsApi.generateTemporaryTableCredentials(credRequest)) - .satisfies( - ex -> - assertThat(ex.getCode()) - .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); - } - - /** - * Test: user can read metric view metadata (GET /tables) with SELECT on the metric view, even - * without SELECT on the source table. Metadata access is allowed. - */ - @Test - public void testMetricViewMetadataAccessible() throws Exception { - createTestUser(USER_EMAIL, "Test User"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - createMetricView(adminTablesApi, sourceTable); - - grantUserBasePermissions(); - grantPermissions(USER_EMAIL, SecurableType.TABLE, METRIC_VIEW_FULL_NAME, Privileges.SELECT); - - ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); - TablesApi userTablesApi = new TablesApi(TestUtils.createApiClient(userConfig)); - - TableInfo metricViewInfo = userTablesApi.getTable(METRIC_VIEW_FULL_NAME, false, false); - assertThat(metricViewInfo).isNotNull(); - assertThat(metricViewInfo.getTableType()).isEqualTo(TableType.METRIC_VIEW); - assertThat(metricViewInfo.getViewDefinition()).isEqualTo(VIEW_DEFINITION); - assertThat(metricViewInfo.getViewDependencies()).isNotNull(); - } - - /** Negative test: user without SELECT on the metric view cannot access its metadata. */ - @Test - public void testMetricViewMetadataDeniedWithoutSelect() throws Exception { - createTestUser(USER_EMAIL, "Test User"); - - TablesApi adminTablesApi = new TablesApi(adminApiClient); - TableInfo sourceTable = createSourceTable(adminTablesApi); - createMetricView(adminTablesApi, sourceTable); - - grantUserBasePermissions(); - - ServerConfig userConfig = createTestUserServerConfig(USER_EMAIL); - TablesApi userTablesApi = new TablesApi(TestUtils.createApiClient(userConfig)); - - assertThatExceptionOfType(ApiException.class) - .isThrownBy(() -> userTablesApi.getTable(METRIC_VIEW_FULL_NAME, false, false)) - .satisfies( - ex -> - assertThat(ex.getCode()) - .isEqualTo(ErrorCode.PERMISSION_DENIED.getHttpStatus().code())); - } - - private TableInfo createSourceTable(TablesApi tablesApi) throws ApiException { - CreateTable createTable = - new CreateTable() - .name(SOURCE_TABLE_NAME) - .catalogName(TestUtils.CATALOG_NAME) - .schemaName(TestUtils.SCHEMA_NAME) - .columns(SOURCE_COLUMNS) - .tableType(TableType.EXTERNAL) - .dataSourceFormat(DataSourceFormat.PARQUET) - .storageLocation("file:///tmp/uc-test-metric-view/" + SOURCE_TABLE_NAME); - return tablesApi.createTable(createTable); - } - - private TableInfo createMetricView(TablesApi tablesApi, TableInfo sourceTable) - throws ApiException { - Dependency dep = new Dependency(); - dep.setTable(new TableDependency().tableFullName(SOURCE_TABLE_FULL_NAME)); - DependencyList depList = new DependencyList(); - depList.setDependencies(List.of(dep)); - - CreateTable createMetricView = - new CreateTable() - .name(METRIC_VIEW_NAME) - .catalogName(TestUtils.CATALOG_NAME) - .schemaName(TestUtils.SCHEMA_NAME) - .tableType(TableType.METRIC_VIEW) - .viewDefinition(VIEW_DEFINITION) - .viewDependencies(depList); - return tablesApi.createTable(createMetricView); - } - - private void grantUserBasePermissions() throws Exception { - grantPermissions( - USER_EMAIL, SecurableType.CATALOG, TestUtils.CATALOG_NAME, Privileges.USE_CATALOG); - grantPermissions( - USER_EMAIL, SecurableType.SCHEMA, TestUtils.SCHEMA_FULL_NAME, Privileges.USE_SCHEMA); - } -} From 10ff1d8e159bd73c166a59cc6cda9a0ff5eef554 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Mon, 13 Apr 2026 16:03:03 +0000 Subject: [PATCH 11/26] Clean up connector: revert unrelated Guava and cosmetic changes Revert Guava import changes (org.sparkproject.guava -> com.google.common) and credential vending reformatting that were unrelated to metric views. The connector diff now only contains metric view additions: - createTable(TableInfo) override with METRIC_VIEW routing - loadMetricView method returning V1Table with YAML - createMetricViewFromTableInfo for V2 catalog create path - METRIC_VIEW detection in loadTable --- .../spark/auth/CredPropsUtil.java | 4 +-- .../auth/storage/AwsVendedTokenProvider.java | 2 +- .../auth/storage/GcsVendedTokenProvider.java | 2 +- .../storage/GenericCredentialProvider.java | 6 ++-- .../spark/fs/CredScopedFileSystem.java | 4 +-- .../unitycatalog/spark/UCSingleCatalog.scala | 29 +++++++++++++------ .../spark/AzureCredentialTestFileSystem.java | 2 +- .../auth/storage/BaseCredRenewITTest.java | 4 +-- 8 files changed, 32 insertions(+), 21 deletions(-) diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java index f707d0daff..20dbdd0e3d 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/CredPropsUtil.java @@ -4,8 +4,6 @@ import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_ACCOUNT_IS_HNS_ENABLED; import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; import io.unitycatalog.client.auth.TokenProvider; import io.unitycatalog.client.model.AwsCredentials; import io.unitycatalog.client.model.AzureUserDelegationSAS; @@ -21,6 +19,8 @@ import io.unitycatalog.spark.fs.CredScopedFs; import java.util.Map; import java.util.UUID; +import org.sparkproject.guava.base.Preconditions; +import org.sparkproject.guava.collect.ImmutableMap; public class CredPropsUtil { private CredPropsUtil() {} diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java index b0e3481b88..4b461527b9 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/AwsVendedTokenProvider.java @@ -1,8 +1,8 @@ package io.unitycatalog.spark.auth.storage; -import com.google.common.base.Preconditions; import io.unitycatalog.spark.UCHadoopConf; import org.apache.hadoop.conf.Configuration; +import org.sparkproject.guava.base.Preconditions; import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java index 357f45628f..99ea6621a3 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GcsVendedTokenProvider.java @@ -1,12 +1,12 @@ package io.unitycatalog.spark.auth.storage; import com.google.cloud.hadoop.util.AccessTokenProvider; -import com.google.common.base.Preconditions; import io.unitycatalog.client.model.GcpOauthToken; import io.unitycatalog.spark.UCHadoopConf; import java.io.IOException; import java.time.Instant; import org.apache.hadoop.conf.Configuration; +import org.sparkproject.guava.base.Preconditions; public class GcsVendedTokenProvider extends GenericCredentialProvider implements AccessTokenProvider { diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java index 28dd643aed..2ac23ca98d 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/auth/storage/GenericCredentialProvider.java @@ -1,8 +1,5 @@ package io.unitycatalog.spark.auth.storage; -import com.google.common.base.Preconditions; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; import io.unitycatalog.client.ApiException; import io.unitycatalog.client.api.TemporaryCredentialsApi; import io.unitycatalog.client.auth.TokenProvider; @@ -17,6 +14,9 @@ import io.unitycatalog.spark.UCHadoopConf; import java.net.URI; import org.apache.hadoop.conf.Configuration; +import org.sparkproject.guava.base.Preconditions; +import org.sparkproject.guava.cache.Cache; +import org.sparkproject.guava.cache.CacheBuilder; public abstract class GenericCredentialProvider { // The credential cache, for saving QPS to unity catalog server. diff --git a/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java b/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java index c5240c33d4..481f08cd79 100644 --- a/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java +++ b/connectors/spark/src/main/java/io/unitycatalog/spark/fs/CredScopedFileSystem.java @@ -1,13 +1,13 @@ package io.unitycatalog.spark.fs; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; import java.io.IOException; import java.net.URI; import java.util.concurrent.ExecutionException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FilterFileSystem; +import org.sparkproject.guava.cache.Cache; +import org.sparkproject.guava.cache.CacheBuilder; /** * A Hadoop {@link FileSystem} wrapper that enables multiple credential scopes to coexist within a diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index add5540314..aa79846b27 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -18,7 +18,7 @@ import org.apache.spark.sql.connector.catalog._ import org.apache.spark.sql.connector.expressions.Transform import org.apache.spark.sql.types._ import org.apache.spark.sql.util.CaseInsensitiveStringMap -import com.google.common.base.Preconditions +import org.sparkproject.guava.base.Preconditions import java.net.URI import java.util @@ -132,6 +132,8 @@ class UCSingleCatalog val newProps = prepareExternalTableProperties(properties) delegate.createTable(ident, columns, partitions, newProps) } else { + // TODO: for path-based tables, Spark should generate a location property using the qualified + // path string. delegate.createTable(ident, columns, partitions, properties) } } @@ -584,10 +586,9 @@ private class UCProxy( } override def loadTable(ident: Identifier): Table = { - val fullName = UCSingleCatalog.fullTableNameForApi(this.name, ident) val t = try { tablesApi.getTable( - fullName, + UCSingleCatalog.fullTableNameForApi(this.name, ident), /* readStreamingTableAsManaged = */ true, /* readMaterializedViewAsManaged = */ true) } catch { @@ -611,18 +612,25 @@ private class UCProxy( val locationUri = CatalogUtils.stringToURI(t.getStorageLocation) val tableId = t.getTableId var tableOp = TableOperation.READ_WRITE - val credRequest = new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) val temporaryCredentials = { try { - temporaryCredentialsApi.generateTemporaryTableCredentials(credRequest) - } catch { + temporaryCredentialsApi + .generateTemporaryTableCredentials( + // TODO: at this time, we don't know if the table will be read or written. For now we always + // request READ_WRITE credentials as the server doesn't distinguish between READ and + // READ_WRITE credentials as of today. When loading a table, Spark should tell if it's + // for read or write, we can request the proper credential after fixing Spark. + new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) + ) + } catch { case e: ApiException => logWarning(s"READ_WRITE credential generation failed for table $identifier: ${e.getMessage}") try { tableOp = TableOperation.READ - val readCredRequest = new GenerateTemporaryTableCredential() - .tableId(tableId).operation(tableOp) - temporaryCredentialsApi.generateTemporaryTableCredentials(readCredRequest) + temporaryCredentialsApi + .generateTemporaryTableCredentials( + new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) + ) } catch { case e: ApiException => logWarning(s"READ credential generation failed for table $identifier: ${e.getMessage}") @@ -668,6 +676,9 @@ private class UCProxy( tracksPartitionsInCatalog = false, partitionColumnNames = partitionCols.sortBy(_._2).map(_._1).toSeq ) + // Spark separates table lookup and data source resolution. To support Spark native data + // sources, here we return the `V1Table` which only contains the table metadata. Spark will + // resolve the data source and create scan node later. Class.forName("org.apache.spark.sql.connector.catalog.V1Table") .getDeclaredConstructor(classOf[CatalogTable]) .newInstance(sparkTable) diff --git a/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java b/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java index e4df6a7fca..a812dfd5e1 100644 --- a/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java +++ b/connectors/spark/src/test/java/io/unitycatalog/spark/AzureCredentialTestFileSystem.java @@ -3,10 +3,10 @@ import static io.unitycatalog.spark.UCHadoopConf.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE; import static org.assertj.core.api.Assertions.assertThat; -import com.google.common.base.Objects; import io.unitycatalog.spark.auth.storage.AbfsVendedTokenProvider; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; +import org.sparkproject.guava.base.Objects; public class AzureCredentialTestFileSystem extends CredentialTestFileSystem { private volatile AbfsVendedTokenProvider provider; diff --git a/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java b/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java index 3b750b1a43..daedb31da7 100644 --- a/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java +++ b/connectors/spark/src/test/java/io/unitycatalog/spark/auth/storage/BaseCredRenewITTest.java @@ -3,8 +3,6 @@ import static io.unitycatalog.server.utils.TestUtils.createApiClient; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterators; import io.delta.tables.DeltaTable; import io.unitycatalog.client.internal.Clock; import io.unitycatalog.client.model.CreateCatalog; @@ -43,6 +41,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.sparkproject.guava.collect.ImmutableList; +import org.sparkproject.guava.collect.Iterators; /** * Integration test to verify that cloud vendor credential renewal works as expected. From 1b801641c41d29ec9dd0133079634441f74614b1 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 14 Apr 2026 19:41:30 +0000 Subject: [PATCH 12/26] Remove server-side dependency persistence OSS UC no longer persists view dependencies (no uc_dependencies table). Under invoker's rights, Spark resolves dependencies at query time by parsing the YAML. The view_dependencies field is accepted in CreateTable payloads for wire compatibility with Databricks UC but not persisted. Removed: - DependencyDAO.java and DependencyRepository.java - DependencyRepository from Repositories.java - DependencyDAO from HibernateConfigurator - Dependency create/read/delete logic in TableRepository - Dependency assertions in BaseMetricViewCRUDTest (now tests that view_dependencies is accepted without error) --- .../server/persist/DependencyRepository.java | 56 --------- .../server/persist/Repositories.java | 2 - .../server/persist/TableRepository.java | 39 ------ .../server/persist/dao/DependencyDAO.java | 118 ------------------ .../persist/utils/HibernateConfigurator.java | 2 - .../base/table/BaseMetricViewCRUDTest.java | 12 +- 6 files changed, 4 insertions(+), 225 deletions(-) delete mode 100644 server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java delete mode 100644 server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java diff --git a/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java b/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java deleted file mode 100644 index 0cb33b8b93..0000000000 --- a/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java +++ /dev/null @@ -1,56 +0,0 @@ -package io.unitycatalog.server.persist; - -import io.unitycatalog.server.persist.dao.DependencyDAO; -import java.util.List; -import java.util.UUID; -import org.hibernate.Session; -import org.hibernate.query.Query; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Repository for managing view/metric-view dependency records in uc_dependencies. Methods accept a - * Session so they can participate in the caller's transaction. - */ -public class DependencyRepository { - private static final Logger LOGGER = LoggerFactory.getLogger(DependencyRepository.class); - - public void createDependencies( - Session session, UUID dependentId, String dependentType, List dependencies) { - for (DependencyDAO dep : dependencies) { - dep.setDependentId(dependentId); - dep.setDependentType(dependentType); - session.persist(dep); - } - LOGGER.debug( - "Created {} dependencies for {}:{}", dependencies.size(), dependentType, dependentId); - } - - public List getDependencies( - Session session, UUID dependentId, String dependentType) { - String hql = - "FROM DependencyDAO d WHERE d.dependentId = :dependentId" - + " AND d.dependentType = :dependentType"; - Query query = session.createQuery(hql, DependencyDAO.class); - query.setParameter("dependentId", dependentId); - query.setParameter("dependentType", dependentType); - return query.list(); - } - - public void deleteDependencies(Session session, UUID dependentId, String dependentType) { - String hql = - "DELETE FROM DependencyDAO d WHERE d.dependentId = :dependentId" - + " AND d.dependentType = :dependentType"; - Query query = session.createQuery(hql); - query.setParameter("dependentId", dependentId); - query.setParameter("dependentType", dependentType); - int deleted = query.executeUpdate(); - LOGGER.debug("Deleted {} dependencies for {}:{}", deleted, dependentType, dependentId); - } - - public void updateDependencies( - Session session, UUID dependentId, String dependentType, List dependencies) { - deleteDependencies(session, dependentId, dependentType); - createDependencies(session, dependentId, dependentType, dependencies); - } -} diff --git a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java index ac4e6f4964..7222450e5b 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java +++ b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java @@ -29,7 +29,6 @@ public class Repositories { private final CredentialRepository credentialRepository; private final ExternalLocationRepository externalLocationRepository; private final DeltaCommitRepository deltaCommitRepository; - private final DependencyRepository dependencyRepository; private final KeyMapper keyMapper; @@ -51,7 +50,6 @@ public Repositories(SessionFactory sessionFactory, ServerProperties serverProper this.credentialRepository = new CredentialRepository(this, sessionFactory, serverProperties); this.externalLocationRepository = new ExternalLocationRepository(this, sessionFactory); this.deltaCommitRepository = new DeltaCommitRepository(sessionFactory, serverProperties); - this.dependencyRepository = new DependencyRepository(); // KeyMapper uses all the repositories above. this.keyMapper = new KeyMapper(this); diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index 6bba8567e7..bbe585b787 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -5,11 +5,9 @@ import io.unitycatalog.server.model.ColumnInfo; import io.unitycatalog.server.model.CreateTable; import io.unitycatalog.server.model.DataSourceFormat; -import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.model.TableType; -import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.PropertyDAO; import io.unitycatalog.server.persist.dao.SchemaInfoDAO; import io.unitycatalog.server.persist.dao.StagingTableDAO; @@ -153,16 +151,6 @@ public TableInfo getTable(String fullName) { TableInfo tableInfo = tableInfoDAO.toTableInfo(true, catalogName, schemaName); RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); - if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { - List deps = - repositories - .getDependencyRepository() - .getDependencies(session, tableInfoDAO.getId(), "TABLE"); - if (!deps.isEmpty()) { - tableInfo.setViewDependencies( - new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); - } - } return tableInfo; }, "Failed to get table", @@ -241,18 +229,6 @@ public TableInfo createTable(CreateTable createTable) { } storageLocation = null; tableID = UUID.randomUUID().toString(); - // Persist view dependencies - DependencyList viewDeps = createTable.getViewDependencies(); - if (viewDeps != null && viewDeps.getDependencies() != null) { - UUID tableUUID = UUID.fromString(tableID); - List depDAOs = - viewDeps.getDependencies().stream() - .map(dep -> DependencyDAO.from(dep, tableUUID, "TABLE")) - .collect(Collectors.toList()); - repositories - .getDependencyRepository() - .createDependencies(session, tableUUID, "TABLE", depDAOs); - } } else if (tableType == TableType.STREAMING_TABLE) { throw new BaseException( ErrorCode.INVALID_ARGUMENT, "STREAMING TABLE creation is not supported yet."); @@ -384,16 +360,6 @@ public ListTablesResponse listTables( RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); } - if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { - List deps = - repositories - .getDependencyRepository() - .getDependencies(session, tableInfoDAO.getId(), "TABLE"); - if (!deps.isEmpty()) { - tableInfo.setViewDependencies( - new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); - } - } result.add(tableInfo); } return new ListTablesResponse().tables(result).nextPageToken(nextPageToken); @@ -436,11 +402,6 @@ public void deleteTable(Session session, UUID schemaId, String tableName) { .getDeltaCommitRepository() .permanentlyDeleteTableCommits(session, tableInfoDAO.getId()); } - if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { - repositories - .getDependencyRepository() - .deleteDependencies(session, tableInfoDAO.getId(), "TABLE"); - } PropertyRepository.findProperties(session, tableInfoDAO.getId(), Constants.TABLE) .forEach(session::remove); session.remove(tableInfoDAO); diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java deleted file mode 100644 index b1872bad05..0000000000 --- a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java +++ /dev/null @@ -1,118 +0,0 @@ -package io.unitycatalog.server.persist.dao; - -import io.unitycatalog.server.model.Dependency; -import io.unitycatalog.server.model.TableDependency; -import jakarta.persistence.Column; -import jakarta.persistence.Entity; -import jakarta.persistence.Id; -import jakarta.persistence.Index; -import jakarta.persistence.Table; -import java.util.ArrayList; -import java.util.List; -import java.util.UUID; -import java.util.stream.Collectors; -import lombok.AllArgsConstructor; -import lombok.Builder; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; -import lombok.ToString; -import org.hibernate.annotations.UuidGenerator; - -@Entity -@Table( - name = "uc_dependencies", - indexes = { - @Index(name = "idx_dependent", columnList = "dependent_type,dependent_id"), - }) -@Getter -@Setter -@NoArgsConstructor -@AllArgsConstructor -@EqualsAndHashCode -@ToString -@Builder -public class DependencyDAO { - @Id - @UuidGenerator - @Column(name = "id", updatable = false, nullable = false) - private UUID id; - - @Column(name = "dependent_type", nullable = false) - private String dependentType; - - @Column(name = "dependent_id", nullable = false) - private UUID dependentId; - - @Column(name = "dependency_type", nullable = false) - private String dependencyType; - - @Column(name = "dependency_catalog") - private String dependencyCatalog; - - @Column(name = "dependency_schema") - private String dependencySchema; - - @Column(name = "dependency_name") - private String dependencyName; - - @Column(name = "dependency_target_id") - private UUID dependencyTargetId; - - /** Converts a Dependency API model to a DependencyDAO for a given dependent. */ - public static DependencyDAO from(Dependency dependency, UUID dependentId, String dependentType) { - DependencyDAOBuilder builder = - DependencyDAO.builder().dependentId(dependentId).dependentType(dependentType); - - if (dependency.getTable() != null) { - builder.dependencyType("TABLE"); - String fullName = dependency.getTable().getTableFullName(); - String[] parts = fullName.split("\\."); - if (parts.length == 3) { - builder.dependencyCatalog(parts[0]); - builder.dependencySchema(parts[1]); - builder.dependencyName(parts[2]); - } else { - builder.dependencyName(fullName); - } - } else if (dependency.getFunction() != null) { - builder.dependencyType("FUNCTION"); - String fullName = dependency.getFunction().getFunctionFullName(); - String[] parts = fullName.split("\\."); - if (parts.length == 3) { - builder.dependencyCatalog(parts[0]); - builder.dependencySchema(parts[1]); - builder.dependencyName(parts[2]); - } else { - builder.dependencyName(fullName); - } - } - - return builder.build(); - } - - /** Converts this DAO to a Dependency API model. */ - public Dependency toDependency() { - Dependency dependency = new Dependency(); - if ("TABLE".equals(dependencyType)) { - String fullName = dependencyCatalog + "." + dependencySchema + "." + dependencyName; - dependency.setTable(new TableDependency().tableFullName(fullName)); - } - return dependency; - } - - public String getDependencyFullName() { - if (dependencyCatalog != null && dependencySchema != null && dependencyName != null) { - return dependencyCatalog + "." + dependencySchema + "." + dependencyName; - } - return dependencyName; - } - - public static List toDependencyList(List daos) { - if (daos == null) { - return new ArrayList<>(); - } - return daos.stream().map(DependencyDAO::toDependency).collect(Collectors.toList()); - } -} diff --git a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java index 486828bd5a..e3ee6672a3 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java +++ b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java @@ -4,7 +4,6 @@ import io.unitycatalog.server.persist.dao.ColumnInfoDAO; import io.unitycatalog.server.persist.dao.CredentialDAO; import io.unitycatalog.server.persist.dao.DeltaCommitDAO; -import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.ExternalLocationDAO; import io.unitycatalog.server.persist.dao.FunctionInfoDAO; import io.unitycatalog.server.persist.dao.FunctionParameterInfoDAO; @@ -72,7 +71,6 @@ private static SessionFactory createSessionFactory(Properties hibernatePropertie configuration.addAnnotatedClass(CredentialDAO.class); configuration.addAnnotatedClass(ExternalLocationDAO.class); configuration.addAnnotatedClass(DeltaCommitDAO.class); - configuration.addAnnotatedClass(DependencyDAO.class); ServiceRegistry serviceRegistry = new StandardServiceRegistryBuilder().applySettings(configuration.getProperties()).build(); diff --git a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java index 4c4b4f5b32..d5cf1462ba 100644 --- a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java +++ b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java @@ -92,7 +92,7 @@ public void testMetricViewCRUD() throws Exception { } @Test - public void testMetricViewWithDependencies() throws Exception { + public void testMetricViewWithDependenciesAccepted() throws Exception { String sourceTableFullName = TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".source_events"; @@ -109,20 +109,16 @@ public void testMetricViewWithDependencies() throws Exception { .tableType(TableType.METRIC_VIEW) .viewDefinition(VIEW_DEFINITION) .viewDependencies(depList) - .comment("Metric view with dependencies"); + .comment("Metric view with dependencies in payload"); TableInfo created = tableOperations.createTable(createRequest); assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); - // Verify dependencies are returned on GET TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); - assertThat(fetched.getViewDependencies()).isNotNull(); - assertThat(fetched.getViewDependencies().getDependencies()).hasSize(1); - assertThat(fetched.getViewDependencies().getDependencies().get(0).getTable().getTableFullName()) - .isEqualTo(sourceTableFullName); + assertThat(fetched.getTableType()).isEqualTo(TableType.METRIC_VIEW); + assertThat(fetched.getViewDefinition()).isEqualTo(VIEW_DEFINITION); - // Delete and verify dependencies are cleaned up tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) .isInstanceOf(Exception.class); From dcaf6fda1073e6d50c4b8591050aa1c488fd8703 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 14 Apr 2026 20:06:37 +0000 Subject: [PATCH 13/26] Generalize createTable(TableInfo) to handle all table types Remove metric-view-specific branching in createTable(ident, tableInfo). The method now forwards all TableInfo fields (tableType, viewDefinition, viewDependencies, columns, properties) to the UC server generically, working for any table type without type-specific conditional logic. --- .../unitycatalog/spark/UCSingleCatalog.scala | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index aa79846b27..e8415bed2f 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -103,12 +103,7 @@ class UCSingleCatalog ident: Identifier, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - val tableType = tableInfo.tableType() - if (TableSummary.METRIC_VIEW_TABLE_TYPE.equals(tableType)) { - return delegate.asInstanceOf[UCProxy].createMetricViewFromTableInfo( - ident, this.name, tableInfo) - } - super.createTable(ident, tableInfo) + delegate.asInstanceOf[UCProxy].createTableFromTableInfo(ident, this.name, tableInfo) } override def createTable( @@ -715,10 +710,11 @@ private class UCProxy( } /** - * Creates a metric view using structured fields from Spark's TableInfo. - * Called by UCSingleCatalog when tableInfo.tableType() is METRIC_VIEW. + * Creates a table from Spark's structured TableInfo, forwarding all fields + * (tableType, viewDefinition, viewDependencies, columns, properties) to the + * UC server. Works for any table type including METRIC_VIEW. */ - def createMetricViewFromTableInfo( + def createTableFromTableInfo( ident: Identifier, catalogName: String, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { @@ -726,8 +722,11 @@ private class UCProxy( ct.setName(ident.name()) ct.setSchemaName(ident.namespace().head) ct.setCatalogName(catalogName) - ct.setTableType(TableType.METRIC_VIEW) - ct.setViewDefinition(tableInfo.viewDefinition()) + + Option(tableInfo.tableType()).foreach { tt => + ct.setTableType(TableType.fromValue(tt)) + } + Option(tableInfo.viewDefinition()).foreach(ct.setViewDefinition(_)) Option(tableInfo.properties().get("comment")).foreach(ct.setComment(_)) val columns: Seq[ColumnInfo] = tableInfo.columns().toSeq.zipWithIndex.map { case (col, i) => From 7a5f5d91de8bf1cd2b32d6932b23b6f6741c2016 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 14 Apr 2026 21:22:36 +0000 Subject: [PATCH 14/26] Move createTable(TableInfo) to UCProxy, use standard delegation Now that DelegatingCatalogExtension forwards createTable(Identifier, TableInfo) properly (Spark PR), the override can live on UCProxy like all other TableCatalog methods. UCSingleCatalog simply delegates to the delegate chain without casting or bypassing DeltaCatalog. --- .../io/unitycatalog/spark/UCSingleCatalog.scala | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index e8415bed2f..9309d91edf 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -103,7 +103,7 @@ class UCSingleCatalog ident: Identifier, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - delegate.asInstanceOf[UCProxy].createTableFromTableInfo(ident, this.name, tableInfo) + delegate.createTable(ident, tableInfo) } override def createTable( @@ -709,19 +709,14 @@ private class UCProxy( .asInstanceOf[Table] } - /** - * Creates a table from Spark's structured TableInfo, forwarding all fields - * (tableType, viewDefinition, viewDependencies, columns, properties) to the - * UC server. Works for any table type including METRIC_VIEW. - */ - def createTableFromTableInfo( + override def createTable( ident: Identifier, - catalogName: String, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { + UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) val ct = new CreateTable() ct.setName(ident.name()) ct.setSchemaName(ident.namespace().head) - ct.setCatalogName(catalogName) + ct.setCatalogName(this.name) Option(tableInfo.tableType()).foreach { tt => ct.setTableType(TableType.fromValue(tt)) From 1c23fdea8e535a2b2952095e5577e1f65685cfe1 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Tue, 14 Apr 2026 21:26:29 +0000 Subject: [PATCH 15/26] Extract shared helpers for createTable overloads Factor out common logic between createTable(TableInfo) and createTable(StructType, ...) into two shared helpers: - initCreateTable: sets name, schema, catalog, comment, properties - convertColumns: converts Spark Column[] to UC ColumnInfo[] Both createTable overloads now use the same base initialization, ensuring consistent behavior for common fields. The TableInfo overload adds tableType, viewDefinition, and viewDependencies on top; the legacy overload adds storage location, data source format, and partitions. --- .../unitycatalog/spark/UCSingleCatalog.scala | 75 +++++++++---------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 9309d91edf..1a7b8c3ba7 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -713,27 +713,14 @@ private class UCProxy( ident: Identifier, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - val ct = new CreateTable() - ct.setName(ident.name()) - ct.setSchemaName(ident.namespace().head) - ct.setCatalogName(this.name) + val ct = initCreateTable(ident, tableInfo.properties()) Option(tableInfo.tableType()).foreach { tt => ct.setTableType(TableType.fromValue(tt)) } Option(tableInfo.viewDefinition()).foreach(ct.setViewDefinition(_)) - Option(tableInfo.properties().get("comment")).foreach(ct.setComment(_)) - val columns: Seq[ColumnInfo] = tableInfo.columns().toSeq.zipWithIndex.map { case (col, i) => - val column = new ColumnInfo() - column.setName(col.name()) - column.setNullable(col.nullable()) - column.setTypeText(col.dataType().catalogString) - column.setTypeName(convertDataTypeToTypeName(col.dataType())) - column.setTypeJson(col.dataType().json) - column.setPosition(i) - column - } + val columns = convertColumns(tableInfo.columns()) ct.setColumns(columns.asJava) Option(tableInfo.viewDependencies()).foreach { sparkDepList => @@ -757,25 +744,15 @@ private class UCProxy( ct.setViewDependencies(ucDepList) } - val reservedKeys = Set("comment", "provider") - val serverProps = tableInfo.properties().asScala - .filterKeys(!reservedKeys.contains(_)) - .toMap.asJava - ct.setProperties(serverProps) - tablesApi.createTable(ct) loadTable(ident) } override def createTable(ident: Identifier, schema: StructType, partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - UCSingleCatalog.requireProviderSpecified("CREATE TABLE", properties) - val createTable = new CreateTable() - createTable.setName(ident.name()) - createTable.setSchemaName(ident.namespace().head) - createTable.setCatalogName(this.name) + val ct = initCreateTable(ident, properties) val hasExternalClause = properties.containsKey(TableCatalog.PROP_EXTERNAL) val storageLocation = properties.get(TableCatalog.PROP_LOCATION) @@ -788,11 +765,11 @@ private class UCProxy( if (!format.equalsIgnoreCase(DataSourceFormat.DELTA.name)) { throw new ApiException("Unity Catalog does not support non-Delta managed table.") } - createTable.setTableType(TableType.MANAGED) + ct.setTableType(TableType.MANAGED) } else { - createTable.setTableType(TableType.EXTERNAL) + ct.setTableType(TableType.EXTERNAL) } - createTable.setStorageLocation(storageLocation) + ct.setStorageLocation(storageLocation) val partitionColNames: Seq[String] = partitions.flatMap { t => t.name() match { @@ -822,18 +799,40 @@ private class UCProxy( if (partitionIdx >= 0) column.setPartitionIndex(partitionIdx) column } - val comment = Option(properties.get(TableCatalog.PROP_COMMENT)) - comment.foreach(createTable.setComment(_)) - createTable.setColumns(columns) - createTable.setDataSourceFormat(convertDatasourceFormat(format)) - // Do not send the V2 table properties as they are made part of the `createTable` already. - val propertiesToServer = - properties.view.filterKeys(!UCTableProperties.V2_TABLE_PROPERTIES.contains(_)).toMap - createTable.setProperties(propertiesToServer) - tablesApi.createTable(createTable) + ct.setColumns(columns) + ct.setDataSourceFormat(convertDatasourceFormat(format)) + tablesApi.createTable(ct) loadTable(ident) } + private def initCreateTable( + ident: Identifier, + properties: util.Map[String, String]): CreateTable = { + val ct = new CreateTable() + ct.setName(ident.name()) + ct.setSchemaName(ident.namespace().head) + ct.setCatalogName(this.name) + Option(properties.get(TableCatalog.PROP_COMMENT)).foreach(ct.setComment(_)) + val serverProps = + properties.view.filterKeys(!UCTableProperties.V2_TABLE_PROPERTIES.contains(_)).toMap + ct.setProperties(serverProps) + ct + } + + private def convertColumns( + columns: Array[org.apache.spark.sql.connector.catalog.Column]): Seq[ColumnInfo] = { + columns.toSeq.zipWithIndex.map { case (col, i) => + val column = new ColumnInfo() + column.setName(col.name()) + column.setNullable(col.nullable()) + column.setTypeText(col.dataType().catalogString) + column.setTypeName(convertDataTypeToTypeName(col.dataType())) + column.setTypeJson(col.dataType().json) + column.setPosition(i) + column + } + } + private def convertDatasourceFormat(format: String): DataSourceFormat = { format.toUpperCase match { case "PARQUET" => DataSourceFormat.PARQUET From 1964e510f5ca58d0727d95c196998bdfcce32211 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 15:34:49 +0000 Subject: [PATCH 16/26] Unify createTable: old overload delegates to TableInfo overload The old createTable(StructType, Transform[], Map) now converts its arguments into a TableInfo and calls createTable(Identifier, TableInfo). All table creation logic lives in one method that handles all fields: tableType, viewDefinition, viewDependencies, columns, partitions, storage location, data source format, and properties. --- .../unitycatalog/spark/UCSingleCatalog.scala | 128 ++++++++---------- 1 file changed, 57 insertions(+), 71 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 1a7b8c3ba7..d7803e53ec 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -713,16 +713,56 @@ private class UCProxy( ident: Identifier, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - val ct = initCreateTable(ident, tableInfo.properties()) + val properties = tableInfo.properties() + val ct = new CreateTable() + ct.setName(ident.name()) + ct.setSchemaName(ident.namespace().head) + ct.setCatalogName(this.name) + Option(properties.get(TableCatalog.PROP_COMMENT)).foreach(ct.setComment(_)) + val serverProps = + properties.view.filterKeys(!UCTableProperties.V2_TABLE_PROPERTIES.contains(_)).toMap + ct.setProperties(serverProps) - Option(tableInfo.tableType()).foreach { tt => - ct.setTableType(TableType.fromValue(tt)) + // Table type: use explicit tableType if provided, otherwise infer from properties + Option(tableInfo.tableType()) match { + case Some(tt) => ct.setTableType(TableType.fromValue(tt)) + case None => + val isManagedLocation = Option(properties.get(TableCatalog.PROP_IS_MANAGED_LOCATION)) + .exists(_.equalsIgnoreCase("true")) + if (isManagedLocation) { + ct.setTableType(TableType.MANAGED) + } else { + ct.setTableType(TableType.EXTERNAL) + } } - Option(tableInfo.viewDefinition()).foreach(ct.setViewDefinition(_)) - val columns = convertColumns(tableInfo.columns()) + // Storage location (null for metric views and other view types) + Option(properties.get(TableCatalog.PROP_LOCATION)).foreach(ct.setStorageLocation(_)) + + // Data source format + Option(properties.get("provider")).foreach { format => + ct.setDataSourceFormat(convertDatasourceFormat(format)) + } + + // Columns + val partitionColNames = extractPartitionColumnNames(tableInfo.partitions()) + val columns: Seq[ColumnInfo] = tableInfo.columns().toSeq.zipWithIndex.map { case (col, i) => + val column = new ColumnInfo() + column.setName(col.name()) + column.setNullable(col.nullable()) + column.setTypeText(col.dataType().catalogString) + column.setTypeName(convertDataTypeToTypeName(col.dataType())) + column.setTypeJson(col.dataType().json) + column.setPosition(i) + Option(col.comment()).foreach(column.setComment(_)) + val partitionIdx = partitionColNames.indexWhere(_.equalsIgnoreCase(col.name())) + if (partitionIdx >= 0) column.setPartitionIndex(partitionIdx) + column + } ct.setColumns(columns.asJava) + // View definition and dependencies (null for non-view tables) + Option(tableInfo.viewDefinition()).foreach(ct.setViewDefinition(_)) Option(tableInfo.viewDependencies()).foreach { sparkDepList => val ucDepList = new io.unitycatalog.client.model.DependencyList() val ucDeps = sparkDepList.dependencies().map { dep => @@ -751,27 +791,20 @@ private class UCProxy( override def createTable(ident: Identifier, schema: StructType, partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) UCSingleCatalog.requireProviderSpecified("CREATE TABLE", properties) - - val ct = initCreateTable(ident, properties) - - val hasExternalClause = properties.containsKey(TableCatalog.PROP_EXTERNAL) - val storageLocation = properties.get(TableCatalog.PROP_LOCATION) - assert(storageLocation != null, "location should either be user specified or system generated.") - val isManagedLocation = Option(properties.get(TableCatalog.PROP_IS_MANAGED_LOCATION)) - .exists(_.equalsIgnoreCase("true")) - val format = properties.get("provider") - if (isManagedLocation) { - assert(!hasExternalClause, "location is only generated for managed tables.") - if (!format.equalsIgnoreCase(DataSourceFormat.DELTA.name)) { - throw new ApiException("Unity Catalog does not support non-Delta managed table.") - } - ct.setTableType(TableType.MANAGED) - } else { - ct.setTableType(TableType.EXTERNAL) + val columns: Array[Column] = schema.fields.map { f => + Column.create(f.name, f.dataType, f.nullable, f.getComment().orNull, null) } - ct.setStorageLocation(storageLocation) + val tableInfo = new org.apache.spark.sql.connector.catalog.TableInfo.Builder() + .withColumns(columns) + .withPartitions(partitions) + .withProperties(properties) + .build() + createTable(ident, tableInfo) + } - val partitionColNames: Seq[String] = partitions.flatMap { t => + private def extractPartitionColumnNames(partitions: Array[Transform]): Seq[String] = { + if (partitions == null) return Seq.empty + partitions.flatMap { t => t.name() match { case "identity" => val fieldNames = t.references().flatMap(_.fieldNames()) @@ -784,53 +817,6 @@ private class UCProxy( throw new ApiException(s"Unsupported partition transform: $other") } }.toSeq - val columns: Seq[ColumnInfo] = schema.fields.toSeq.zipWithIndex.map { case (field, i) => - val column = new ColumnInfo() - column.setName(field.name) - if (field.getComment().isDefined) { - column.setComment(field.getComment.get) - } - column.setNullable(field.nullable) - column.setTypeText(field.dataType.catalogString) - column.setTypeName(convertDataTypeToTypeName(field.dataType)) - column.setTypeJson(field.dataType.json) - column.setPosition(i) - val partitionIdx = partitionColNames.indexWhere(_.equalsIgnoreCase(field.name)) - if (partitionIdx >= 0) column.setPartitionIndex(partitionIdx) - column - } - ct.setColumns(columns) - ct.setDataSourceFormat(convertDatasourceFormat(format)) - tablesApi.createTable(ct) - loadTable(ident) - } - - private def initCreateTable( - ident: Identifier, - properties: util.Map[String, String]): CreateTable = { - val ct = new CreateTable() - ct.setName(ident.name()) - ct.setSchemaName(ident.namespace().head) - ct.setCatalogName(this.name) - Option(properties.get(TableCatalog.PROP_COMMENT)).foreach(ct.setComment(_)) - val serverProps = - properties.view.filterKeys(!UCTableProperties.V2_TABLE_PROPERTIES.contains(_)).toMap - ct.setProperties(serverProps) - ct - } - - private def convertColumns( - columns: Array[org.apache.spark.sql.connector.catalog.Column]): Seq[ColumnInfo] = { - columns.toSeq.zipWithIndex.map { case (col, i) => - val column = new ColumnInfo() - column.setName(col.name()) - column.setNullable(col.nullable()) - column.setTypeText(col.dataType().catalogString) - column.setTypeName(convertDataTypeToTypeName(col.dataType())) - column.setTypeJson(col.dataType().json) - column.setPosition(i) - column - } } private def convertDatasourceFormat(format: String): DataSourceFormat = { From 1db7e14f6ad21f026f525c08b75db33d7c8bcffd Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 17:58:32 +0000 Subject: [PATCH 17/26] Unify loadTable: remove separate loadMetricView method Merge loadMetricView into loadTable by branching on whether the table has storage (storageLocation != null) rather than checking table type. Tables with storage get credential vending, partition extraction, and storage format. Tables without storage (metric views, future view types) get empty storage, viewText from viewDefinition, and skip credential vending. No more type-specific method. --- .../unitycatalog/spark/UCSingleCatalog.scala | 178 +++++++++--------- 1 file changed, 85 insertions(+), 93 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index d7803e53ec..00c59a8c63 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -591,82 +591,103 @@ private class UCProxy( throw new NoSuchTableException(ident) } - if (t.getTableType == TableType.METRIC_VIEW) { - return loadMetricView(t, ident) - } - val identifier = TableIdentifier(t.getName, Some(t.getSchemaName), Some(t.getCatalogName)) + val hasStorage = t.getStorageLocation != null + + // Columns val partitionCols = scala.collection.mutable.ArrayBuffer.empty[(String, Int)] - val fields = t.getColumns.asScala.map { col => - Option(col.getPartitionIndex).foreach { index => - partitionCols += col.getName -> index - } - StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) - .withComment(col.getComment) - }.toArray - val locationUri = CatalogUtils.stringToURI(t.getStorageLocation) - val tableId = t.getTableId - var tableOp = TableOperation.READ_WRITE - val temporaryCredentials = { - try { - temporaryCredentialsApi - .generateTemporaryTableCredentials( - // TODO: at this time, we don't know if the table will be read or written. For now we always - // request READ_WRITE credentials as the server doesn't distinguish between READ and - // READ_WRITE credentials as of today. When loading a table, Spark should tell if it's - // for read or write, we can request the proper credential after fixing Spark. - new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - ) - } catch { - case e: ApiException => - logWarning(s"READ_WRITE credential generation failed for table $identifier: ${e.getMessage}") - try { - tableOp = TableOperation.READ - temporaryCredentialsApi - .generateTemporaryTableCredentials( - new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) - ) - } catch { - case e: ApiException => - logWarning(s"READ credential generation failed for table $identifier: ${e.getMessage}") - if (serverSidePlanningEnabled) null else throw e + val fields = if (t.getColumns != null) { + t.getColumns.asScala.map { col => + if (hasStorage) { + Option(col.getPartitionIndex).foreach { index => + partitionCols += col.getName -> index } - } + } + StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) + .withComment(col.getComment) + }.toArray + } else { + Array.empty[StructField] } - if (serverSidePlanningEnabled && temporaryCredentials == null) { - enableServerSidePlanningConfig(identifier) - } + // Credential vending -- only for tables with storage + val (storage, extraProps) = if (hasStorage) { + val locationUri = CatalogUtils.stringToURI(t.getStorageLocation) + val tableId = t.getTableId + var tableOp = TableOperation.READ_WRITE + val temporaryCredentials = { + try { + temporaryCredentialsApi + .generateTemporaryTableCredentials( + // TODO: at this time, we don't know if the table will be read or written. For now we always + // request READ_WRITE credentials as the server doesn't distinguish between READ and + // READ_WRITE credentials as of today. When loading a table, Spark should tell if it's + // for read or write, we can request the proper credential after fixing Spark. + new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) + ) + } catch { + case e: ApiException => + logWarning(s"READ_WRITE credential generation failed for table $identifier: ${e.getMessage}") + try { + tableOp = TableOperation.READ + temporaryCredentialsApi + .generateTemporaryTableCredentials( + new GenerateTemporaryTableCredential().tableId(tableId).operation(tableOp) + ) + } catch { + case e: ApiException => + logWarning(s"READ credential generation failed for table $identifier: ${e.getMessage}") + if (serverSidePlanningEnabled) null else throw e + } + } + } - val extraSerdeProps = if (temporaryCredentials == null) { - Map.empty[String, String].asJava - } else { - CredPropsUtil.createTableCredProps( - renewCredEnabled, - credScopedFsEnabled, - UCSingleCatalog.sessionHadoopFsImplProps(), - locationUri.getScheme, - uri.toString, - tokenProvider, - tableId, - tableOp, - temporaryCredentials, + if (serverSidePlanningEnabled && temporaryCredentials == null) { + enableServerSidePlanningConfig(identifier) + } + + val credProps = if (temporaryCredentials == null) { + Map.empty[String, String].asJava + } else { + CredPropsUtil.createTableCredProps( + renewCredEnabled, + credScopedFsEnabled, + UCSingleCatalog.sessionHadoopFsImplProps(), + locationUri.getScheme, + uri.toString, + tokenProvider, + tableId, + tableOp, + temporaryCredentials, + ) + } + + val storageFormat = CatalogStorageFormat.empty.copy( + locationUri = Some(locationUri), + properties = t.getProperties.asScala.toMap ++ credProps ) + (storageFormat, Map.empty[String, String]) + } else { + (CatalogStorageFormat.empty, + Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty)) + } + + // Table type mapping + val catalogTableType = t.getTableType match { + case TableType.MANAGED => CatalogTableType.MANAGED + case TableType.METRIC_VIEW => CatalogTableType.METRIC_VIEW + case _ => CatalogTableType.EXTERNAL } val sparkTable = CatalogTable( identifier, - tableType = if (t.getTableType == TableType.MANAGED) { - CatalogTableType.MANAGED - } else { - CatalogTableType.EXTERNAL - }, - storage = CatalogStorageFormat.empty.copy( - locationUri = Some(locationUri), - properties = t.getProperties.asScala.toMap ++ extraSerdeProps - ), + tableType = catalogTableType, + storage = storage, schema = StructType(fields), - provider = Some(t.getDataSourceFormat.getValue.toLowerCase()), + provider = Option(t.getDataSourceFormat).map(_.getValue.toLowerCase()), + viewText = Option(t.getViewDefinition), + viewOriginalText = Option(t.getViewDefinition), + properties = extraProps, createTime = t.getCreatedAt, tracksPartitionsInCatalog = false, partitionColumnNames = partitionCols.sortBy(_._2).map(_._1).toSeq @@ -680,35 +701,6 @@ private class UCProxy( .asInstanceOf[Table] } - private def loadMetricView(t: TableInfo, ident: Identifier): Table = { - val identifier = TableIdentifier(t.getName, Some(t.getSchemaName), Some(t.getCatalogName)) - val fields = if (t.getColumns != null) { - t.getColumns.asScala.map { col => - StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) - .withComment(col.getComment) - }.toArray - } else { - Array.empty[StructField] - } - - val props = Option(t.getProperties).map(_.asScala.toMap).getOrElse(Map.empty) - - val sparkTable = CatalogTable( - identifier, - tableType = CatalogTableType.METRIC_VIEW, - storage = CatalogStorageFormat.empty, - schema = StructType(fields), - viewText = Some(t.getViewDefinition), - viewOriginalText = Some(t.getViewDefinition), - properties = props, - tracksPartitionsInCatalog = false - ) - Class.forName("org.apache.spark.sql.connector.catalog.V1Table") - .getDeclaredConstructor(classOf[CatalogTable]) - .newInstance(sparkTable) - .asInstanceOf[Table] - } - override def createTable( ident: Identifier, tableInfo: org.apache.spark.sql.connector.catalog.TableInfo): Table = { From cb85b70a25da4161496e13545f8d112d6bcfc924 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 19:44:53 +0000 Subject: [PATCH 18/26] Add back view dependency persistence, clean up blank lines Re-introduce uc_dependencies persistence so that view_dependencies round-trips through the API: what clients send on create is persisted and returned on read. This makes the API self-consistent and enables non-Spark clients to discover metric view dependencies. Restored: - DependencyDAO and DependencyRepository - DependencyRepository in Repositories and HibernateConfigurator - Dependency create/read/delete logic in TableRepository (via shared attachDependencies helper) - Dependency assertions in BaseMetricViewCRUDTest Also removed unnecessary blank line in UCSingleCatalog.scala. --- .../unitycatalog/spark/UCSingleCatalog.scala | 1 - .../server/persist/DependencyRepository.java | 56 +++++++++ .../server/persist/Repositories.java | 2 + .../server/persist/TableRepository.java | 33 +++++ .../server/persist/dao/DependencyDAO.java | 118 ++++++++++++++++++ .../persist/utils/HibernateConfigurator.java | 2 + .../base/table/BaseMetricViewCRUDTest.java | 10 +- 7 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java create mode 100644 server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 00c59a8c63..6def509e5f 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -112,7 +112,6 @@ class UCSingleCatalog partitions: Array[Transform], properties: util.Map[String, String]): Table = { UCSingleCatalog.checkUnsupportedNestedNamespace(ident.namespace()) - val hasExternalClause = properties.containsKey(TableCatalog.PROP_EXTERNAL) val hasLocationClause = properties.containsKey(TableCatalog.PROP_LOCATION) if (hasExternalClause && !hasLocationClause) { diff --git a/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java b/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java new file mode 100644 index 0000000000..0cb33b8b93 --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/persist/DependencyRepository.java @@ -0,0 +1,56 @@ +package io.unitycatalog.server.persist; + +import io.unitycatalog.server.persist.dao.DependencyDAO; +import java.util.List; +import java.util.UUID; +import org.hibernate.Session; +import org.hibernate.query.Query; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Repository for managing view/metric-view dependency records in uc_dependencies. Methods accept a + * Session so they can participate in the caller's transaction. + */ +public class DependencyRepository { + private static final Logger LOGGER = LoggerFactory.getLogger(DependencyRepository.class); + + public void createDependencies( + Session session, UUID dependentId, String dependentType, List dependencies) { + for (DependencyDAO dep : dependencies) { + dep.setDependentId(dependentId); + dep.setDependentType(dependentType); + session.persist(dep); + } + LOGGER.debug( + "Created {} dependencies for {}:{}", dependencies.size(), dependentType, dependentId); + } + + public List getDependencies( + Session session, UUID dependentId, String dependentType) { + String hql = + "FROM DependencyDAO d WHERE d.dependentId = :dependentId" + + " AND d.dependentType = :dependentType"; + Query query = session.createQuery(hql, DependencyDAO.class); + query.setParameter("dependentId", dependentId); + query.setParameter("dependentType", dependentType); + return query.list(); + } + + public void deleteDependencies(Session session, UUID dependentId, String dependentType) { + String hql = + "DELETE FROM DependencyDAO d WHERE d.dependentId = :dependentId" + + " AND d.dependentType = :dependentType"; + Query query = session.createQuery(hql); + query.setParameter("dependentId", dependentId); + query.setParameter("dependentType", dependentType); + int deleted = query.executeUpdate(); + LOGGER.debug("Deleted {} dependencies for {}:{}", deleted, dependentType, dependentId); + } + + public void updateDependencies( + Session session, UUID dependentId, String dependentType, List dependencies) { + deleteDependencies(session, dependentId, dependentType); + createDependencies(session, dependentId, dependentType, dependencies); + } +} diff --git a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java index 7222450e5b..ac4e6f4964 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/Repositories.java +++ b/server/src/main/java/io/unitycatalog/server/persist/Repositories.java @@ -29,6 +29,7 @@ public class Repositories { private final CredentialRepository credentialRepository; private final ExternalLocationRepository externalLocationRepository; private final DeltaCommitRepository deltaCommitRepository; + private final DependencyRepository dependencyRepository; private final KeyMapper keyMapper; @@ -50,6 +51,7 @@ public Repositories(SessionFactory sessionFactory, ServerProperties serverProper this.credentialRepository = new CredentialRepository(this, sessionFactory, serverProperties); this.externalLocationRepository = new ExternalLocationRepository(this, sessionFactory); this.deltaCommitRepository = new DeltaCommitRepository(sessionFactory, serverProperties); + this.dependencyRepository = new DependencyRepository(); // KeyMapper uses all the repositories above. this.keyMapper = new KeyMapper(this); diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index bbe585b787..d490dae6ec 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -5,9 +5,11 @@ import io.unitycatalog.server.model.ColumnInfo; import io.unitycatalog.server.model.CreateTable; import io.unitycatalog.server.model.DataSourceFormat; +import io.unitycatalog.server.model.DependencyList; import io.unitycatalog.server.model.ListTablesResponse; import io.unitycatalog.server.model.TableInfo; import io.unitycatalog.server.model.TableType; +import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.PropertyDAO; import io.unitycatalog.server.persist.dao.SchemaInfoDAO; import io.unitycatalog.server.persist.dao.StagingTableDAO; @@ -151,6 +153,7 @@ public TableInfo getTable(String fullName) { TableInfo tableInfo = tableInfoDAO.toTableInfo(true, catalogName, schemaName); RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); + attachDependencies(tableInfo, tableInfoDAO, session); return tableInfo; }, "Failed to get table", @@ -229,6 +232,17 @@ public TableInfo createTable(CreateTable createTable) { } storageLocation = null; tableID = UUID.randomUUID().toString(); + DependencyList viewDeps = createTable.getViewDependencies(); + if (viewDeps != null && viewDeps.getDependencies() != null) { + UUID tableUUID = UUID.fromString(tableID); + List depDAOs = + viewDeps.getDependencies().stream() + .map(dep -> DependencyDAO.from(dep, tableUUID, "TABLE")) + .collect(Collectors.toList()); + repositories + .getDependencyRepository() + .createDependencies(session, tableUUID, "TABLE", depDAOs); + } } else if (tableType == TableType.STREAMING_TABLE) { throw new BaseException( ErrorCode.INVALID_ARGUMENT, "STREAMING TABLE creation is not supported yet."); @@ -360,6 +374,7 @@ public ListTablesResponse listTables( RepositoryUtils.attachProperties( tableInfo, tableInfo.getTableId(), Constants.TABLE, session); } + attachDependencies(tableInfo, tableInfoDAO, session); result.add(tableInfo); } return new ListTablesResponse().tables(result).nextPageToken(nextPageToken); @@ -402,8 +417,26 @@ public void deleteTable(Session session, UUID schemaId, String tableName) { .getDeltaCommitRepository() .permanentlyDeleteTableCommits(session, tableInfoDAO.getId()); } + if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { + repositories + .getDependencyRepository() + .deleteDependencies(session, tableInfoDAO.getId(), "TABLE"); + } PropertyRepository.findProperties(session, tableInfoDAO.getId(), Constants.TABLE) .forEach(session::remove); session.remove(tableInfoDAO); } + + private void attachDependencies(TableInfo tableInfo, TableInfoDAO tableInfoDAO, Session session) { + if ("METRIC_VIEW".equals(tableInfoDAO.getType())) { + List deps = + repositories + .getDependencyRepository() + .getDependencies(session, tableInfoDAO.getId(), "TABLE"); + if (!deps.isEmpty()) { + tableInfo.setViewDependencies( + new DependencyList().dependencies(DependencyDAO.toDependencyList(deps))); + } + } + } } diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java new file mode 100644 index 0000000000..b1872bad05 --- /dev/null +++ b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java @@ -0,0 +1,118 @@ +package io.unitycatalog.server.persist.dao; + +import io.unitycatalog.server.model.Dependency; +import io.unitycatalog.server.model.TableDependency; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; +import org.hibernate.annotations.UuidGenerator; + +@Entity +@Table( + name = "uc_dependencies", + indexes = { + @Index(name = "idx_dependent", columnList = "dependent_type,dependent_id"), + }) +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@EqualsAndHashCode +@ToString +@Builder +public class DependencyDAO { + @Id + @UuidGenerator + @Column(name = "id", updatable = false, nullable = false) + private UUID id; + + @Column(name = "dependent_type", nullable = false) + private String dependentType; + + @Column(name = "dependent_id", nullable = false) + private UUID dependentId; + + @Column(name = "dependency_type", nullable = false) + private String dependencyType; + + @Column(name = "dependency_catalog") + private String dependencyCatalog; + + @Column(name = "dependency_schema") + private String dependencySchema; + + @Column(name = "dependency_name") + private String dependencyName; + + @Column(name = "dependency_target_id") + private UUID dependencyTargetId; + + /** Converts a Dependency API model to a DependencyDAO for a given dependent. */ + public static DependencyDAO from(Dependency dependency, UUID dependentId, String dependentType) { + DependencyDAOBuilder builder = + DependencyDAO.builder().dependentId(dependentId).dependentType(dependentType); + + if (dependency.getTable() != null) { + builder.dependencyType("TABLE"); + String fullName = dependency.getTable().getTableFullName(); + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + builder.dependencyCatalog(parts[0]); + builder.dependencySchema(parts[1]); + builder.dependencyName(parts[2]); + } else { + builder.dependencyName(fullName); + } + } else if (dependency.getFunction() != null) { + builder.dependencyType("FUNCTION"); + String fullName = dependency.getFunction().getFunctionFullName(); + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + builder.dependencyCatalog(parts[0]); + builder.dependencySchema(parts[1]); + builder.dependencyName(parts[2]); + } else { + builder.dependencyName(fullName); + } + } + + return builder.build(); + } + + /** Converts this DAO to a Dependency API model. */ + public Dependency toDependency() { + Dependency dependency = new Dependency(); + if ("TABLE".equals(dependencyType)) { + String fullName = dependencyCatalog + "." + dependencySchema + "." + dependencyName; + dependency.setTable(new TableDependency().tableFullName(fullName)); + } + return dependency; + } + + public String getDependencyFullName() { + if (dependencyCatalog != null && dependencySchema != null && dependencyName != null) { + return dependencyCatalog + "." + dependencySchema + "." + dependencyName; + } + return dependencyName; + } + + public static List toDependencyList(List daos) { + if (daos == null) { + return new ArrayList<>(); + } + return daos.stream().map(DependencyDAO::toDependency).collect(Collectors.toList()); + } +} diff --git a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java index e3ee6672a3..486828bd5a 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java +++ b/server/src/main/java/io/unitycatalog/server/persist/utils/HibernateConfigurator.java @@ -4,6 +4,7 @@ import io.unitycatalog.server.persist.dao.ColumnInfoDAO; import io.unitycatalog.server.persist.dao.CredentialDAO; import io.unitycatalog.server.persist.dao.DeltaCommitDAO; +import io.unitycatalog.server.persist.dao.DependencyDAO; import io.unitycatalog.server.persist.dao.ExternalLocationDAO; import io.unitycatalog.server.persist.dao.FunctionInfoDAO; import io.unitycatalog.server.persist.dao.FunctionParameterInfoDAO; @@ -71,6 +72,7 @@ private static SessionFactory createSessionFactory(Properties hibernatePropertie configuration.addAnnotatedClass(CredentialDAO.class); configuration.addAnnotatedClass(ExternalLocationDAO.class); configuration.addAnnotatedClass(DeltaCommitDAO.class); + configuration.addAnnotatedClass(DependencyDAO.class); ServiceRegistry serviceRegistry = new StandardServiceRegistryBuilder().applySettings(configuration.getProperties()).build(); diff --git a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java index d5cf1462ba..4ef1b14d57 100644 --- a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java +++ b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java @@ -92,7 +92,7 @@ public void testMetricViewCRUD() throws Exception { } @Test - public void testMetricViewWithDependenciesAccepted() throws Exception { + public void testMetricViewWithDependencies() throws Exception { String sourceTableFullName = TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".source_events"; @@ -109,15 +109,17 @@ public void testMetricViewWithDependenciesAccepted() throws Exception { .tableType(TableType.METRIC_VIEW) .viewDefinition(VIEW_DEFINITION) .viewDependencies(depList) - .comment("Metric view with dependencies in payload"); + .comment("Metric view with dependencies"); TableInfo created = tableOperations.createTable(createRequest); assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); - assertThat(fetched.getTableType()).isEqualTo(TableType.METRIC_VIEW); - assertThat(fetched.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(fetched.getViewDependencies()).isNotNull(); + assertThat(fetched.getViewDependencies().getDependencies()).hasSize(1); + assertThat(fetched.getViewDependencies().getDependencies().get(0).getTable().getTableFullName()) + .isEqualTo(sourceTableFullName); tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) From 4f74b84efaf6e0b21e894f8f2ac684653d670b80 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 20:45:51 +0000 Subject: [PATCH 19/26] Keep original column/partition logic in loadTable unchanged --- .../unitycatalog/spark/UCSingleCatalog.scala | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 6def509e5f..07229bbdd9 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -593,21 +593,14 @@ private class UCProxy( val identifier = TableIdentifier(t.getName, Some(t.getSchemaName), Some(t.getCatalogName)) val hasStorage = t.getStorageLocation != null - // Columns val partitionCols = scala.collection.mutable.ArrayBuffer.empty[(String, Int)] - val fields = if (t.getColumns != null) { - t.getColumns.asScala.map { col => - if (hasStorage) { - Option(col.getPartitionIndex).foreach { index => - partitionCols += col.getName -> index - } - } - StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) - .withComment(col.getComment) - }.toArray - } else { - Array.empty[StructField] - } + val fields = t.getColumns.asScala.map { col => + Option(col.getPartitionIndex).foreach { index => + partitionCols += col.getName -> index + } + StructField(col.getName, DataType.fromDDL(col.getTypeText), col.getNullable) + .withComment(col.getComment) + }.toArray // Credential vending -- only for tables with storage val (storage, extraProps) = if (hasStorage) { From bfba3492165b2ed541d79773eeeaf5982638f4ff Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 20:52:07 +0000 Subject: [PATCH 20/26] Rename credProps back to extraSerdeProps to minimize diff --- .../main/scala/io/unitycatalog/spark/UCSingleCatalog.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 07229bbdd9..19b633a951 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -638,7 +638,7 @@ private class UCProxy( enableServerSidePlanningConfig(identifier) } - val credProps = if (temporaryCredentials == null) { + val extraSerdeProps = if (temporaryCredentials == null) { Map.empty[String, String].asJava } else { CredPropsUtil.createTableCredProps( @@ -656,7 +656,7 @@ private class UCProxy( val storageFormat = CatalogStorageFormat.empty.copy( locationUri = Some(locationUri), - properties = t.getProperties.asScala.toMap ++ credProps + properties = t.getProperties.asScala.toMap ++ extraSerdeProps ) (storageFormat, Map.empty[String, String]) } else { From 09f103c2f0ba0af0f77a8f3b91a692269d419c2c Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 21:50:46 +0000 Subject: [PATCH 21/26] Remove unnecessary null checks on columns to match original code --- .../server/persist/TableRepository.java | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index d490dae6ec..b70a463883 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -177,11 +177,9 @@ public TableInfo createTable(CreateTable createTable) { ValidationUtils.validateSqlObjectName(createTable.getName()); String callerId = IdentityUtils.findPrincipalEmailAddress(); List columnInfos = - createTable.getColumns() != null - ? createTable.getColumns().stream() - .map(c -> c.typeText(c.getTypeText().toLowerCase(Locale.ROOT))) - .collect(Collectors.toList()) - : List.of(); + createTable.getColumns().stream() + .map(c -> c.typeText(c.getTypeText().toLowerCase(Locale.ROOT))) + .collect(Collectors.toList()); Long createTime = System.currentTimeMillis(); String fullName = getTableFullName(createTable); LOGGER.debug("Creating table: {}", fullName); @@ -275,15 +273,13 @@ public TableInfo createTable(CreateTable createTable) { TableInfoDAO tableInfoDAO = TableInfoDAO.from(tableInfo, schemaId); // create columns - if (tableInfoDAO.getColumns() != null) { - tableInfoDAO - .getColumns() - .forEach( - c -> { - c.setId(UUID.randomUUID()); - c.setTable(tableInfoDAO); - }); - } + tableInfoDAO + .getColumns() + .forEach( + c -> { + c.setId(UUID.randomUUID()); + c.setTable(tableInfoDAO); + }); // create properties PropertyDAO.from(tableInfo.getProperties(), tableInfoDAO.getId(), Constants.TABLE) .forEach(session::persist); From 5583685a5fb5a5fd54b8919b7fd71fd7d8944820 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 21:55:45 +0000 Subject: [PATCH 22/26] Remove unused getTableById (leftover from definer's rights) --- .../java/io/unitycatalog/server/persist/TableRepository.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index b70a463883..cd0ad3172c 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -290,11 +290,6 @@ public TableInfo createTable(CreateTable createTable) { /* readOnly = */ false); } - /** Retrieves a TableInfoDAO by its ID within an existing session/transaction. */ - public TableInfoDAO getTableById(Session session, UUID tableId) { - return session.get(TableInfoDAO.class, tableId); - } - public TableInfoDAO findBySchemaIdAndName(Session session, UUID schemaId, String name) { String hql = "FROM TableInfoDAO t WHERE t.schemaId = :schemaId AND t.name = :name"; Query query = session.createQuery(hql, TableInfoDAO.class); From 06abf027ca6d364607e0f91987574853d749f3b9 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 21:57:51 +0000 Subject: [PATCH 23/26] Fix test: use YAML view definition instead of SQL --- .../server/base/table/BaseMetricViewCRUDTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java index 4ef1b14d57..16d51df971 100644 --- a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java +++ b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java @@ -21,7 +21,18 @@ public abstract class BaseMetricViewCRUDTest extends BaseTableCRUDTestEnv { protected static final String METRIC_VIEW_FULL_NAME = TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + METRIC_VIEW_NAME; protected static final String VIEW_DEFINITION = - "SELECT date_trunc('day', event_time) AS day, count(*) AS event_count FROM events GROUP BY 1"; + "version: \"0.1\"\n" + + "source: " + + TestUtils.CATALOG_NAME + + "." + + TestUtils.SCHEMA_NAME + + ".source_events\n" + + "dimensions:\n" + + " - name: event_day\n" + + " expr: date_trunc('day', event_time)\n" + + "measures:\n" + + " - name: event_count\n" + + " expr: count(*)"; protected static final Map PROPERTIES = Map.of("team", "analytics", "refresh", "daily"); From b67664024765172a58ff0ead28853c235bafbc60 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Wed, 15 Apr 2026 22:06:08 +0000 Subject: [PATCH 24/26] Require view_dependencies on metric view creation, improve tests Server now validates that view_dependencies is provided when creating a metric view, matching how view_definition is already required. Rewrote tests: - testMetricViewCRUD: includes dependencies in payload (mimics Spark), verifies dependency round-trip on GET, tests full CRUD lifecycle - testMetricViewWithSqlSource: tests SQL statement as source with dependencies - testCreateMetricViewWithoutDefinitionFails: negative test - testCreateMetricViewWithoutDependenciesFails: negative test - Fixed VIEW_DEFINITION to use YAML format instead of SQL --- .../server/persist/TableRepository.java | 10 +- .../base/table/BaseMetricViewCRUDTest.java | 103 ++++++++++++------ 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java index cd0ad3172c..93006a79cc 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java +++ b/server/src/main/java/io/unitycatalog/server/persist/TableRepository.java @@ -228,10 +228,16 @@ public TableInfo createTable(CreateTable createTable) { throw new BaseException( ErrorCode.INVALID_ARGUMENT, "view_definition is required for metric view"); } + DependencyList viewDeps = createTable.getViewDependencies(); + if (viewDeps == null + || viewDeps.getDependencies() == null + || viewDeps.getDependencies().isEmpty()) { + throw new BaseException( + ErrorCode.INVALID_ARGUMENT, "view_dependencies is required for metric view"); + } storageLocation = null; tableID = UUID.randomUUID().toString(); - DependencyList viewDeps = createTable.getViewDependencies(); - if (viewDeps != null && viewDeps.getDependencies() != null) { + if (viewDeps.getDependencies() != null) { UUID tableUUID = UUID.fromString(tableID); List depDAOs = viewDeps.getDependencies().stream() diff --git a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java index 16d51df971..0d0a09cb6a 100644 --- a/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java +++ b/server/src/test/java/io/unitycatalog/server/base/table/BaseMetricViewCRUDTest.java @@ -20,35 +20,59 @@ public abstract class BaseMetricViewCRUDTest extends BaseTableCRUDTestEnv { protected static final String METRIC_VIEW_NAME = "uc_test_metric_view"; protected static final String METRIC_VIEW_FULL_NAME = TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + "." + METRIC_VIEW_NAME; - protected static final String VIEW_DEFINITION = + protected static final String SOURCE_TABLE_FULL_NAME = + TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".source_events"; + + protected static final String VIEW_DEFINITION_ASSET_SOURCE = "version: \"0.1\"\n" + "source: " - + TestUtils.CATALOG_NAME - + "." - + TestUtils.SCHEMA_NAME - + ".source_events\n" + + SOURCE_TABLE_FULL_NAME + + "\n" + "dimensions:\n" + " - name: event_day\n" + " expr: date_trunc('day', event_time)\n" + "measures:\n" + " - name: event_count\n" + " expr: count(*)"; + + protected static final String VIEW_DEFINITION_SQL_SOURCE = + "version: \"0.1\"\n" + + "source: SELECT * FROM " + + SOURCE_TABLE_FULL_NAME + + " WHERE status = 'active'\n" + + "dimensions:\n" + + " - name: region\n" + + " expr: region\n" + + "measures:\n" + + " - name: total_amount\n" + + " expr: SUM(amount)"; + protected static final Map PROPERTIES = Map.of("team", "analytics", "refresh", "daily"); + private static DependencyList makeDependencyList(String... tableFullNames) { + DependencyList depList = new DependencyList(); + depList.setDependencies( + java.util.Arrays.stream(tableFullNames) + .map(name -> new Dependency().table(new TableDependency().tableFullName(name))) + .collect(java.util.stream.Collectors.toList())); + return depList; + } + @Test public void testMetricViewCRUD() throws Exception { assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) .isInstanceOf(Exception.class); - // --- Create --- + // --- Create with asset source --- CreateTable createRequest = new CreateTable() .name(METRIC_VIEW_NAME) .catalogName(TestUtils.CATALOG_NAME) .schemaName(TestUtils.SCHEMA_NAME) .tableType(TableType.METRIC_VIEW) - .viewDefinition(VIEW_DEFINITION) + .viewDefinition(VIEW_DEFINITION_ASSET_SOURCE) + .viewDependencies(makeDependencyList(SOURCE_TABLE_FULL_NAME)) .comment("Daily event counts by day") .properties(PROPERTIES); @@ -57,20 +81,24 @@ public void testMetricViewCRUD() throws Exception { assertThat(created.getCatalogName()).isEqualTo(TestUtils.CATALOG_NAME); assertThat(created.getSchemaName()).isEqualTo(TestUtils.SCHEMA_NAME); assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); - assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION_ASSET_SOURCE); assertThat(created.getTableId()).isNotNull(); assertThat(created.getStorageLocation()) .as("Metric views should have no storage location") .isNull(); - // --- Get --- + // --- Get and verify dependencies round-trip --- TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); assertThat(fetched.getName()).isEqualTo(METRIC_VIEW_NAME); assertThat(fetched.getTableType()).isEqualTo(TableType.METRIC_VIEW); - assertThat(fetched.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(fetched.getViewDefinition()).isEqualTo(VIEW_DEFINITION_ASSET_SOURCE); assertThat(fetched.getComment()).isEqualTo("Daily event counts by day"); assertThat(fetched.getCreatedAt()).isNotNull(); assertThat(fetched.getTableId()).isNotNull(); + assertThat(fetched.getViewDependencies()).isNotNull(); + assertThat(fetched.getViewDependencies().getDependencies()).hasSize(1); + assertThat(fetched.getViewDependencies().getDependencies().get(0).getTable().getTableFullName()) + .isEqualTo(SOURCE_TABLE_FULL_NAME); // Verify properties round-trip assertThat(fetched.getProperties()).isNotNull(); @@ -87,15 +115,6 @@ public void testMetricViewCRUD() throws Exception { METRIC_VIEW_NAME.equals(t.getName()) && TableType.METRIC_VIEW.equals(t.getTableType())); - // --- Create without view_definition should fail --- - CreateTable badRequest = - new CreateTable() - .name("bad_metric_view") - .catalogName(TestUtils.CATALOG_NAME) - .schemaName(TestUtils.SCHEMA_NAME) - .tableType(TableType.METRIC_VIEW); - assertThatThrownBy(() -> tableOperations.createTable(badRequest)).isInstanceOf(Exception.class); - // --- Delete --- tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) @@ -103,14 +122,8 @@ public void testMetricViewCRUD() throws Exception { } @Test - public void testMetricViewWithDependencies() throws Exception { - String sourceTableFullName = - TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".source_events"; - - Dependency dep = new Dependency(); - dep.setTable(new TableDependency().tableFullName(sourceTableFullName)); - DependencyList depList = new DependencyList(); - depList.setDependencies(List.of(dep)); + public void testMetricViewWithSqlSource() throws Exception { + String otherTable = TestUtils.CATALOG_NAME + "." + TestUtils.SCHEMA_NAME + ".other_table"; CreateTable createRequest = new CreateTable() @@ -118,22 +131,42 @@ public void testMetricViewWithDependencies() throws Exception { .catalogName(TestUtils.CATALOG_NAME) .schemaName(TestUtils.SCHEMA_NAME) .tableType(TableType.METRIC_VIEW) - .viewDefinition(VIEW_DEFINITION) - .viewDependencies(depList) - .comment("Metric view with dependencies"); + .viewDefinition(VIEW_DEFINITION_SQL_SOURCE) + .viewDependencies(makeDependencyList(SOURCE_TABLE_FULL_NAME)) + .comment("Metric view with SQL source"); TableInfo created = tableOperations.createTable(createRequest); assertThat(created.getTableType()).isEqualTo(TableType.METRIC_VIEW); - assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION); + assertThat(created.getViewDefinition()).isEqualTo(VIEW_DEFINITION_SQL_SOURCE); TableInfo fetched = tableOperations.getTable(METRIC_VIEW_FULL_NAME); assertThat(fetched.getViewDependencies()).isNotNull(); assertThat(fetched.getViewDependencies().getDependencies()).hasSize(1); - assertThat(fetched.getViewDependencies().getDependencies().get(0).getTable().getTableFullName()) - .isEqualTo(sourceTableFullName); tableOperations.deleteTable(METRIC_VIEW_FULL_NAME); - assertThatThrownBy(() -> tableOperations.getTable(METRIC_VIEW_FULL_NAME)) - .isInstanceOf(Exception.class); + } + + @Test + public void testCreateMetricViewWithoutDefinitionFails() throws Exception { + CreateTable badRequest = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDependencies(makeDependencyList(SOURCE_TABLE_FULL_NAME)); + assertThatThrownBy(() -> tableOperations.createTable(badRequest)).isInstanceOf(Exception.class); + } + + @Test + public void testCreateMetricViewWithoutDependenciesFails() throws Exception { + CreateTable badRequest = + new CreateTable() + .name(METRIC_VIEW_NAME) + .catalogName(TestUtils.CATALOG_NAME) + .schemaName(TestUtils.SCHEMA_NAME) + .tableType(TableType.METRIC_VIEW) + .viewDefinition(VIEW_DEFINITION_ASSET_SOURCE); + assertThatThrownBy(() -> tableOperations.createTable(badRequest)).isInstanceOf(Exception.class); } } From 31c7ab4059c576702cccaecb81b047fe3cfeafef Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Thu, 16 Apr 2026 19:43:08 +0000 Subject: [PATCH 25/26] Remove FunctionDependency from view dependencies UDFs are not supported in metric view expressions (the OSS connector does not implement FunctionCatalog), so function dependencies are not applicable. Removed: - FunctionDependency schema from all.yaml - function field from Dependency schema - FunctionDependency case in connector dependency conversion - FUNCTION handling in DependencyDAO.from() --- api/all.yaml | 14 +------------- .../io/unitycatalog/spark/UCSingleCatalog.scala | 4 ---- .../server/persist/dao/DependencyDAO.java | 11 ----------- 3 files changed, 1 insertion(+), 28 deletions(-) diff --git a/api/all.yaml b/api/all.yaml index 547aed5989..bb3b302adf 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -2082,15 +2082,6 @@ components: "$ref": "#/components/schemas/CreateFunction" required: - function_info - FunctionDependency: - description: A function that is dependent on a SQL object. - type: object - required: - - function_full_name - properties: - function_full_name: - description: Full name of the dependent function, in the form of __catalog_name__.__schema_name__.__function_name__. - type: string TableDependency: description: A table that is dependent on a SQL object. type: object @@ -2101,14 +2092,11 @@ components: description: Full name of the dependent table, in the form of __catalog_name__.__schema_name__.__table_name__. type: string Dependency: - description: A dependency of a SQL object. Either the __table__ field or the - __function__ field must be defined. + description: A dependency of a SQL object. type: object properties: table: "$ref": "#/components/schemas/TableDependency" - function: - "$ref": "#/components/schemas/FunctionDependency" DependencyList: description: A list of dependencies. type: object diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 19b633a951..7640c05027 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -756,10 +756,6 @@ private class UCProxy( ucDep.setTable( new io.unitycatalog.client.model.TableDependency() .tableFullName(td.tableFullName())) - case fd: org.apache.spark.sql.connector.catalog.FunctionDependency => - ucDep.setFunction( - new io.unitycatalog.client.model.FunctionDependency() - .functionFullName(fd.functionFullName())) case _ => } ucDep diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java index b1872bad05..2b3a6bd623 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java +++ b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java @@ -76,17 +76,6 @@ public static DependencyDAO from(Dependency dependency, UUID dependentId, String } else { builder.dependencyName(fullName); } - } else if (dependency.getFunction() != null) { - builder.dependencyType("FUNCTION"); - String fullName = dependency.getFunction().getFunctionFullName(); - String[] parts = fullName.split("\\."); - if (parts.length == 3) { - builder.dependencyCatalog(parts[0]); - builder.dependencySchema(parts[1]); - builder.dependencyName(parts[2]); - } else { - builder.dependencyName(fullName); - } } return builder.build(); From d10f8e4dc9da9e5905bb9cbc50464fdc054449b6 Mon Sep 17 00:00:00 2001 From: "chen.wang" Date: Fri, 17 Apr 2026 18:32:07 +0000 Subject: [PATCH 26/26] Revert "Remove FunctionDependency from view dependencies" This reverts commit 31c7ab4059c576702cccaecb81b047fe3cfeafef. --- api/all.yaml | 14 +++++++++++++- .../io/unitycatalog/spark/UCSingleCatalog.scala | 4 ++++ .../server/persist/dao/DependencyDAO.java | 11 +++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/api/all.yaml b/api/all.yaml index bb3b302adf..547aed5989 100644 --- a/api/all.yaml +++ b/api/all.yaml @@ -2082,6 +2082,15 @@ components: "$ref": "#/components/schemas/CreateFunction" required: - function_info + FunctionDependency: + description: A function that is dependent on a SQL object. + type: object + required: + - function_full_name + properties: + function_full_name: + description: Full name of the dependent function, in the form of __catalog_name__.__schema_name__.__function_name__. + type: string TableDependency: description: A table that is dependent on a SQL object. type: object @@ -2092,11 +2101,14 @@ components: description: Full name of the dependent table, in the form of __catalog_name__.__schema_name__.__table_name__. type: string Dependency: - description: A dependency of a SQL object. + description: A dependency of a SQL object. Either the __table__ field or the + __function__ field must be defined. type: object properties: table: "$ref": "#/components/schemas/TableDependency" + function: + "$ref": "#/components/schemas/FunctionDependency" DependencyList: description: A list of dependencies. type: object diff --git a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala index 7640c05027..19b633a951 100644 --- a/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala +++ b/connectors/spark/src/main/scala/io/unitycatalog/spark/UCSingleCatalog.scala @@ -756,6 +756,10 @@ private class UCProxy( ucDep.setTable( new io.unitycatalog.client.model.TableDependency() .tableFullName(td.tableFullName())) + case fd: org.apache.spark.sql.connector.catalog.FunctionDependency => + ucDep.setFunction( + new io.unitycatalog.client.model.FunctionDependency() + .functionFullName(fd.functionFullName())) case _ => } ucDep diff --git a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java index 2b3a6bd623..b1872bad05 100644 --- a/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java +++ b/server/src/main/java/io/unitycatalog/server/persist/dao/DependencyDAO.java @@ -76,6 +76,17 @@ public static DependencyDAO from(Dependency dependency, UUID dependentId, String } else { builder.dependencyName(fullName); } + } else if (dependency.getFunction() != null) { + builder.dependencyType("FUNCTION"); + String fullName = dependency.getFunction().getFunctionFullName(); + String[] parts = fullName.split("\\."); + if (parts.length == 3) { + builder.dependencyCatalog(parts[0]); + builder.dependencySchema(parts[1]); + builder.dependencyName(parts[2]); + } else { + builder.dependencyName(fullName); + } } return builder.build();