lasdf1234 commented on PR #11828: URL: https://github.com/apache/gravitino/pull/11828#issuecomment-4851130745
> Thanks for the review @lasdf1234! Both points addressed in [dedb8ee](https://github.com/apache/gravitino/commit/dedb8eebdc85dc884ba4bd2b67fa02aa72579de4): > > Fail hard on import: Added a new importViewForRegister() method that propagates exceptions instead of using the best-effort importView(). This matches the registerTable pattern in IcebergNamespaceHookDispatcher. > > Skip if entity already exists: importViewForRegister() checks EntityStore.exists() before importing/setting ownership. If the view entity is already in the Gravitino store, it returns early with a log message. You can refer to this code to modify (based on IcebergNamespaceHookDispatcher's registerTable) : @Override public LoadViewResponse registerView( IcebergRequestContext context, Namespace namespace, RegisterViewRequest registerViewRequest) { LoadViewResponse response = dispatcher.registerView(context, namespace, registerViewRequest); if (hasViewEntityInStore(context, namespace, registerViewRequest.name())) { // Gravitino already tracks this view: preserve view_id and existing role/owner/tag/policy // bindings after the backend registerView completes. return response; } // Import is intentionally NOT wrapped in try-catch: ... importViewForRegister(context.catalogName(), namespace, registerViewRequest.name()); IcebergOwnershipUtils.setViewOwner( metalake, context.catalogName(), namespace, registerViewRequest.name(), context.userName(), GravitinoEnv.getInstance().internalOwnerDispatcher()); return response; } private boolean hasViewEntityInStore( IcebergRequestContext context, Namespace namespace, String viewName) { .... } -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
