This is an automated email from the ASF dual-hosted git repository.
xiazcy pushed a commit to branch 3.8-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.8-dev by this push:
new f422d780ca TINKERPOP-3150 Prevent sample step from having multiple by
modulators… (#3114)
f422d780ca is described below
commit f422d780ca46b37c48d4dae0fbd25d4dbc24f8d2
Author: Peter Tribe <peter.tr...@improving.com>
AuthorDate: Mon May 12 17:34:02 2025 -0600
TINKERPOP-3150 Prevent sample step from having multiple by modulators…
(#3114)
* TINKERPOP-3150 Prevent sample step from having multiple by modulators
---
CHANGELOG.asciidoc | 1 +
.../process/traversal/step/filter/SampleGlobalStep.java | 11 +++++++++--
.../process/traversal/step/filter/SampleGlobalStepTest.java | 6 ++++++
.../test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 1 +
gremlin-go/driver/cucumber/gremlin.go | 1 +
.../javascript/gremlin-javascript/test/cucumber/gremlin.js | 1 +
gremlin-python/src/main/python/radish/gremlin.py | 1 +
.../tinkerpop/gremlin/test/features/filter/Sample.feature | 9 +++++++++
8 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7271c32e2f..2af24acb1e 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -83,6 +83,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-7-4]]
=== TinkerPop 3.7.4 (NOT OFFICIALLY RELEASED YET)
+* Changed sample step to not allow multiple by modulators.
* Added log entry in `WsAndHttpChannelizerHandler` to catch general errors
that escape the handlers.
* Added a `MessageSizeEstimator` implementation to cover `Frame` allowing
Gremlin Server to better estimate message sizes for the direct buffer.
* Improved logging around triggers of the `writeBufferHighWaterMark` so that
they occur more than once but do not excessively fill the logs.
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStep.java
index 82d566f4b2..fbc3e8d66b 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStep.java
@@ -43,7 +43,8 @@ import java.util.Set;
*/
public final class SampleGlobalStep<S> extends CollectingBarrierStep<S>
implements TraversalParent, ByModulating, Seedable {
- private Traversal.Admin<S, Number> probabilityTraversal = new
ConstantTraversal<>(1.0d);
+ private boolean detectedMutipleBy = false;
+ private Traversal.Admin<S, Number> probabilityTraversal = new
ConstantTraversal<>(1.0d);;
private final int amountToSample;
private final Random random = new Random();
@@ -68,13 +69,18 @@ public final class SampleGlobalStep<S> extends
CollectingBarrierStep<S> implemen
@Override
public void modulateBy(final Traversal.Admin<?, ?> probabilityTraversal) {
+ if (this.detectedMutipleBy)
+ throw new IllegalStateException("Sample step can only have one by
modulator");
+ this.detectedMutipleBy = true;
this.probabilityTraversal = this.integrateChild(probabilityTraversal);
}
@Override
public void replaceLocalChild(final Traversal.Admin<?, ?> oldTraversal,
final Traversal.Admin<?, ?> newTraversal) {
- if (null != this.probabilityTraversal &&
this.probabilityTraversal.equals(oldTraversal))
+ if (null != this.probabilityTraversal &&
this.probabilityTraversal.equals(oldTraversal)) {
+ this.detectedMutipleBy = true;
this.probabilityTraversal = this.integrateChild(newTraversal);
+ }
}
@Override
@@ -155,6 +161,7 @@ public final class SampleGlobalStep<S> extends
CollectingBarrierStep<S> implemen
public SampleGlobalStep<S> clone() {
final SampleGlobalStep<S> clone = (SampleGlobalStep<S>) super.clone();
clone.probabilityTraversal = this.probabilityTraversal.clone();
+ clone.detectedMutipleBy = this.detectedMutipleBy;
return clone;
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStepTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStepTest.java
index 3d032eefc4..eebea0a40f 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStepTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/SampleGlobalStepTest.java
@@ -22,6 +22,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Scope;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
+import org.apache.tinkerpop.gremlin.structure.T;
import org.junit.Test;
import java.util.ArrayList;
@@ -59,4 +60,9 @@ public class SampleGlobalStepTest extends StepTest {
assertTrue(list.containsAll(result));
}
}
+
+ @Test(expected = IllegalStateException.class)
+ public void shouldThrowForMultipleByModulators() {
+ __.V().sample(1).by("age").by(T.id);
+ }
}
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
index bf205d5068..336e367b65 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
@@ -342,6 +342,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
{"g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p)
=>g.V().HasLabel("person").Order().By("age").Values<object>("name").Skip<object>(1)}},
{"g_VX1X_valuesXageX_rangeXlocal_20_30X", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V(p["vid1"]).Values<object>("age").Range<object>(Scope.Local, 20,
30)}},
{"g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p)
=>g.V().Map<object>(__.In().HasId(p["vid1"])).Limit<object>(2).Values<object>("name")}},
+ {"g_V_sampleX1X_byXageX_byXT_idX", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V().Sample(1).By("age").By(T.Id)}},
{"g_V_rangeX2_1X", new List<Func<GraphTraversalSource,
IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Range<object>(2, 1)}},
{"g_V_rangeX3_2X", new List<Func<GraphTraversalSource,
IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Range<object>(3, 2)}},
{"g_E_sampleX1X", new List<Func<GraphTraversalSource,
IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().Sample(1)}},
diff --git a/gremlin-go/driver/cucumber/gremlin.go
b/gremlin-go/driver/cucumber/gremlin.go
index 8d6dc28d90..6cb740ea7b 100644
--- a/gremlin-go/driver/cucumber/gremlin.go
+++ b/gremlin-go/driver/cucumber/gremlin.go
@@ -312,6 +312,7 @@ var translationMap = map[string][]func(g
*gremlingo.GraphTraversalSource, p map[
"g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V().HasLabel("person").Order().By("age").Values("name").Skip(1)}},
"g_VX1X_valuesXageX_rangeXlocal_20_30X": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V(p["vid1"]).Values("age").Range(gremlingo.Scope.Local, 20, 30)}},
"g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V().Map(gremlingo.T__.In().HasId(p["vid1"])).Limit(2).Values("name")}},
+ "g_V_sampleX1X_byXageX_byXT_idX": {func(g *gremlingo.GraphTraversalSource,
p map[string]interface{}) *gremlingo.GraphTraversal {return
g.V().Sample(1).By("age").By(gremlingo.T.Id)}},
"g_V_rangeX2_1X": {func(g *gremlingo.GraphTraversalSource, p
map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Range(2, 1)}},
"g_V_rangeX3_2X": {func(g *gremlingo.GraphTraversalSource, p
map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Range(3, 2)}},
"g_E_sampleX1X": {func(g *gremlingo.GraphTraversalSource, p
map[string]interface{}) *gremlingo.GraphTraversal {return g.E().Sample(1)}},
diff --git
a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
index 3d24f23ace..fc9e6b0f05 100644
---
a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
+++
b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
@@ -342,6 +342,7 @@ const gremlins = {
g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X: [function({g}) {
return g.V().hasLabel("person").order().by("age").values("name").skip(1) }],
g_VX1X_valuesXageX_rangeXlocal_20_30X: [function({g, vid1}) { return
g.V(vid1).values("age").range(Scope.local, 20, 30) }],
g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX: [function({g, vid1}) { return
g.V().map(__.in_().hasId(vid1)).limit(2).values("name") }],
+ g_V_sampleX1X_byXageX_byXT_idX: [function({g}) { return
g.V().sample(1).by("age").by(T.id) }],
g_V_rangeX2_1X: [function({g}) { return g.V().range(2, 1) }],
g_V_rangeX3_2X: [function({g}) { return g.V().range(3, 2) }],
g_E_sampleX1X: [function({g}) { return g.E().sample(1) }],
diff --git a/gremlin-python/src/main/python/radish/gremlin.py
b/gremlin-python/src/main/python/radish/gremlin.py
index 438e94a04c..647c50a309 100644
--- a/gremlin-python/src/main/python/radish/gremlin.py
+++ b/gremlin-python/src/main/python/radish/gremlin.py
@@ -315,6 +315,7 @@ world.gremlins = {
'g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X': [(lambda
g:g.V().has_label('person').order().by('age').values('name').skip(1))],
'g_VX1X_valuesXageX_rangeXlocal_20_30X': [(lambda g,
vid1=None:g.V(vid1).values('age').range_(Scope.local, 20, 30))],
'g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX': [(lambda g,
vid1=None:g.V().map(__.in_().has_id(vid1)).limit(2).values('name'))],
+ 'g_V_sampleX1X_byXageX_byXT_idX': [(lambda
g:g.V().sample(1).by('age').by(T.id_))],
'g_V_rangeX2_1X': [(lambda g:g.V().range_(2, 1))],
'g_V_rangeX3_2X': [(lambda g:g.V().range_(3, 2))],
'g_E_sampleX1X': [(lambda g:g.E().sample(1))],
diff --git
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/Sample.feature
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/Sample.feature
index c18e01a8b5..3706652ae7 100644
---
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/Sample.feature
+++
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/Sample.feature
@@ -18,6 +18,15 @@
@StepClassFilter @StepSample
Feature: Step - sample()
+ Scenario: g_V_sampleX1X_byXageX_byXT_idX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().sample(1).by("age").by(T.id)
+ """
+ When iterated to list
+ Then the traversal will raise an error with message containing text of
"Sample step can only have one by modulator"
+
Scenario: g_E_sampleX1X
Given the modern graph
And the traversal of
