Copilot commented on code in PR #10831:
URL: https://github.com/apache/gravitino/pull/10831#discussion_r3135054782
##########
api/src/main/java/org/apache/gravitino/rel/View.java:
##########
@@ -20,33 +20,108 @@
import java.util.Collections;
import java.util.Map;
+import java.util.Optional;
import javax.annotation.Nullable;
import org.apache.gravitino.Auditable;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.annotation.Unstable;
/**
- * An interface representing a view in a {@link Namespace}. It defines the
basic properties of a
- * view. A catalog implementation with {@link ViewCatalog} should implement
this interface.
+ * An interface representing a logical view in a {@link Namespace}. A view is
a named query whose
+ * definition may be expressed in one or more SQL dialects. A catalog
implementation with {@link
+ * ViewCatalog} should implement this interface.
*/
Review Comment:
PR description/issue mention adding a view security mode API (e.g.,
SecurityMode DEFINER/INVOKER), but there is no SecurityMode type in the repo
and View doesn't expose any security-mode attribute. Either add the missing
SecurityMode API surface (and corresponding View accessor) or update the PR
description/scope so it matches what’s actually being introduced.
##########
api/src/main/java/org/apache/gravitino/rel/View.java:
##########
@@ -20,33 +20,108 @@
import java.util.Collections;
import java.util.Map;
+import java.util.Optional;
import javax.annotation.Nullable;
import org.apache.gravitino.Auditable;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.annotation.Unstable;
/**
- * An interface representing a view in a {@link Namespace}. It defines the
basic properties of a
- * view. A catalog implementation with {@link ViewCatalog} should implement
this interface.
+ * An interface representing a logical view in a {@link Namespace}. A view is
a named query whose
+ * definition may be expressed in one or more SQL dialects. A catalog
implementation with {@link
+ * ViewCatalog} should implement this interface.
*/
@Unstable
public interface View extends Auditable {
/**
- * @return The name of the view.
+ * Returns the name of the view.
+ *
+ * @return The view name.
*/
String name();
/**
- * @return The comment of the view, null if no comment is set.
+ * Returns the comment of the view, or {@code null} if no comment is set.
+ *
+ * @return The view comment, or {@code null}.
*/
@Nullable
default String comment() {
return null;
}
/**
- * @return The properties of the view, empty map if no properties are set.
+ * Returns the output schema of the view. Implementations should return an
empty array when the
+ * output schema is unknown; callers should not rely on a {@code null}
return value.
+ *
+ * @return The view output columns.
+ */
+ Column[] columns();
+
+ /**
+ * Returns the representations of the view. A view carries at least one
{@link Representation},
+ * typically a {@link SQLRepresentation} for one or more dialects.
+ *
+ * @return The view representations.
+ */
Review Comment:
`representations()` is declared as non-null in the signature/Javadoc, but
`sqlFor(...)` explicitly treats a `null` return as valid. For a public API,
please make the nullability contract explicit and consistent (e.g.,
document/annotate `@Nullable` and handle null everywhere, or require
non-null/empty arrays and drop the null branch in `sqlFor`).
--
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]
