Copilot commented on code in PR #2938:
URL:
https://github.com/apache/incubator-hugegraph/pull/2938#discussion_r2692992957
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/builder/VertexLabelBuilder.java:
##########
@@ -546,7 +533,35 @@ private void checkStableVars() {
}
}
- private void checkTtl() {
+ /**
+ * Update TTL in two cases:
+ * 1) ttl > 0L: set or change a positive TTL
+ * 2) ttl == 0L and existing ttl > 0L: explicitly clear an existing TTL
+ * This allows removing TTL from a label that previously had TTL
configured.
+ * Note: ttl == null means not set, so we skip the update.
+ */
+ private void updateTTL(VertexLabel vertexLabel) {
+ if (this.ttl == null) {
+ return;
+ }
+ if (this.ttl > 0L) {
+ vertexLabel.ttl(this.ttl);
+ if (this.ttlStartTime != null) {
+
vertexLabel.ttlStartTime(this.graph().propertyKey(this.ttlStartTime).id());
+ }
+ } else if (this.ttl == 0L && vertexLabel.ttl() > 0L) {
+ // Clear TTL and ttlStartTime
+ vertexLabel.ttl(0L);
+ vertexLabel.ttlStartTime(IdGenerator.ZERO);
+ }
+ }
Review Comment:
The `updateTTL` method does not handle the case where a user wants to remove
`ttlStartTime` while keeping a positive TTL. When `this.ttl > 0L` and
`this.ttlStartTime == null`, the existing `ttlStartTime` on the label is
preserved. Consider adding an else clause at line 551 to explicitly clear
`ttlStartTime` when `this.ttlStartTime == null` and `this.ttl > 0L`, by calling
`vertexLabel.ttlStartTime(IdGenerator.ZERO)`. This would allow users to remove
ttlStartTime while maintaining TTL.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/builder/EdgeLabelBuilder.java:
##########
@@ -670,7 +667,35 @@ private void checkStableVars() {
}
}
- private void checkTtl() {
+ /**
+ * Update TTL in two cases:
+ * 1) ttl > 0L: set or change a positive TTL
+ * 2) ttl == 0L and existing ttl > 0L: explicitly clear an existing TTL
+ * This allows removing TTL from a label that previously had TTL
configured.
+ * Note: ttl == null means not set, so we skip the update.
+ */
+ private void updateTTL(EdgeLabel edgeLabel) {
+ if (this.ttl == null) {
+ return;
+ }
+ if (this.ttl > 0L) {
+ edgeLabel.ttl(this.ttl);
+ if (this.ttlStartTime != null) {
+
edgeLabel.ttlStartTime(this.graph().propertyKey(this.ttlStartTime).id());
+ }
+ } else if (this.ttl == 0L && edgeLabel.ttl() > 0L) {
+ // Clear TTL and ttlStartTime
+ edgeLabel.ttl(0L);
+ edgeLabel.ttlStartTime(IdGenerator.ZERO);
+ }
+ }
Review Comment:
The `updateTTL` method does not handle the case where a user wants to remove
`ttlStartTime` while keeping a positive TTL. When `this.ttl > 0L` and
`this.ttlStartTime == null`, the existing `ttlStartTime` on the label is
preserved. Consider adding an else clause at line 685 to explicitly clear
`ttlStartTime` when `this.ttlStartTime == null` and `this.ttl > 0L`, by calling
`edgeLabel.ttlStartTime(IdGenerator.ZERO)`. This would allow users to remove
ttlStartTime while maintaining TTL.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/builder/VertexLabelBuilder.java:
##########
@@ -454,22 +446,18 @@ private void checkNullableKeys(Action action) {
"must belong to the origin/new properties: %s/%s",
this.nullableKeys, originProps, appendProps);
- List<String> primaryKeys = vertexLabel == null ?
- this.primaryKeys :
- this.graph()
-
.mapPkId2Name(vertexLabel.primaryKeys());
+ List<String> primaryKeys = vertexLabel == null ? this.primaryKeys :
+
this.graph().mapPkId2Name(vertexLabel.primaryKeys());
Review Comment:
Potentially confusing name: method [checkNullableKeys](1) also refers to
field [primaryKeys](2) (as this.primaryKeys).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
