jerryshao commented on code in PR #14281: URL: https://github.com/apache/gravitino/pull/14281#discussion_r4412707458
########## core/src/main/java/org/apache/gravitino/storage/relational/po/DomainPO.java: ########## @@ -0,0 +1,200 @@ +/* + * 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 static org.apache.gravitino.storage.relational.utils.POConverters.DEFAULT_DELETED_AT; +import static org.apache.gravitino.storage.relational.utils.POConverters.INIT_VERSION; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.google.common.base.Preconditions; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import lombok.experimental.Accessors; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.domain.Domain; +import org.apache.gravitino.json.JsonUtils; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.DomainEntity; + +/** + * The persistent object of a row in {@code domain_meta}. The backing tag name is not a column of + * the row: reads that join the live tag fill it, and it is null when the domain has no backing tag + * or the tag is deleted. + * + * <p>A tag bound without a value has a NULL {@code backing_tag_value}, while {@code + * tag_relation_meta.tag_value} stores the empty string for an assignment without a value. A lookup + * from tag assignments to the domains they back must map the empty string to NULL instead of + * comparing the two columns directly. + */ +@Getter +@Accessors(fluent = true) +@EqualsAndHashCode +@ToString +public class DomainPO { + + private Long domainId; + private String domainName; + private Long metalakeId; + private String comment; + private String status; + private Long backingTagId; + private String backingTagName; + private String backingTagValue; + private String properties; + private String auditInfo; + private Long currentVersion; + private Long lastVersion; + private Long deletedAt; + + /** Creates an empty domain persistent object, used by MyBatis to map result rows. */ + public DomainPO() {} + + @lombok.Builder(setterPrefix = "with") + private DomainPO( + Long domainId, + String domainName, + Long metalakeId, + String comment, + String status, + Long backingTagId, + String backingTagValue, + String properties, + String auditInfo, + Long currentVersion, + Long lastVersion, + Long deletedAt) { + Preconditions.checkArgument(domainId != null, "domainId cannot be null"); + Preconditions.checkArgument(StringUtils.isNotBlank(domainName), "domainName cannot be blank"); + Preconditions.checkArgument(metalakeId != null, "metalakeId cannot be null"); + Preconditions.checkArgument(StringUtils.isNotBlank(status), "status cannot be blank"); + Preconditions.checkArgument( + backingTagId != null || backingTagValue == null, + "backingTagValue cannot be set without backingTagId"); + Preconditions.checkArgument(StringUtils.isNotBlank(auditInfo), "auditInfo cannot be blank"); + Preconditions.checkArgument(currentVersion != null, "currentVersion cannot be null"); + Preconditions.checkArgument(lastVersion != null, "lastVersion cannot be null"); + Preconditions.checkArgument(deletedAt != null, "deletedAt cannot be null"); + + this.domainId = domainId; + this.domainName = domainName; + this.metalakeId = metalakeId; + this.comment = comment; + this.status = status; + this.backingTagId = backingTagId; + this.backingTagValue = backingTagValue; + this.properties = properties; + this.auditInfo = auditInfo; + this.currentVersion = currentVersion; + this.lastVersion = lastVersion; + this.deletedAt = deletedAt; + } + + /** + * Creates the first version of the persistent object of a new domain. + * + * @param domainEntity The domain entity. + * @param metalakeId The ID of the metalake of the domain. + * @return The domain persistent object. + */ + public static DomainPO initializeDomainPO(DomainEntity domainEntity, Long metalakeId) { + return toDomainPO(domainEntity, domainEntity.id(), metalakeId, INIT_VERSION); + } + + /** + * Creates the next version of the persistent object of a domain from its updated entity. + * + * @param oldDomainPO The domain persistent object observed by the caller. + * @param newDomainEntity The updated domain entity. + * @return The domain persistent object with the next version. + */ + public static DomainPO updateDomainPO(DomainPO oldDomainPO, DomainEntity newDomainEntity) { + Long nextVersion = oldDomainPO.currentVersion() + 1; + return toDomainPO( + newDomainEntity, oldDomainPO.domainId(), oldDomainPO.metalakeId(), nextVersion); + } + + /** + * Converts a domain persistent object to a domain entity. The parent is not stored in the domain + * row, so it is not set. The backing tag name is set only when the read joined the live tag. + * + * @param domainPO The domain persistent object. + * @param namespace The namespace of the domain. + * @return The domain entity. + */ + @SuppressWarnings("unchecked") + public static DomainEntity fromDomainPO(DomainPO domainPO, Namespace namespace) { + try { + return DomainEntity.builder() + .withId(domainPO.domainId()) + .withName(domainPO.domainName()) + .withNamespace(namespace) + .withComment(domainPO.comment()) + .withStatus(Domain.Status.valueOf(domainPO.status())) + .withBackingTagId(domainPO.backingTagId()) + .withBackingTagName(domainPO.backingTagName()) + .withBackingTagValue(domainPO.backingTagValue()) + .withProperties(JsonUtils.anyFieldMapper().readValue(domainPO.properties(), Map.class)) + .withAuditInfo( + JsonUtils.anyFieldMapper().readValue(domainPO.auditInfo(), AuditInfo.class)) + .build(); + } catch (JsonProcessingException e) { + throw new RuntimeException("Failed to deserialize domain PO", e); + } + } Review Comment: The write path always serializes the map with Jackson, so a null map is stored as the JSON text `null`, never as SQL NULL. The read matches how `POConverters` reads tag properties. No change. ########## core/src/main/java/org/apache/gravitino/utils/NamespaceUtil.java: ########## @@ -424,6 +434,19 @@ public static void checkJobTemplate(Namespace namespace) { namespace); } + /** + * Check if the given domain namespace is legal, throw an {@link IllegalNamespaceException} if + * it's illegal. + * + * @param namespace The domain namespace + */ + public static void checkDomain(Namespace namespace) { + check( + namespace != null && namespace.length() == 3, + "Domain namespace must be non-null and have 3 levels, the input namespace is %s", Review Comment: This matches the other namespace checks in this class, such as `checkJobTemplate`, which check the number of levels only. I'd keep the domain check consistent with them. -- 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]
