Copilot commented on code in PR #10988:
URL: https://github.com/apache/gravitino/pull/10988#discussion_r3209448091
##########
common/src/main/java/org/apache/gravitino/dto/rel/ViewDTO.java:
##########
@@ -104,7 +106,23 @@ public Column[] columns() {
@Override
public Representation[] representations() {
- return representations == null ? new RepresentationDTO[0] :
representations;
+ if (representations == null) {
+ return new Representation[0];
+ }
+ return Stream.of(representations)
+ .map(
+ rep -> {
+ if (rep instanceof SQLRepresentationDTO) {
+ SQLRepresentationDTO dto = (SQLRepresentationDTO) rep;
+ return (Representation)
+ SQLRepresentation.builder()
+ .withDialect(dto.dialect())
+ .withSql(dto.sql())
+ .build();
+ }
+ return (Representation) rep;
+ })
+ .toArray(Representation[]::new);
Review Comment:
ViewDTO.representations() rebuilds SQLRepresentation objects on every call
and duplicates conversion logic that already exists in DTOConverters
(fromDTO/fromDTOs). Consider delegating to
DTOConverters.fromDTOs(representations) and/or caching the converted array to
avoid repeated allocations and keep conversion logic centralized.
##########
clients/client-java/src/main/java/org/apache/gravitino/client/RelationalCatalog.java:
##########
@@ -283,8 +300,169 @@ public boolean purgeTable(NameIdentifier ident) throws
UnsupportedOperationExcep
return resp.dropped();
}
+ /**
+ * List all the views under the given Schema namespace.
+ *
+ * @param namespace The namespace to list the views under it. This namespace
should have 1 level,
+ * which is the schema name.
+ * @return An array of {@link NameIdentifier} of the views under the given
namespace.
+ * @throws NoSuchSchemaException if the schema with specified namespace does
not exist.
+ */
+ @Override
+ public NameIdentifier[] listViews(Namespace namespace) throws
NoSuchSchemaException {
+ checkViewNamespace(namespace);
+
+ Namespace fullNamespace = getEntityFullNamespace(namespace);
+ EntityListResponse resp =
+ restClient.get(
+ formatViewRequestPath(fullNamespace),
+ EntityListResponse.class,
+ Collections.emptyMap(),
+ ErrorHandlers.viewErrorHandler());
+ resp.validate();
+
+ return Arrays.stream(resp.identifiers())
+ .map(ident -> NameIdentifier.of(ident.namespace().level(2),
ident.name()))
+ .toArray(NameIdentifier[]::new);
+ }
+
+ /**
+ * Load the view with specified identifier.
+ *
+ * @param ident The identifier of the view to load, which should be
"schema.view" format.
+ * @return The {@link View} with specified identifier.
+ * @throws NoSuchViewException if the view with specified identifier does
not exist.
+ */
+ @Override
+ public View loadView(NameIdentifier ident) throws NoSuchViewException {
+ checkViewNameIdentifier(ident);
+
+ Namespace fullNamespace = getEntityFullNamespace(ident.namespace());
+ ViewResponse resp =
+ restClient.get(
+ formatViewRequestPath(fullNamespace) + "/" +
RESTUtils.encodeString(ident.name()),
+ ViewResponse.class,
+ Collections.emptyMap(),
+ ErrorHandlers.viewErrorHandler());
+ resp.validate();
+
+ return resp.getView();
+ }
+
+ /**
+ * Create a new view with specified identifier, columns, representations and
other metadata.
+ *
+ * @param ident The identifier of the view, which should be "schema.view"
format.
+ * @param comment The comment of the view, may be {@code null}.
+ * @param columns The output columns of the view.
+ * @param representations The representations of the view. At least one
representation is
+ * expected.
+ * @param defaultCatalog The default catalog used to resolve unqualified
identifiers referenced by
+ * the view definition, or {@code null} if not set.
+ * @param defaultSchema The default schema used to resolve unqualified
identifiers referenced by
+ * the view definition, or {@code null} if not set.
+ * @param properties The properties of the view.
+ * @return The created {@link View}.
+ * @throws NoSuchSchemaException if the schema with specified namespace does
not exist.
+ * @throws ViewAlreadyExistsException if the view with specified identifier
already exists.
+ */
+ @Override
+ public View createView(
+ NameIdentifier ident,
+ String comment,
+ Column[] columns,
+ Representation[] representations,
+ String defaultCatalog,
+ String defaultSchema,
+ Map<String, String> properties)
+ throws NoSuchSchemaException, ViewAlreadyExistsException {
+ checkViewNameIdentifier(ident);
+
+ ColumnDTO[] columnDTOs = columns == null ? new ColumnDTO[0] :
toDTOs(columns);
Review Comment:
In createView(), the explicit null-to-empty conversion for columns is
redundant because DTOConverters.toDTOs(Column[]) already returns an empty array
for null/empty input. Using toDTOs(columns) directly would simplify the code
and keep it consistent with createTable().
--
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]
