jerryshao commented on code in PR #7734:
URL: https://github.com/apache/gravitino/pull/7734#discussion_r2273339615


##########
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java:
##########
@@ -260,4 +261,42 @@ public <E extends Entity & HasIdentifier> List<E> 
updateEntityRelations(
     return backend.updateEntityRelations(
         relType, srcEntityIdent, srcEntityType, destEntitiesToAdd, 
destEntitiesToRemove);
   }
+
+  @Override
+  public <E extends Entity & HasIdentifier> void insertEntitiesAndRelations(
+      Type relType, List<RelationalEntity<E>> relationalEntities, boolean 
overwrite)
+      throws IOException {
+    for (RelationalEntity<E> relationalEntity : relationalEntities) {
+      if (relationalEntity.vertexType() == Relation.VertexType.SOURCE) {
+        cache.invalidate(
+            relationalEntity.entity().nameIdentifier(), 
relationalEntity.entity().type(), relType);
+        cache.invalidate(
+            relationalEntity.entity().nameIdentifier(), 
relationalEntity.entity().type());
+      } else {
+        for (NameIdentifier relatedNameIdent : 
relationalEntity.relatedIdents()) {
+          cache.invalidate(relatedNameIdent, 
relationalEntity.relatedEntityType(), relType);
+        }
+        cache.invalidate(
+            relationalEntity.entity().nameIdentifier(), 
relationalEntity.entity().type());
+      }
+    }

Review Comment:
   Can you explain more about this?



##########
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntity.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage.relational;
+
+import java.util.List;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Relation;
+
+/**
+ * Represents a relational entity that contains an entity, its vertex type, 
related identifiers, and
+ * the type of related entity.
+ *
+ * @param <E> the type of the entity, which must extend {@link Entity} and 
implement {@link
+ *     HasIdentifier}
+ */
+public class RelationalEntity<E extends Entity & HasIdentifier> {

Review Comment:
   I suggest we move this class to put with `Entity`.



##########
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntity.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage.relational;
+
+import java.util.List;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Relation;
+
+/**
+ * Represents a relational entity that contains an entity, its vertex type, 
related identifiers, and
+ * the type of related entity.
+ *
+ * @param <E> the type of the entity, which must extend {@link Entity} and 
implement {@link
+ *     HasIdentifier}
+ */
+public class RelationalEntity<E extends Entity & HasIdentifier> {
+  private final E entity;
+  private final Relation.VertexType vertexType;
+  private final List<NameIdentifier> relatedIdents;
+  private final Entity.EntityType relatedEntityType;

Review Comment:
   Who is the source entity, and who is the destination entity, you'd better 
clearly naming the variables, which will be easy to understand.



##########
core/src/main/java/org/apache/gravitino/stats/StatisticManager.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.stats;
+
+import com.google.common.collect.Lists;
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.Relation;
+import org.apache.gravitino.SupportsRelationOperations;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
+import org.apache.gravitino.exceptions.UnmodifiableStatisticException;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.StatisticEntity;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.relational.RelationalEntity;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class StatisticManager {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(StatisticManager.class);
+
+  private final EntityStore store;
+
+  private final IdGenerator idGenerator;
+
+  public StatisticManager(EntityStore store, IdGenerator idGenerator) {
+    this.store = store;
+    this.idGenerator = idGenerator;
+  }
+
+  public List<Statistic> listStatistics(String metalake, MetadataObject 
metadataObject) {
+    try {
+      // TODO: we will add lock when we support Iceberg, Hive statistics. 
Because the operations are
+      // only related to entity store now. We don't need to lock the metadata 
object.
+      NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalake, 
metadataObject);
+      Entity.EntityType type = MetadataObjectUtil.toEntityType(metadataObject);
+      return store.relationOperations()
+          .listEntitiesByRelation(
+              SupportsRelationOperations.Type.METADATA_OBJECT_STAT_REL, 
identifier, type)
+          .stream()
+          .map(
+              entity -> {
+                StatisticEntity statisticEntity = (StatisticEntity) entity;
+                String name = statisticEntity.name();
+                StatisticValue<?> value = statisticEntity.value();
+                return new CustomStatistic(name, value);
+              })
+          .collect(Collectors.toList());
+    } catch (NoSuchEntityException nse) {
+      LOG.warn(
+          "Failed to list statistics for metadata object {} in the metalake 
{}: {}",
+          metadataObject.fullName(),
+          metalake,
+          nse.getMessage());
+      throw new NoSuchMetadataObjectException(
+          "The metadata object  %s in the metalake %s isn't found",
+          metadataObject.fullName(), metalake);
+    } catch (IOException ioe) {
+      LOG.error(
+          "Failed to list statistics for metadata object {} in the metalake {} 
: {}",
+          metadataObject.fullName(),
+          metalake,
+          ioe.getMessage());
+      throw new RuntimeException(ioe);
+    }
+  }
+
+  public List<Statistic> updateStatistics(

Review Comment:
   Returning a list of statistics that is passed in by the user seems 
meaningless, either you can return a full table stats, or you just return void.



##########
core/src/main/java/org/apache/gravitino/stats/StatisticManager.java:
##########
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.stats;
+
+import com.google.common.collect.Lists;
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.Relation;
+import org.apache.gravitino.SupportsRelationOperations;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
+import org.apache.gravitino.exceptions.UnmodifiableStatisticException;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.StatisticEntity;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.relational.RelationalEntity;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class StatisticManager {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(StatisticManager.class);
+
+  private final EntityStore store;
+
+  private final IdGenerator idGenerator;
+
+  public StatisticManager(EntityStore store, IdGenerator idGenerator) {
+    this.store = store;
+    this.idGenerator = idGenerator;
+  }
+
+  public List<Statistic> listStatistics(String metalake, MetadataObject 
metadataObject) {
+    try {
+      // TODO: we will add lock when we support Iceberg, Hive statistics. 
Because the operations are
+      // only related to entity store now. We don't need to lock the metadata 
object.
+      NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalake, 
metadataObject);
+      Entity.EntityType type = MetadataObjectUtil.toEntityType(metadataObject);
+      return store.relationOperations()
+          .listEntitiesByRelation(
+              SupportsRelationOperations.Type.METADATA_OBJECT_STAT_REL, 
identifier, type)
+          .stream()
+          .map(
+              entity -> {
+                StatisticEntity statisticEntity = (StatisticEntity) entity;
+                String name = statisticEntity.name();
+                StatisticValue<?> value = statisticEntity.value();
+                return new CustomStatistic(name, value);
+              })
+          .collect(Collectors.toList());
+    } catch (NoSuchEntityException nse) {
+      LOG.warn(
+          "Failed to list statistics for metadata object {} in the metalake 
{}: {}",
+          metadataObject.fullName(),
+          metalake,
+          nse.getMessage());
+      throw new NoSuchMetadataObjectException(
+          "The metadata object  %s in the metalake %s isn't found",

Review Comment:
   One more whitespace before "%s".



-- 
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]

Reply via email to