jerryshao commented on code in PR #7690:
URL: https://github.com/apache/gravitino/pull/7690#discussion_r2262529802
##########
common/src/main/java/org/apache/gravitino/json/JsonUtils.java:
##########
@@ -1343,6 +1349,94 @@ public Expression deserialize(JsonParser p,
DeserializationContext ctxt) throws
}
}
+ /** Custom JSON deserializer for StatisticValue objects. */
+ public static class StatisticValueDeserializer extends
JsonDeserializer<StatisticValue> {
+ @Override
+ public StatisticValue<?> deserialize(JsonParser p, DeserializationContext
ctxt)
+ throws IOException {
+ JsonNode node = p.getCodec().readTree(p);
+ return getStatisticValue(node);
+ }
+ }
Review Comment:
Add test to cover this serializer and deserializer.
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/StatisticBaseSQLProvider.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.mapper.provider.base;
+
+import static
org.apache.gravitino.storage.relational.mapper.StatisticMetaMapper.STATISTIC_META_TABLE_NAME;
+
+import java.util.List;
+import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.FilesetMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.TableMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.TopicMetaMapper;
+import org.apache.gravitino.storage.relational.po.StatisticPO;
+import org.apache.ibatis.annotations.Param;
+
+public class StatisticBaseSQLProvider {
+
+ public String batchInsertStatisticPOs(@Param("statisticPOs")
List<StatisticPO> statisticPOs) {
+ return "<script>"
+ + "INSERT INTO "
+ + STATISTIC_META_TABLE_NAME
+ + " (statistic_id, statistic_name, statistic_value, metalake_id,
metadata_object_id,"
+ + " metadata_object_type, audit_info, current_version, last_version,
deleted_at) VALUES "
+ + "<foreach collection='statisticPOs' item='item' separator=','>"
+ + "(#{item.statisticId}, "
+ + "#{item.statisticName}, "
+ + "#{item.statisticValue}, "
+ + "#{item.metalakeId}, "
+ + "#{item.metadataObjectId}, "
+ + "#{item.metadataObjectType}, "
+ + "#{item.auditInfo}, "
+ + "#{item.currentVersion}, "
+ + "#{item.lastVersion}, "
+ + "#{item.deletedAt})"
+ + "</foreach>"
+ + "</script>";
+ }
+
+ public String batchDeleteStatisticPOs(
+ @Param("objectId") Long objectId, @Param("statisticNames") List<String>
statisticNames) {
+ return "<script>"
+ + "UPDATE "
+ + STATISTIC_META_TABLE_NAME
+ + softDeleteSQL()
+ + " WHERE FALSE "
+ + "<foreach collection='statisticNames' item='item' separator=' '>"
+ + " OR (statistic_name = #{item} AND"
+ + " deleted_at = 0 AND metadata_object_id = #{objectId})"
+ + "</foreach>"
+ + "</script>";
Review Comment:
Shall we use `static_name IN`, rather than concatenate the SQL?
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/StatisticMetaMapper.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.mapper;
+
+import java.util.List;
+import org.apache.gravitino.storage.relational.po.StatisticPO;
+import org.apache.ibatis.annotations.DeleteProvider;
+import org.apache.ibatis.annotations.InsertProvider;
+import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.annotations.SelectProvider;
+import org.apache.ibatis.annotations.UpdateProvider;
+
+public interface StatisticMetaMapper {
+
+ String STATISTIC_META_TABLE_NAME = "statistic_meta";
+
+ @SelectProvider(type = StatisticSQLProviderFactory.class, method =
"listStatisticPOsByObjectId")
+ List<StatisticPO> listStatisticPOsByObjectId(@Param("objectId") long
objectId);
Review Comment:
Change all the object id to entity id.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/StatisticMetaService.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.service;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.meta.StatisticEntity;
+import org.apache.gravitino.storage.relational.mapper.StatisticMetaMapper;
+import org.apache.gravitino.storage.relational.po.StatisticPO;
+import org.apache.gravitino.storage.relational.utils.POConverters;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/**
+ * The service class for statistic metadata. It provides the basic database
operations for
+ * statistic.
+ */
+public class StatisticMetaService {
+
+ private static final StatisticMetaService INSTANCE = new
StatisticMetaService();
+
+ public static StatisticMetaService getInstance() {
+ return INSTANCE;
+ }
+
+ private StatisticMetaService() {}
+
+ public List<StatisticEntity> listStatisticsByObject(
+ NameIdentifier identifier, Entity.EntityType type) {
+ long metalakeId =
+ MetalakeMetaService.getInstance()
+ .getMetalakeIdByName(NameIdentifierUtil.getMetalake(identifier));
+ MetadataObject object = NameIdentifierUtil.toMetadataObject(identifier,
type);
+ long objectId =
+ MetadataObjectService.getMetadataObjectId(metalakeId,
object.fullName(), object.type());
+ List<StatisticPO> statisticPOs =
+ SessionUtils.getWithoutCommit(
+ StatisticMetaMapper.class, mapper ->
mapper.listStatisticPOsByObjectId(objectId));
+ return
statisticPOs.stream().map(POConverters::fromStatisticPO).collect(Collectors.toList());
+ }
Review Comment:
We should unify all the places using "entity" rather than "object" for
internal representation.
##########
core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java:
##########
@@ -1720,4 +1723,50 @@ private static ModelVersionAliasRelPO
createAliasRelPO(ModelVersionEntity entity
.withDeletedAt(DEFAULT_DELETED_AT)
.build();
}
+
+ public static StatisticEntity fromStatisticPO(StatisticPO statisticPO) {
+ try {
+ return StatisticEntity.builder()
+ .withId(statisticPO.getStatisticId())
+ .withName(statisticPO.getStatisticName())
+ .withValue(
+ JsonUtils.anyFieldMapper()
+ .readValue(statisticPO.getStatisticValue(),
StatisticValue.class))
+ .withAuditInfo(
+ JsonUtils.anyFieldMapper().readValue(statisticPO.getAuditInfo(),
AuditInfo.class))
+ .build();
+ } catch (JsonProcessingException je) {
+ throw new RuntimeException(je);
+ }
+ }
+
+ public static List<StatisticPO> initializeStatisticPOs(
Review Comment:
Can you please move these PO conversion logic to `StatisticPO` like JobPO,
this class is too big.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/StatisticMetaService.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.service;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.meta.StatisticEntity;
+import org.apache.gravitino.storage.relational.mapper.StatisticMetaMapper;
+import org.apache.gravitino.storage.relational.po.StatisticPO;
+import org.apache.gravitino.storage.relational.utils.POConverters;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+
+/**
+ * The service class for statistic metadata. It provides the basic database
operations for
+ * statistic.
+ */
+public class StatisticMetaService {
+
+ private static final StatisticMetaService INSTANCE = new
StatisticMetaService();
+
+ public static StatisticMetaService getInstance() {
+ return INSTANCE;
+ }
+
+ private StatisticMetaService() {}
+
+ public List<StatisticEntity> listStatisticsByObject(
+ NameIdentifier identifier, Entity.EntityType type) {
+ long metalakeId =
+ MetalakeMetaService.getInstance()
+ .getMetalakeIdByName(NameIdentifierUtil.getMetalake(identifier));
+ MetadataObject object = NameIdentifierUtil.toMetadataObject(identifier,
type);
+ long objectId =
+ MetadataObjectService.getMetadataObjectId(metalakeId,
object.fullName(), object.type());
+ List<StatisticPO> statisticPOs =
+ SessionUtils.getWithoutCommit(
+ StatisticMetaMapper.class, mapper ->
mapper.listStatisticPOsByObjectId(objectId));
+ return
statisticPOs.stream().map(POConverters::fromStatisticPO).collect(Collectors.toList());
+ }
+
+ public void batchInsertStatisticPOs(
+ List<StatisticEntity> statisticEntities,
+ String metalake,
+ NameIdentifier entity,
+ Entity.EntityType type) {
+ if (statisticEntities == null || statisticEntities.isEmpty()) {
+ return;
+ }
+ Long metalakeId =
MetalakeMetaService.getInstance().getMetalakeIdByName(metalake);
+ Long objectId;
+ MetadataObject object = NameIdentifierUtil.toMetadataObject(entity, type);
+ if (type == Entity.EntityType.METALAKE) {
+ objectId = metalakeId;
Review Comment:
I think this should not be happened, we don't support metalake statistics,
right?
##########
core/src/main/java/org/apache/gravitino/storage/relational/po/StatisticPO.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.po;
+
+import com.google.common.base.Preconditions;
+import java.util.Objects;
+
+public class StatisticPO {
+ private Long metalakeId;
+ private Long statisticId;
+
+ private String statisticName;
+
+ private String statisticValue;
+ private Long metadataObjectId;
+
+ private String metadataObjectType;
+
+ private String auditInfo;
+
+ private Long currentVersion;
+ private Long lastVersion;
+ private Long deletedAt;
+
+ private StatisticPO() {}
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public Long getMetalakeId() {
+ return metalakeId;
+ }
+
+ public Long getStatisticId() {
+ return statisticId;
+ }
+
+ public Long getMetadataObjectId() {
+ return metadataObjectId;
+ }
+
+ public String getMetadataObjectType() {
+ return metadataObjectType;
+ }
+
+ public String getStatisticName() {
+ return statisticName;
+ }
+
+ public String getStatisticValue() {
+ return statisticValue;
+ }
+
+ public String getAuditInfo() {
+ return auditInfo;
+ }
+
+ public Long getCurrentVersion() {
+ return currentVersion;
+ }
+
+ public Long getLastVersion() {
+ return lastVersion;
+ }
+
+ public Long getDeletedAt() {
+ return deletedAt;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof StatisticPO)) {
+ return false;
+ }
+ StatisticPO that = (StatisticPO) o;
+ return statisticId.equals(that.statisticId)
+ && metadataObjectId.equals(that.metadataObjectId)
+ && metalakeId.equals(that.metalakeId)
+ && metadataObjectType.equals(that.metadataObjectType)
+ && statisticName.equals(that.statisticName)
+ && statisticValue.equals(that.statisticValue)
+ && auditInfo.equals(that.auditInfo)
+ && currentVersion.equals(that.currentVersion)
+ && lastVersion.equals(that.lastVersion)
+ && deletedAt.equals(that.deletedAt);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ metalakeId,
+ statisticId,
+ metadataObjectId,
+ metadataObjectType,
+ statisticName,
+ statisticValue,
+ auditInfo,
+ currentVersion,
+ lastVersion,
+ deletedAt);
+ }
+
+ public static class Builder {
+
+ private final StatisticPO statisticPO;
+
+ public Builder() {
+ this.statisticPO = new StatisticPO();
+ }
+
+ public Builder withMetalakeId(Long metalakeId) {
+ statisticPO.metalakeId = metalakeId;
+ return this;
+ }
+
+ public Builder withStatisticId(Long statisticId) {
+ statisticPO.statisticId = statisticId;
+ return this;
+ }
+
+ public Builder withMetadataObjectId(Long objectId) {
+ statisticPO.metadataObjectId = objectId;
+ return this;
+ }
+
+ public Builder withMetadataObjectType(String objectType) {
+ statisticPO.metadataObjectType = objectType;
+ return this;
+ }
+
+ public Builder withStatisticName(String statisticName) {
+ statisticPO.statisticName = statisticName;
+ return this;
+ }
+
+ public Builder withStatisticValue(String value) {
+ statisticPO.statisticValue = value;
+ return this;
+ }
+
+ public Builder withAuditInfo(String auditInfo) {
+ statisticPO.auditInfo = auditInfo;
+ return this;
+ }
+
+ public Builder withCurrentVersion(Long currentVersion) {
+ statisticPO.currentVersion = currentVersion;
+ return this;
+ }
+
+ public Builder withLastVersion(Long lastVersion) {
+ statisticPO.lastVersion = lastVersion;
+ return this;
+ }
+
+ public Builder withDeletedAt(Long deletedAt) {
+ statisticPO.deletedAt = deletedAt;
+ return this;
+ }
+
+ public StatisticPO build() {
+ Preconditions.checkArgument(statisticPO.metadataObjectId != null,
"`objectId is required");
+ Preconditions.checkArgument(
+ statisticPO.metadataObjectType != null, "`objectType` is required");
+ Preconditions.checkArgument(statisticPO.statisticId != null,
"`statisticId` is required");
+ Preconditions.checkArgument(statisticPO.statisticName != null,
"`statisticName` is required");
+ Preconditions.checkArgument(statisticPO.statisticValue != null, "`value`
is required");
+ Preconditions.checkArgument(statisticPO.auditInfo != null, "`auditInfo`
is required");
+ Preconditions.checkArgument(statisticPO.metalakeId != null,
"`metalakeId` is required");
+ Preconditions.checkArgument(statisticPO.deletedAt != null, "`deletedAt`
is required");
+ Preconditions.checkArgument(statisticPO.lastVersion != null,
"`lastVersion` is required");
+ Preconditions.checkArgument(
+ statisticPO.currentVersion != null, "`currentVersion` is required");
+ return statisticPO;
+ }
+ }
+}
Review Comment:
We can use the lombok annotation to simplify the code.
--
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]
