Skip to content

Commit

Permalink
Change substring() to use startIndex, endIndex instead of `startInd…
Browse files Browse the repository at this point in the history
…ex, length`
  • Loading branch information
ryn5 committed Oct 17, 2023
1 parent 1e3c881 commit c87171b
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 72 deletions.
8 changes: 4 additions & 4 deletions docs/src/dev/provider/gremlin-semantics.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1194,9 +1194,9 @@ link:https://tinkerpop.apache.org/docs/x.y.z/reference/#split-step[reference]
[[substring-step]]
=== substring()
*Description:* Returns a substring of the incoming string traverser with a 0-based start index (inclusive) and length specified.
*Description:* Returns a substring of the incoming string traverser with a 0-based start index (inclusive) and end index (exclusive).
*Syntax:* `substring(long, long)`
*Syntax:* `substring(int, int)`
[width="100%",options="header"]
|=========================================================
Expand All @@ -1208,8 +1208,8 @@ link:https://tinkerpop.apache.org/docs/x.y.z/reference/#split-step[reference]
* `int` - The start index, 0 based. If the start index is negative then it will begin at the specified index counted
from the end of the string, or 0 if it exceeds the string length.
* `int` - The number of characters to return. Optional, if it is not specific or if it exceeds the length of the string
then all remaining characters will be returned. Length ≤ 0 will return the empty string.
* `int` - The end index, 0 based. Optional, if it is not specific then all remaining characters will be returned. End
index ≤ start index will return the empty string.
Null values from the incoming traverser are not processed and remain as null when returned.
Expand Down
21 changes: 11 additions & 10 deletions docs/src/upgrade/release-3.7.x.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ gremlin> g.V().hasLabel("person").values("name").split("a")
==>[peter]
----
For `substring()`, the new Gremlin step follows the SQL standard, taking parameters start index and desired length of substring,
instead of start and end indices like Java/Groovy, enabling certain operations that would be complex to achieve with closure:
For `substring()`, the new Gremlin step follows the Python standard, taking parameters start index and optionally an
end index. This will enable certain operations that would be complex to achieve with closure:
[source,text]
----
gremlin> g.V().hasLabel("person").values("name").map{it.get().substring(1,3)}
==>ar
==>ad
==>os
==>et
gremlin> g.V().hasLabel("person").values("name").map{it.get().substring(1,4)}
==>ark
==>ada
==>osh
==>ete
gremlin> g.V().hasLabel("person").values("name").map{it.get().substring(1)}
==>arko
==>adas
Expand All @@ -206,11 +206,12 @@ String index out of range: -2
Type ':help' or ':h' for help.
----
The `substring()`-step will begin at the start index and return a substring with the length specified. Negative start
indices are allowed and will begin at the specified index counted from the end of the string:
The `substring()`-step will return a substring with indices specified by the start and end indices, or from
the start index to the remainder of the string if an end index is not specified. Negative indices are allowed and will
count from the end of the string:
[source,text]
----
gremlin> g.V().hasLabel("person").values("name").substring(1,3)
gremlin> g.V().hasLabel("person").values("name").substring(1,4)
==>ark
==>ada
==>osh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1608,20 +1608,20 @@ public default GraphTraversal<S, String> substring(final int startIndex) {
}

/**
* Returns a substring of the incoming string traverser with a 0-based start index (inclusive) index and length
* specified. If the start index is negative then it will begin at the specified index counted from the end
* of the string, or 0 if exceeding the string length. Length is optional, if it is not specific or if it exceeds
* the length of the string then all remaining characters will be returned. Length <= 0 will return the empty string.
* Null values are not processed and remain as null when returned. If the incoming traverser is a non-String value then an
* {@code IllegalArgumentException} will be thrown.
* Returns a substring of the incoming string traverser with a 0-based start index (inclusive) and end index
* (exclusive). If the start index is negative then it will begin at the specified index counted from the end of the
* string, or 0 if exceeding the string length. If the end index is negative then it will end at the specified index
* counted from the end, or at the end of the string if exceeding the string length. End index <= start index will
* return the empty string. Null values are not processed and remain as null when returned. If the incoming
* traverser is a non-String value then an {@code IllegalArgumentException} will be thrown.
*
* @return the traversal with an appended {@link SubstringStep}.
* @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#replace-step" target="_blank">Reference Documentation - Substring Step</a>
* @since 3.7.1
*/
public default GraphTraversal<S, String> substring(final int startIndex, final int length) {
this.asAdmin().getBytecode().addStep(Symbols.substring, startIndex, length);
return this.asAdmin().addStep(new SubstringStep<>(this.asAdmin(), startIndex, length));
public default GraphTraversal<S, String> substring(final int startIndex, final int endIndex) {
this.asAdmin().getBytecode().addStep(Symbols.substring, startIndex, endIndex);
return this.asAdmin().addStep(new SubstringStep<>(this.asAdmin(), startIndex, endIndex));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,8 @@ public static <A> GraphTraversal<A, String> substring(final int startIndex) {
/**
* @see GraphTraversal#substring(int, int)
*/
public static <A> GraphTraversal<A, String> substring(final int startIndex, final int length) {
return __.<A>start().substring(startIndex, length);
public static <A> GraphTraversal<A, String> substring(final int startIndex, final int endIndex) {
return __.<A>start().substring(startIndex, endIndex);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@

/**
* Reference implementation for substring step, a mid-traversal step which returns a substring of the incoming string
* traverser with a 0-based start index (inclusive) and length specified. If the start index is negative then it will
* begin at the specified index counted from the end of the string, or 0 if exceeding the string length.
* Length is optional, if it is not specific or if it exceeds the length of the string then all remaining characters will
* be returned. Length <= 0 will return the empty string. Null values are not processed and remain as null when returned.
* traverser with a 0-based start index (inclusive) and optionally an end index (exclusive). If the start index is negative then it will
* begin at the specified index counted from the end of the string, or 0 if exceeding the string length. Likewise, if
* the end index is negative then it will end at the specified index counted from the end of the string, or 0 if exceeding the string length.
* End index is optional, if it is not specified or if it exceeds the length of the string then all remaining characters will
* be returned. End index <= start index will return the empty string. Null values are not processed and remain as null when returned.
* If the incoming traverser is a non-String value then an {@code IllegalArgumentException} will be thrown.
*
* @author David Bechberger (http://bechberger.com)
Expand All @@ -40,12 +41,12 @@
public final class SubstringStep<S> extends ScalarMapStep<S, String> {

private final Integer start;
private final Integer length;
private final Integer end;

public SubstringStep(final Traversal.Admin traversal, final Integer startIndex, final Integer length) {
public SubstringStep(final Traversal.Admin traversal, final Integer startIndex, final Integer end) {
super(traversal);
this.start = startIndex;
this.length = length;
this.end = end;
}

public SubstringStep(final Traversal.Admin traversal, final Integer startIndex) {
Expand All @@ -67,17 +68,16 @@ protected String map(final Traverser.Admin<S> traverser) {
if (null == strItem)
return null;

final int newStart = processStartIndex(strItem.length());
if (null == this.length)
final int newStart = processStringIndex(strItem.length(), this.start);
if (null == this.end)
return strItem.substring(newStart);

// length < 0 will return the empty string.
if (this.length <= 0)
final int newEnd = processStringIndex(strItem.length(), this.end);

if (newEnd <= newStart)
return "";

// if length specified exceeds the string length it is assumed to be equal to the length, which returns all
// remaining characters in the string.
return strItem.substring(newStart, Math.min(this.length + newStart, strItem.length()));
return strItem.substring(newStart, newEnd);
}

@Override
Expand All @@ -89,19 +89,18 @@ public Set<TraverserRequirement> getRequirements() {
public int hashCode() {
int result = super.hashCode();
result = 31 * result + this.start.hashCode();
result = 31 * result + (null != this.length ? this.length.hashCode() : 0);
result = 31 * result + (null != this.end ? this.end.hashCode() : 0);
return result;
}

// Helper function to process the start index, if it is negative (which counts from end of string) it is converted
// to the positive index position or 0 when negative index exceeds the string length, if it is positive and exceeds
// Helper function to process indices. If it is negative (which counts from end of string) it is converted
// to the positive index position or 0 when negative index exceeds the string length. If it is positive and exceeds
// the length of the string, it is assumed to equal to the length, which means an empty string will be returned.
private int processStartIndex(int strLen) {
if (this.start < 0) {
return Math.max(0, (strLen + this.start) % strLen);
private int processStringIndex(int strLen, int index) {
if (index < 0) {
return Math.max(0, (strLen + index) % strLen);
} else {
return Math.min(this.start, strLen);
return Math.min(index, strLen);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,28 @@ protected List<Traversal> getTraversals() {

@Test
public void testReturnTypes() {
assertEquals("ello wor", __.__("hello world").substring(1, 8).next());
assertEquals("ello", __.__("hello").substring(1, 8).next());
assertEquals("world", __.__("hello world").substring(6).next());
assertEquals("hello world", __.__("hello world").substring(0).next());
assertEquals("ello world", __.__("hello world").substring(1).next());
assertEquals("d", __.__("hello world").substring(-1).next());
assertEquals("d", __.__("hello world").substring(10).next());
assertEquals("", __.__("hello world").substring(11).next());

assertEquals("world", __.__("world").substring(-10).next());
assertEquals("orld", __.__("world").substring(-4).next());
assertEquals("orl", __.__("world").substring(-4, 3).next());
assertEquals("", __.__("world").substring(1, -1).next());
assertEquals("", __.__("hello world").substring(0, 0).next());
assertEquals("h", __.__("hello world").substring(0, 1).next());
assertEquals("", __.__("hello world").substring(1, 0).next());
assertEquals("hello worl", __.__("hello world").substring(0, -1).next());
assertEquals("", __.__("hello world").substring(-1, 0).next());
assertEquals("ello worl", __.__("hello world").substring(1, -1).next());
assertEquals("", __.__("hello world").substring(-1, 1).next());
assertEquals("rl", __.__("hello world").substring(-3, -1).next());
assertEquals("", __.__("hello world").substring(-1, -3).next());
assertEquals("d", __.__("hello world").substring(-1, 11).next());
assertEquals("ello world", __.__("hello world").substring(1, 11).next());
assertEquals("d", __.__("hello world").substring(10, 11).next());
assertEquals("hello world", __.__("hello world").substring(-11, 11).next());
assertEquals("h", __.__("hello world").substring(-11, 1).next());

assertArrayEquals(new String[]{"st", "llo worl", null, ""},
assertArrayEquals(new String[]{"st", "llo wo", null, ""},
__.__("test", "hello world", null, "").substring(2, 8).toList().toArray());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1938,9 +1938,9 @@ public GraphTraversal<TStart, Edge> Subgraph (string sideEffectKey)
/// <summary>
/// Adds the subgraph step to this <see cref="GraphTraversal{SType, EType}" />.
/// </summary>
public GraphTraversal<TStart, string?> Substring (int startIndex, int length)
public GraphTraversal<TStart, string?> Substring (int startIndex, int endIndex)
{
Bytecode.AddStep("substring", startIndex, length);
Bytecode.AddStep("substring", startIndex, endIndex);
return Wrap<TStart, string?>(this);
}

Expand Down
4 changes: 2 additions & 2 deletions gremlin-dotnet/src/Gremlin.Net/Process/Traversal/__.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1381,9 +1381,9 @@ public static GraphTraversal<object, Edge> Subgraph(string sideEffectKey)
/// <summary>
/// Spawns a <see cref="GraphTraversal{SType, EType}" /> and adds the substring step to that traversal.
/// </summary>
public static GraphTraversal<object, string?> Substring(int startIndex, int length)
public static GraphTraversal<object, string?> Substring(int startIndex, int endIndex)
{
return new GraphTraversal<object, string?>().Substring(startIndex, length);
return new GraphTraversal<object, string?>().Substring(startIndex, endIndex);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1005,11 +1005,14 @@ private static IDictionary<string, List<Func<GraphTraversalSource, IDictionary<s
{"g_V_hasLabelXpersonX_valueXnameX_splitXnullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Values<object>("name").Split(null)}},
{"g_V_hasLabelXpersonX_valueXnameX_splitXaX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Values<object>("name").Split("a")}},
{"g_injectXthat_this_testX_substringX1_8X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject<object>("test","hello world",null).Substring(1,8)}},
{"g_injectXListXa_bXcX_substringX1_1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject(p["xx1"]).Substring(1,1)}},
{"g_injectXListXa_bXcX_substringX1_2X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.Inject(p["xx1"]).Substring(1,2)}},
{"g_V_hasLabelXpersonX_valueXnameX_substringX2X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(2)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringX1_3X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(1,3)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringX1_4X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(1,4)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringX1_0X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(1,0)}},
{"g_V_hasLabelXpersonX_valueXnameX_substringXneg3X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Values<object>("name").Substring(-3)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringX1_neg1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(1,-1)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringXneg4_2X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(-4,2)}},
{"g_V_hasLabelXsoftwareX_valueXnameX_substringXneg3_neg1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("software").Values<object>("name").Substring(-3,-1)}},
{"g_V_age_sum", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Sum<object>()}},
{"g_V_foo_sum", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("foo").Sum<object>()}},
{"g_V_age_fold_sumXlocalX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Values<object>("age").Fold().Sum<object>(Scope.Local)}},
Expand Down
Loading

0 comments on commit c87171b

Please sign in to comment.