roryqi commented on code in PR #12354:
URL: https://github.com/apache/gravitino/pull/12354#discussion_r3711987584
##########
scripts/h2/schema-2.0.0-h2.sql:
##########
@@ -308,14 +309,15 @@ CREATE TABLE IF NOT EXISTS `tag_relation_meta` (
`tag_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'tag id',
`metadata_object_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metadata object
id',
`metadata_object_type` VARCHAR(64) NOT NULL COMMENT 'metadata object type',
+ `tag_value` VARCHAR(256) DEFAULT NULL COMMENT 'tag relation value',
Review Comment:
Fixed in 25b6f3fd2. Added 1.3.0 -> 2.0.0 upgrade changes for MySQL, H2, and
PostgreSQL: add `tag_meta.allowed_values`, add `tag_relation_meta.tag_value`,
add the `(tag_id, tag_value)` index, and drop the old tag-object unique
constraint.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java:
##########
@@ -394,6 +514,99 @@ public int deleteTagMetasByLegacyTimeline(long
legacyTimeline, int limit) {
return tagDeletedCount[0] + tagMetadataObjectRelDeletedCount[0];
}
+ private static List<TagEntity> tagPOsToTagEntities(List<TagPO> tagPOs,
Namespace namespace) {
+ Map<Long, List<TagPO>> tagPOsByTagId = new LinkedHashMap<>();
+ for (TagPO tagPO : tagPOs) {
+ tagPOsByTagId.computeIfAbsent(tagPO.getTagId(), ignored -> new
ArrayList<>()).add(tagPO);
+ }
+
+ return tagPOsByTagId.values().stream()
+ .map(tagPOGroup -> tagPOsToTagEntity(tagPOGroup, namespace))
+ .collect(Collectors.toList());
+ }
+
+ private static TagEntity tagPOsToTagEntity(List<TagPO> tagPOGroup, Namespace
namespace) {
+ TagPO firstTagPO = tagPOGroup.get(0);
+ List<String> assignmentValues =
+ tagPOGroup.stream()
+ .map(TagPO::getAssignmentValue)
+ .filter(Objects::nonNull)
+ .distinct()
+ .collect(Collectors.toList());
+
+ TagAssignment assignment =
+ assignmentValues.isEmpty()
+ ? TagAssignment.noValue()
+ : TagAssignment.ofValues(assignmentValues.toArray(new String[0]));
+ return POConverters.fromTagPO(firstTagPO,
namespace).copyWithAssignment(assignment);
+ }
+
+ private static TagValue[] toValuelessTagValues(NameIdentifier[] tags) {
+ if (tags == null) {
+ return null;
+ }
+
+ return Arrays.stream(tags).map(tag ->
TagValue.noValue(tag.name())).toArray(TagValue[]::new);
+ }
+
+ private static TagValue[] nullToEmpty(TagValue[] tagValues) {
+ return tagValues == null ? new TagValue[0] : tagValues;
+ }
+
+ private static List<String> tagNamesToUpdate(
+ List<TagValue> tagsToAdd, List<TagValue> tagsToRemove) {
+ Set<String> tagNames = new LinkedHashSet<>();
+ tagsToAdd.stream().map(TagValue::name).forEach(tagNames::add);
+ tagsToRemove.stream().map(TagValue::name).forEach(tagNames::add);
+ return new ArrayList<>(tagNames);
+ }
+
+ private static Map<String, TagPO> tagPOsByName(List<TagPO> tagPOs) {
+ Map<String, TagPO> tagPOsByName = new LinkedHashMap<>();
+ for (TagPO tagPO : tagPOs) {
+ tagPOsByName.put(tagPO.getTagName(), tagPO);
+ }
+ return tagPOsByName;
+ }
+
+ private static void trackExistingAssignment(
+ TagPO tagPO, Map<Long, Set<Optional<String>>> activeValuesByTagId) {
+ activeValuesByTagId
+ .computeIfAbsent(tagPO.getTagId(), ignored -> new LinkedHashSet<>())
+ .add(Optional.ofNullable(tagPO.getAssignmentValue()));
+ }
+
+ private static TagMetadataObjectRelPO tagRelForValue(
+ TagPO tagPO, Long metadataObjectId, MetadataObject metadataObject,
TagValue tagValue) {
+ return POConverters.initializeTagMetadataObjectRelPOWithVersion(
+ tagPO.getTagId(),
+ metadataObjectId,
+ metadataObject.type().toString(),
+ tagValue.value().orElse(null));
+ }
+
+ private static void validateAllowedValue(TagPO tagPO, TagValue tagValue)
+ throws JsonProcessingException {
+ if (tagPO.getAllowedValues() == null) {
+ return;
+ }
+
+ String[] allowedValues =
+ JsonUtils.anyFieldMapper().readValue(tagPO.getAllowedValues(),
String[].class);
+ if (!tagValue.value().isPresent()) {
Review Comment:
Fixed in 25b6f3fd2. `TagValue.noValue("stage")` is now rejected when the tag
has a non-empty allowed-values constraint, and I added a regression test for
that case in `TestTagMetaService`.
##########
scripts/mysql/schema-2.0.0-mysql.sql:
##########
@@ -286,6 +286,7 @@ CREATE TABLE IF NOT EXISTS `tag_meta` (
`metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
`tag_comment` VARCHAR(256) DEFAULT '' COMMENT 'tag comment',
`properties` MEDIUMTEXT DEFAULT NULL COMMENT 'tag properties',
+ `allowed_values` MEDIUMTEXT DEFAULT NULL COMMENT 'tag allowed values',
Review Comment:
Fixed in 25b6f3fd2. The schema comments now state that `allowed_values` is
stored as a JSON string array. Storage encoding is: `NULL` => `ANY_VALUE`, `[]`
=> `NO_VALUE`, and `["a","b"]` => allowed values.
##########
scripts/mysql/schema-2.0.0-mysql.sql:
##########
@@ -299,14 +300,15 @@ CREATE TABLE IF NOT EXISTS `tag_relation_meta` (
`tag_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'tag id',
`metadata_object_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metadata object
id',
`metadata_object_type` VARCHAR(64) NOT NULL COMMENT 'metadata object type',
+ `tag_value` VARCHAR(256) DEFAULT NULL COMMENT 'tag relation value',
`audit_info` MEDIUMTEXT NOT NULL COMMENT 'tag relation audit info',
`current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'tag relation
current version',
`last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'tag relation last
version',
`deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'tag relation
deleted at',
PRIMARY KEY (`id`),
- UNIQUE KEY `uk_ti_mi_mo_del` (`tag_id`, `metadata_object_id`,
`metadata_object_type`, `deleted_at`),
Review Comment:
The old unique key was on `(tag_id, metadata_object_id,
metadata_object_type, deleted_at)`, so it would reject multiple active rows for
the same tag-object assignment when different `tag_value`s are assigned. The
service layer already de-duplicates requested active values, and the table
keeps the primary key plus the lookup indexes.
##########
core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java:
##########
@@ -1465,8 +1472,34 @@ public static TagPO updateTagPOWithVersion(TagPO
oldTagPO, TagEntity newEntity)
}
}
+ private static String serializeAllowedValues(TagValueConstraint
valueConstraint)
+ throws JsonProcessingException {
+ String[] allowedValues = allowedValuesForStorage(valueConstraint);
+ return allowedValues == null
+ ? null
+ : JsonUtils.anyFieldMapper().writeValueAsString(allowedValues);
+ }
+
+ private static String[] allowedValuesForStorage(TagValueConstraint
valueConstraint) {
+ switch (valueConstraint.type()) {
+ case ANY_VALUE:
+ return null;
+ case NO_VALUE:
Review Comment:
The difference is in the storage encoding and validation semantics:
`ANY_VALUE` is stored as `NULL` and accepts valueless or any non-empty valued
assignment; `NO_VALUE` is stored as an empty JSON array and only accepts
valueless assignments. I added a code comment in `POConverters` to make this
explicit.
--
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]
