[
https://issues.apache.org/jira/browse/TINKERPOP-3261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18094109#comment-18094109
]
ASF GitHub Bot commented on TINKERPOP-3261:
-------------------------------------------
Cole-Greer commented on code in PR #3483:
URL: https://github.com/apache/tinkerpop/pull/3483#discussion_r3521014886
##########
docs/src/dev/provider/gremlin-semantics.asciidoc:
##########
@@ -2564,6 +2688,13 @@ the possible values are:
** 8 for including values (VertexProperty)
** 15 for including all
+The label format in the map is controlled by source-level `with("multilabel")`
and `with("singlelabel")` options, not
+by the graph's `LabelCardinality`. When `"multilabel"` is present and
`"singlelabel"` is not, the label value is a
+`Set<String>`; otherwise it is a single `String`. As a deliberate design
choice, when both options are present on the
+same source, `"singlelabel"` always takes precedence regardless of the order
in which they were applied. This ensures
+deterministic behavior rather than relying on declaration order. Providers who
want label output tied to their
+cardinality configuration should override `PropertyMapStep` and/or
`ElementMapStep`.
Review Comment:
We need an equivalent entry for `elementMap()`
##########
docs/src/dev/provider/gremlin-semantics.asciidoc:
##########
@@ -1637,6 +1715,52 @@ See:
link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/j
link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupCountSideEffectStep.java[source
(sideEffect)],
link:https://tinkerpop.apache.org/docs/x.y.z/reference/#groupcount-step[reference]
+[[label-step]]
+=== label()
+
+*Description:* Maps an element to its label as a single string value.
+
+*Syntax:* `label()`
+
+[width="100%",options="header"]
+|=========================================================
+|Start Step |Mid Step |Modulated |Domain |Range
+|N |Y |N |`Element` |`String`
+|=========================================================
+
+*Considerations:*
+
+The `label()` step returns exactly one label string per element. For graphs
that support multiple labels per vertex,
+`label()` returns only one of the vertex's labels and the choice is
non-deterministic (implementation-dependent
+ordering). Use `labels()` to retrieve all labels reliably.
+
+See:
link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/LabelStep.java[source],
+link:https://tinkerpop.apache.org/docs/x.y.z/reference/#label-step[reference]
+
+[[labels-step]]
+=== labels()
+
+*Description:* Maps an element to its labels, emitting one traverser per label.
+
+*Syntax:* `labels()`
+
+[width="100%",options="header"]
+|=========================================================
+|Start Step |Mid Step |Modulated |Domain |Range
+|N |Y |N |`Element` |`String`
+|=========================================================
+
+*Considerations:*
+
+For vertices with a single label, `labels()` emits one traverser (equivalent
to `label()`). For multi-label vertices,
+it emits one traverser per label. For vertices with zero labels
(`ZERO_OR_MORE` mode), no traversers are emitted.
+
+`hasLabel("a", "b")` uses OR semantics: it matches any vertex that has label
"a" or label "b". To match vertices
Review Comment:
This note on `hasLabel()` doesn't belong here, it belongs in it's own
section for `hasLabel()`.
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java:
##########
@@ -78,6 +81,22 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
}
+ // Reject labels().drop() patterns — users should use dropLabel(label)
or dropLabels() instead
+ for (final DropStep<?> dropStep :
TraversalHelper.getStepsOfClass(DropStep.class, traversal)) {
Review Comment:
Nit:
```suggestion
for (final DropStep<?> dropStep :
TraversalHelper.getStepsOfAssignableClass(DropStep.class, traversal)) {
```
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/WithOptions.java:
##########
@@ -101,4 +103,30 @@ public class WithOptions {
* @since 4.0.0
*/
public static final String queryLanguage = "queryLanguage";
+
+ // Multi-label configuration
+ //
+
+ /**
+ * The user-facing key for multilabel configuration used with {@code
g.with("multilabel")}.
+ */
+ public static final String MULTILABEL_KEY = "multilabel";
+
+ /**
+ * The user-facing key for singlelabel override used with {@code
g.with("singlelabel")}.
+ * When present, overrides multilabel to force single-label output in
valueMap/elementMap steps.
+ */
+ public static final String SINGLELABEL_KEY = "singlelabel";
+
+ /**
+ * Checks whether multi-label output is enabled for the given traversal
via source-level
+ * {@code g.with("multilabel")} configuration. Returns {@code false} if
{@code g.with("singlelabel")}
+ * is also present, as singlelabel overrides multilabel.
+ */
+ public static boolean isMultilabelEnabled(final Traversal.Admin<?, ?>
traversal) {
Review Comment:
Nit: can we potentially relocate this method elsewhere? I'm not sure this
class is the best fit for it.
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AddVertex.feature:
##########
@@ -703,4 +703,71 @@ Feature: Step - addV()
When iterated to list
Then the result should be unordered
| result |
- | d[2010].i |
\ No newline at end of file
+ | d[2010].i |
+
+ @MultiLabel
+ Scenario: g_addVXa_bX_labels
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV("a", "b").labels().fold()
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of
"g.V().hasLabel(\"a\").hasLabel(\"b\")"
+
+ @MultiLabel
+ Scenario: g_addVXa_b_cX_labels_count
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV("a", "b", "c").labels().count()
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | d[3].l |
+
+ @MultiLabel
+ Scenario: g_addVXconstantXaX_constantXbXX_labels
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV(__.constant("a"), __.constant("b")).labels().fold()
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of
"g.V().hasLabel(\"a\").hasLabel(\"b\")"
+
+ @MultiLabel
+ Scenario: g_addVXconstantXaX_constantXbX_constantXcXX_labels_count
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV(__.constant("a"), __.constant("b"),
__.constant("c")).labels().count()
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | d[3].l |
+
+ @MultiLabel
+ Scenario: g_addVXconstantX_a_b_XX_labels
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV(__.constant(["a", "b"])).labels().fold()
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of
"g.V().hasLabel(\"a\").hasLabel(\"b\")"
+
+ @MultiLabel
+ Scenario: g_addVXconstantXa_bX_fold_constantXcXX_label_error
+ Given the empty graph
+ And the traversal of
+ """
+ g.addV(__.inject("a", "b").fold(), __.constant("c")).labels().fold()
Review Comment:
This test should use constant() instead of inject()
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/event/EventUtil.java:
##########
@@ -202,6 +204,24 @@ public static void registerPropertyChange(final
CallbackRegistry<Event.ElementPr
}
}
+ /**
+ * Register a label change event with the callback registry.
+ */
+ @SuppressWarnings("unchecked")
Review Comment:
Nit: can we avoid use of `@SuppressWarnings("unchecked")` in our public
methods?
##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java:
##########
@@ -63,7 +64,19 @@ public static Iterable<Object[]> data() throws Exception {
{repeat(out().values("age").inject(10)).times(2), false},
{repeat(out().choose(constant(true), inject(1),
inject(2))).times(5), false},
{__.V().profile(), true},
- {__.V().profile("metrics").cap("metrics"), true}
+ {__.V().profile("metrics").cap("metrics"), true},
+
+ // labels().drop() patterns should REJECT
Review Comment:
We should have similar tests for `label().drop()` patterns, which should
also REJECT
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature:
##########
@@ -990,3 +990,144 @@ Feature: Step - mergeV()
Then the result should have a count of 1
And the graph should return 1 for count of
"g.V().has(\"person\",\"name\",\"stephen\").hasNot(\"created\")"
And the graph should return 2 for count of "g.V()"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_ab_name_markoX_multilabel_create
+ Given the empty graph
+ And the traversal of
+ """
+ g.mergeV([(T.label): ["person","employee"], name: "marko"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"person\").hasLabel(\"employee\")"
+ And the graph should return 1 for count of "g.V().has(\"name\",\"marko\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_abc_name_testX_multilabel_create
+ Given the empty graph
+ And the traversal of
+ """
+ g.mergeV([(T.label): ["a","b","c"], name: "test"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"a\").hasLabel(\"b\").hasLabel(\"c\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_ab_name_markoX_multilabel_match
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").addLabel("employee").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): ["person","employee"], name: "marko"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"person\").hasLabel(\"employee\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_ab_name_markoX_multilabel_nomatch
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): ["person","employee"], name: "marko"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 2 for count of "g.V()"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_person_name_markoX_optionXonMatch_label_managerX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").addLabel("employee").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): "person", name: "marko"]).
+ option(Merge.onMatch, [(T.label): "manager"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of "g.V().hasLabel(\"manager\")"
+ And the graph should return 1 for count of "g.V().hasLabel(\"person\")"
+ And the graph should return 1 for count of "g.V().hasLabel(\"employee\")"
+
+ @MultiLabel
+ Scenario:
g_mergeVXlabel_person_name_markoX_optionXonMatch_label_manager_directorX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): "person", name: "marko"]).
+ option(Merge.onMatch, [(T.label): ["manager","director"]])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"manager\").hasLabel(\"director\")"
+ And the graph should return 1 for count of "g.V().hasLabel(\"person\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_person_name_markoX_optionXonMatch_label_emptyX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): "person", name: "marko"]).
+ option(Merge.onMatch, [(T.label): []])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of "g.V().hasLabel(\"person\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXname_markoX_optionXonCreate_label_ab_name_markoX
+ Given the empty graph
+ And the traversal of
+ """
+ g.mergeV([name: "marko"]).
+ option(Merge.onCreate, [(T.label): ["person","employee"], name:
"marko"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"person\").hasLabel(\"employee\")"
+ And the graph should return 1 for count of "g.V().has(\"name\",\"marko\")"
+
+ @MultiLabel
+ Scenario: g_mergeVXlabel_person_name_markoX_optionXonMatch_label_existingX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").addLabel("employee").property("name", "marko")
+ """
+ And the traversal of
+ """
+ g.mergeV([(T.label): "person", name: "marko"]).
+ option(Merge.onMatch, [(T.label): "person"])
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 1 for count of "g.V()"
+ And the graph should return 1 for count of
"g.V().hasLabel(\"person\").hasLabel(\"employee\")"
Review Comment:
I think we need additional scenarios for mergeV for single-labelled graphs
(no @MultiLabel tag), which validates the expected exceptions if multiple
labels are attempted to be added to a single-labelled vertex.
##########
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/World.java:
##########
@@ -55,6 +55,27 @@ public interface World {
*/
public GraphTraversalSource getGraphTraversalSource(final GraphData
graphData);
+ /**
+ * Gets a {@link GraphTraversalSource} configured for multi-label support
(ZERO_OR_MORE vertex label cardinality).
+ * This source is used by {@code @MultiLabel} tagged scenarios that
require label mutation operations such as
+ * {@code addLabel()} and {@code dropLabel()}. The default implementation
delegates to
+ * {@link #getGraphTraversalSource(GraphData)} with {@code null} which
works for embedded graphs that already
+ * have multi-label cardinality configured. Remote implementations should
override this to connect to a
+ * dedicated multi-label traversal source.
+ */
+ public default GraphTraversalSource getMultiLabelGraphTraversalSource() {
+ return getGraphTraversalSource(null);
+ }
+
+ /**
+ * Gets a {@link GraphTraversalSource} that returns labels as a set by
default from
+ * {@code elementMap()}/{@code valueMap()}. Used by {@code
@MultiLabelDefault} tagged scenarios to simulate
+ * a provider whose default label output is a set. Applies {@code
with("multilabel")} to the multi-label source.
+ */
+ public default GraphTraversalSource
getMultiLabelDefaultGraphTraversalSource() {
Review Comment:
This reflects the wrong concept for the meaning of the `@MultiLabelDefault`
tagged scenarios. These scenarios are for graphs which default to returning
elementMap and valueMap's with labels as sets by default, in the absence of an
explicit with(multilabel) or with(singlelabel) config. The test framework
should not be secretly appending such a config to the traversal source.
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java:
##########
@@ -78,6 +81,22 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
}
+ // Reject labels().drop() patterns — users should use dropLabel(label)
or dropLabels() instead
+ for (final DropStep<?> dropStep :
TraversalHelper.getStepsOfClass(DropStep.class, traversal)) {
+ Step<?, ?> current = dropStep.getPreviousStep();
+
+ // Walk backward through filter steps (IsStep, HasStep, WhereStep,
NotStep, etc.)
+ while (current instanceof FilterStep) {
+ current = current.getPreviousStep();
+ }
+
+ if (current instanceof LabelsStep) {
Review Comment:
we should also block `label().drop()` patterns.
```suggestion
if (current instanceof LabelsStep || current instanceof
LabelStep) {
```
> Enable multiple label support on vertex with configurable label cardinality
> ---------------------------------------------------------------------------
>
> Key: TINKERPOP-3261
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3261
> Project: TinkerPop
> Issue Type: Task
> Affects Versions: 4.0.0
> Reporter: Yang Xia
> Priority: Major
>
> Vertices are currently limited to a single immutable label assigned at
> creation. This prevents modeling common real-world scenarios where entities
> naturally belong to multiple categories (e.g., a person who is both an
> employee and a manager).
>
> Introduce a configurable LabelCardinality that controls how many labels a
> vertex may have and whether they can be mutated after creation. Three
> proposed modes: ONE (current behavior, default), ONE_OR_MORE (mutable,
> minimum one), ZERO_OR_MORE (fully flexible).
>
> New steps:
>
> - labels() — flatMap step emitting each label as a traverser
> - addLabel(String, String...) — add labels to a vertex
> - dropLabel(String, String...) — remove specific labels
> - dropLabels() — remove all labels
> Edge labels remain at cardinality ONE. The infrastructure would support
> future edge multi-label enablement without wire format changes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
