lw-yang commented on code in PR #4109:
URL: https://github.com/apache/gravitino/pull/4109#discussion_r1670402552
##########
core/src/main/java/com/datastrato/gravitino/tag/TagManager.java:
##########
@@ -188,22 +204,141 @@ public boolean deleteTag(String metalake, String name) {
});
}
- public String[] listTagsForMetadataObject(String metalake, MetadataObject
metadataObject) {
- throw new UnsupportedOperationException("Not implemented yet");
+ public MetadataObject[] listMetadataObjectsForTag(String metalake, String
name)
+ throws NoSuchTagException {
+ NameIdentifier tagId = ofTagIdent(metalake, name);
+ return TreeLockUtils.doWithTreeLock(
+ tagId,
+ LockType.READ,
+ () -> {
+ try {
+ if (!entityStore.exists(tagId, Entity.EntityType.TAG)) {
+ throw new NoSuchTagException(
+ "Tag with name %s under metalake %s does not exist", name,
metalake);
+ }
+
+ return
supportsExtraOperations.listAssociatedMetadataObjectsForTag(tagId);
+ } catch (IOException e) {
+ LOG.error("Failed to list metadata objects for tag {}", name, e);
+ throw new RuntimeException(e);
+ }
+ });
}
- public Tag[] listTagsInfoForMetadataObject(String metalake, MetadataObject
metadataObject) {
- throw new UnsupportedOperationException("Not implemented yet");
+ public String[] listTagsForMetadataObject(String metalake, MetadataObject
metadataObject)
+ throws NotFoundException {
+ return Arrays.stream(listTagsInfoForMetadataObject(metalake,
metadataObject))
+ .map(Tag::name)
+ .toArray(String[]::new);
+ }
+
+ public Tag[] listTagsInfoForMetadataObject(String metalake, MetadataObject
metadataObject)
+ throws NotFoundException {
+ NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
+
+ return TreeLockUtils.doWithTreeLock(
+ entityIdent,
+ LockType.READ,
+ () -> {
+ try {
+ return supportsExtraOperations.listAssociatedTagsForMetadataObject(
+ entityIdent, entityType);
+ } catch (NoSuchEntityException e) {
+ throw new NotFoundException(
+ e, "Failed to list tags for metadata object %s due to not
found", metadataObject);
+ } catch (IOException e) {
+ LOG.error("Failed to list tags for metadata object {}",
metadataObject, e);
+ throw new RuntimeException(e);
+ }
+ });
}
public Tag getTagForMetadataObject(String metalake, MetadataObject
metadataObject, String name)
- throws NoSuchTagException {
- throw new UnsupportedOperationException("Not implemented yet");
+ throws NotFoundException {
+ NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
+ NameIdentifier tagIdent = ofTagIdent(metalake, name);
+
+ return TreeLockUtils.doWithTreeLock(
+ entityIdent,
+ LockType.READ,
+ () -> {
+ try {
+ return supportsExtraOperations.getTagForMetadataObject(
+ entityIdent, entityType, tagIdent);
+ } catch (NoSuchEntityException e) {
+ if (e.getMessage().contains("No such tag entity")) {
+ throw new NoSuchTagException(
+ e, "Tag %s does not exist for metadata object %s", name,
metadataObject);
+ } else {
+ throw new NotFoundException(
+ e, "Failed to get tag for metadata object %s due to not
found", metadataObject);
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to get tag for metadata object {}",
metadataObject, e);
+ throw new RuntimeException(e);
+ }
+ });
}
public String[] associateTagsForMetadataObject(
- String metalake, MetadataObject metadataObject, String[] tagsToAdd,
String[] tagsToRemove) {
- throw new UnsupportedOperationException("Not implemented yet");
+ String metalake, MetadataObject metadataObject, String[] tagsToAdd,
String[] tagsToRemove)
+ throws NotFoundException, TagAlreadyAssociatedException {
+ Preconditions.checkArgument(
+ !metadataObject.type().equals(MetadataObject.Type.METALAKE)
+ && !metadataObject.type().equals(MetadataObject.Type.COLUMN),
+ "Cannot associate tags for unsupported metadata object type %s",
+ metadataObject.type());
+
+ NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataObject);
+
+ Set<String> tagsToAddSet = tagsToAdd == null ? Sets.newHashSet() :
Sets.newHashSet(tagsToAdd);
+ if (tagsToRemove != null) {
+ for (String tag : tagsToRemove) {
+ tagsToAddSet.remove(tag);
+ }
Review Comment:
If a tag exists in both `tagsToAdd` and `tagsToRemove`, maybe we should do
nothing with this tag
##########
core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/TagMetadataObjectRelMapper.java:
##########
@@ -18,13 +18,113 @@
*/
package com.datastrato.gravitino.storage.relational.mapper;
+import com.datastrato.gravitino.storage.relational.po.TagMetadataObjectRelPO;
+import com.datastrato.gravitino.storage.relational.po.TagPO;
+import java.util.List;
import org.apache.ibatis.annotations.Delete;
+import org.apache.ibatis.annotations.Insert;
import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.annotations.Select;
import org.apache.ibatis.annotations.Update;
public interface TagMetadataObjectRelMapper {
String TAG_METADATA_OBJECT_RELATION_TABLE_NAME = "tag_relation_meta";
+ @Select(
+ "SELECT tm.tag_id as tagId, tm.tag_name as tagName,"
+ + " tm.metalake_id as metalakeId, tm.tag_comment as comment,
tm.properties as properties,"
+ + " tm.audit_info as auditInfo,"
+ + " tm.current_version as currentVersion,"
+ + " tm.last_version as lastVersion,"
+ + " tm.deleted_at as deletedAt"
+ + " FROM "
+ + TagMetaMapper.TAG_TABLE_NAME
+ + " tm JOIN "
+ + TAG_METADATA_OBJECT_RELATION_TABLE_NAME
+ + " te ON tm.tag_id = te.tag_id"
+ + " WHERE te.metadata_object_id = #{metadataObjectId}"
+ + " AND te.metadata_object_type = #{metadataObjectType} AND
te.deleted_at = 0"
+ + " AND tm.deleted_at = 0")
+ List<TagPO> listTagPOsByMetadataObjectIdAndType(
+ @Param("metadataObjectId") Long metadataObjectId,
+ @Param("metadataObjectType") String metadataObjectType);
+
+ @Select(
+ "SELECT tm.tag_id as tagId, tm.tag_name as tagName,"
+ + " tm.metalake_id as metalakeId, tm.tag_comment as comment,
tm.properties as properties,"
+ + " tm.audit_info as auditInfo,"
+ + " tm.current_version as currentVersion,"
+ + " tm.last_version as lastVersion,"
+ + " tm.deleted_at as deletedAt"
+ + " FROM "
+ + TagMetaMapper.TAG_TABLE_NAME
+ + " tm JOIN "
+ + TAG_METADATA_OBJECT_RELATION_TABLE_NAME
+ + " te ON tm.tag_id = te.tag_id"
+ + " WHERE te.metadata_object_id = #{metadataObjectId}"
+ + " AND te.metadata_object_type = #{metadataObjectType} AND
tm.tag_name = #{tagName}"
+ + " AND te.deleted_at = 0 AND tm.deleted_at = 0")
+ TagPO getTagPOsByMetadataObjectAndTagName(
+ @Param("metadataObjectId") Long metadataObjectId,
+ @Param("metadataObjectType") String metadataObjectType,
+ @Param("tagName") String tagName);
+
+ @Select(
+ "SELECT tmo.tag_id as tagId, tmo.metadata_object_id as metadataObjectId,"
+ + " tmo.metadata_object_type as metadataObjectType, tmo.audit_info
as auditInfo,"
+ + " tmo.current_version as currentVersion, tmo.last_version as
lastVersion,"
+ + " tmo.deleted_at as deletedAt"
+ + " FROM "
+ + TAG_METADATA_OBJECT_RELATION_TABLE_NAME
+ + " tmo JOIN "
Review Comment:
The `tag_relation_meta` table alias should be aligned with other places,
which are `te`
##########
core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/TagMetadataObjectRelMapper.java:
##########
@@ -18,13 +18,113 @@
*/
package com.datastrato.gravitino.storage.relational.mapper;
+import com.datastrato.gravitino.storage.relational.po.TagMetadataObjectRelPO;
+import com.datastrato.gravitino.storage.relational.po.TagPO;
+import java.util.List;
import org.apache.ibatis.annotations.Delete;
+import org.apache.ibatis.annotations.Insert;
import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.annotations.Select;
import org.apache.ibatis.annotations.Update;
public interface TagMetadataObjectRelMapper {
String TAG_METADATA_OBJECT_RELATION_TABLE_NAME = "tag_relation_meta";
+ @Select(
+ "SELECT tm.tag_id as tagId, tm.tag_name as tagName,"
+ + " tm.metalake_id as metalakeId, tm.tag_comment as comment,
tm.properties as properties,"
+ + " tm.audit_info as auditInfo,"
+ + " tm.current_version as currentVersion,"
+ + " tm.last_version as lastVersion,"
+ + " tm.deleted_at as deletedAt"
+ + " FROM "
+ + TagMetaMapper.TAG_TABLE_NAME
+ + " tm JOIN "
+ + TAG_METADATA_OBJECT_RELATION_TABLE_NAME
+ + " te ON tm.tag_id = te.tag_id"
+ + " WHERE te.metadata_object_id = #{metadataObjectId}"
+ + " AND te.metadata_object_type = #{metadataObjectType} AND
te.deleted_at = 0"
+ + " AND tm.deleted_at = 0")
+ List<TagPO> listTagPOsByMetadataObjectIdAndType(
+ @Param("metadataObjectId") Long metadataObjectId,
+ @Param("metadataObjectType") String metadataObjectType);
Review Comment:
I found another issue. The unique key `uk_ti_mi_del` in the
`tag_relation_meta` table should include the `metadata_object_type` field.
[`UNIQUE KEY `uk_ti_mi_del` (`tag_id`, `metadata_object_id`,
`deleted_at`)`](https://github.com/apache/gravitino/blob/9e80cbcf9ea8de21eabec9fdb3651d5b376c7b26/scripts/mysql/upgrade-0.5.0-to-0.6.0-mysql.sql#L63)
##########
core/src/main/java/com/datastrato/gravitino/utils/MetadataObjectUtil.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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 com.datastrato.gravitino.utils;
+
+import com.datastrato.gravitino.Entity;
+import com.datastrato.gravitino.MetadataObject;
+import com.datastrato.gravitino.NameIdentifier;
+import com.google.common.base.Joiner;
+
+public class MetadataObjectUtil {
Review Comment:
The name is similar with
`com.datastrato.gravitino.storage.relational.utils.MetadataObjectUtils`
maybe we should merge them or give a more appropriate name
--
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]
