imbajin commented on code in PR #3178:
URL: https://github.com/apache/hugegraph/pull/3178#discussion_r3889917802
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugePrimaryKeyStrategy.java:
##########
@@ -76,26 +77,33 @@ public void apply(Traversal.Admin<?, ?> traversal) {
|| propertyStep.getCardinality() == null) {
Object[] kvs = new Object[2];
- List<Object> kvList = new LinkedList<>();
-
- propertyStep.getParameters().getRaw().forEach((k, v) -> {
- if (T.key.equals(k)) {
- kvs[0] = v.get(0);
- } else if (T.value.equals(k)) {
- kvs[1] = v.get(0);
+ boolean extraParams = false;
+
+ for (Map.Entry<Object, List<Object>> param :
+ propertyStep.getParameters().getRaw().entrySet()) {
+ Object paramKey = param.getKey();
+ if (T.key.equals(paramKey)) {
+ kvs[0] = param.getValue().get(0);
+ } else if (T.value.equals(paramKey)) {
+ kvs[1] = param.getValue().get(0);
} else {
- kvList.add(k.toString());
- kvList.add(v.get(0));
+ extraParams = true;
}
- });
+ }
curAddStep.configure(kvs);
Review Comment:
⚠️ This still gives the failing `AddPropertyStep` a side effect before it
throws. On the exact head, after `addV(...).property("country", "cn", "since",
"2024").next()` throws, catching the exception and committing the transaction
persists `country=cn` because it was copied into `addV()` here. A non-folded
`AddPropertyStep` rejects before setting that property. Please ensure a
rejected step cannot commit its own key/value, and add a catch-and-commit
regression; the explicit rollback in the new test currently masks this
difference.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugePrimaryKeyStrategy.java:
##########
@@ -76,26 +77,33 @@ public void apply(Traversal.Admin<?, ?> traversal) {
|| propertyStep.getCardinality() == null) {
Object[] kvs = new Object[2];
- List<Object> kvList = new LinkedList<>();
-
- propertyStep.getParameters().getRaw().forEach((k, v) -> {
- if (T.key.equals(k)) {
- kvs[0] = v.get(0);
- } else if (T.value.equals(k)) {
- kvs[1] = v.get(0);
+ boolean extraParams = false;
+
+ for (Map.Entry<Object, List<Object>> param :
+ propertyStep.getParameters().getRaw().entrySet()) {
+ Object paramKey = param.getKey();
+ if (T.key.equals(paramKey)) {
+ kvs[0] = param.getValue().get(0);
Review Comment:
⚠️ Traversal-valued parameters are now owned by two steps: the same child
traversal remains on the retained `AddPropertyStep` and is also configured into
`AddVertexStep`. With `val = __.addV("audit").constant("marko")`,
`addV("p").property("name", val, "since", "2024")` creates two `audit` vertices
before raising the metadata error on the exact head. Please leave
traversal-valued pairs unfolded, or otherwise ensure each child traversal is
attached and evaluated exactly once.
--
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]