mchades commented on code in PR #10924:
URL: https://github.com/apache/gravitino/pull/10924#discussion_r3167737250
##########
core/src/main/java/org/apache/gravitino/catalog/ViewOperationDispatcher.java:
##########
@@ -66,149 +73,498 @@ public ViewOperationDispatcher(
}
/**
- * Load view metadata by identifier from the catalog.
+ * Lists the views within a schema.
*
- * <p>This method first checks if the view exists in Gravitino's
EntityStore. If not found, it
- * loads from the catalog and auto-imports into EntityStore.
+ * @param namespace The namespace of the schema containing the views.
+ * @return An array of {@link NameIdentifier} objects representing the
identifiers of the views in
+ * the schema.
+ * @throws NoSuchSchemaException If the specified schema does not exist.
+ */
+ @Override
+ public NameIdentifier[] listViews(Namespace namespace) throws
NoSuchSchemaException {
+ return TreeLockUtils.doWithTreeLock(
+ NameIdentifier.of(namespace.levels()),
+ LockType.READ,
+ () ->
+ doWithCatalog(
+ getCatalogIdentifier(NameIdentifier.of(namespace.levels())),
+ c -> c.doWithViewOps(v -> v.listViews(namespace)),
+ NoSuchSchemaException.class));
+ }
+
+ /**
+ * Loads a view.
*
- * @param ident The view identifier.
- * @return The loaded view metadata.
- * @throws NoSuchViewException If the view does not exist.
+ * @param ident The identifier of the view to load.
+ * @return The loaded {@link View} object representing the requested view.
+ * @throws NoSuchViewException If the specified view does not exist.
*/
@Override
public View loadView(NameIdentifier ident) throws NoSuchViewException {
LOG.info("Loading view: {}", ident);
- // First load with READ lock to check if view is already imported
EntityCombinedView entityCombinedView =
TreeLockUtils.doWithTreeLock(ident, LockType.READ, () ->
internalLoadView(ident));
if (!entityCombinedView.imported()) {
- // Load the schema to make sure the schema is imported.
SchemaDispatcher schemaDispatcher =
GravitinoEnv.getInstance().schemaDispatcher();
NameIdentifier schemaIdent =
NameIdentifier.of(ident.namespace().levels());
schemaDispatcher.loadSchema(schemaIdent);
- // Import the view.
entityCombinedView =
TreeLockUtils.doWithTreeLock(schemaIdent, LockType.WRITE, () ->
importView(ident));
}
return entityCombinedView;
}
+ /**
+ * Creates a new view in a schema.
+ *
+ * @param ident The identifier of the view to create.
+ * @param comment A description or comment associated with the view.
+ * @param columns An array of {@link Column} objects representing the output
columns of the view.
+ * @param representations An array of {@link Representation} objects
representing the SQL
+ * definitions of the view across different dialects.
+ * @param defaultCatalog The default catalog to use for unqualified
references.
+ * @param defaultSchema The default schema to use for unqualified references.
+ * @param properties Additional properties to set for the view.
+ * @return The newly created {@link View} object.
+ * @throws NoSuchSchemaException If the schema in which to create the view
does not exist.
+ * @throws ViewAlreadyExistsException If a view with the same name already
exists in the schema.
+ */
+ @Override
+ public View createView(
+ NameIdentifier ident,
+ String comment,
+ Column[] columns,
+ Representation[] representations,
+ @Nullable String defaultCatalog,
+ @Nullable String defaultSchema,
+ Map<String, String> properties)
+ throws NoSuchSchemaException, ViewAlreadyExistsException {
+ Preconditions.checkArgument(
+ representations != null && representations.length >= 1,
+ "representations must not be null or empty");
+
+ // Load the schema to make sure the schema exists.
+ SchemaDispatcher schemaDispatcher =
GravitinoEnv.getInstance().schemaDispatcher();
+ NameIdentifier schemaIdent = NameIdentifier.of(ident.namespace().levels());
+ schemaDispatcher.loadSchema(schemaIdent);
+
+ return TreeLockUtils.doWithTreeLock(
+ schemaIdent,
+ LockType.WRITE,
+ () ->
+ internalCreateView(
+ ident,
+ comment,
+ columns,
+ representations,
+ defaultCatalog,
+ defaultSchema,
+ properties));
+ }
+
+ /**
+ * Alters an existing view.
+ *
+ * @param ident The identifier of the view to alter.
+ * @param changes An array of {@link ViewChange} objects representing the
changes to apply to the
+ * view.
+ * @return The altered {@link View} object after applying the changes.
+ * @throws NoSuchViewException If the view to alter does not exist.
+ * @throws IllegalArgumentException If an unsupported or invalid change is
specified.
+ */
@Override
- public NameIdentifier[] listViews(Namespace namespace) {
- throw new UnsupportedOperationException("Listing views is not supported
yet");
+ public View alterView(NameIdentifier ident, ViewChange... changes)
+ throws NoSuchViewException, IllegalArgumentException {
+ validateAlterProperties(ident,
HasPropertyMetadata::tablePropertiesMetadata, changes);
+ NameIdentifier lockIdent = ident;
+ for (ViewChange change : changes) {
+ if (change instanceof ViewChange.RenameView) {
+ lockIdent = getSchemaIdentifier(ident);
+ break;
+ }
+ }
+
+ NameIdentifier nameIdentifierForLock = lockIdent;
+ return TreeLockUtils.doWithTreeLock(
+ nameIdentifierForLock,
+ nameIdentifierForLock.equals(ident) ? LockType.READ : LockType.WRITE,
+ () -> {
+ NameIdentifier catalogIdent = getCatalogIdentifier(ident);
+ View alteredView =
+ doWithCatalog(
+ catalogIdent,
+ c ->
+ c.doWithViewOps(
+ v -> v.alterView(ident,
applyCapabilities(c.capabilities(), changes))),
+ NoSuchViewException.class,
+ IllegalArgumentException.class);
+
+ boolean isManagedView = isManagedEntity(catalogIdent,
Capability.Scope.VIEW);
+ if (isManagedView) {
+ return EntityCombinedView.of(alteredView)
+ .withHiddenProperties(
+ getHiddenPropertyNames(
+ catalogIdent,
+ HasPropertyMetadata::tablePropertiesMetadata,
+ alteredView.properties()));
+ }
+
+ ViewEntity existing = getEntity(ident, VIEW, ViewEntity.class);
+ if (existing == null) {
+ return EntityCombinedView.of(alteredView)
+ .withHiddenProperties(
+ getHiddenPropertyNames(
+ catalogIdent,
+ HasPropertyMetadata::tablePropertiesMetadata,
+ alteredView.properties()));
+ }
Review Comment:
Good catch. Fixed to mirror `alterTable`: first resolve entity ID from
`StringIdentifier` in `alteredView.properties()`, fall back to name-based
lookup only when no string ID is present.
--
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]
