Skip to content

Commit 4aef2f9

Browse files
committed
Centralize max string length for functions, and add Replace tests for groups
1 parent 833e381 commit 4aef2f9

File tree

5 files changed

+38
-14
lines changed

5 files changed

+38
-14
lines changed

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/function/scalar/ScalarFunction.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.List;
1414

1515
import static java.util.Collections.emptyList;
16+
import static org.elasticsearch.common.unit.ByteSizeUnit.MB;
1617

1718
/**
1819
* A {@code ScalarFunction} is a {@code Function} that takes values from some
@@ -22,6 +23,14 @@
2223
*/
2324
public abstract class ScalarFunction extends Function {
2425

26+
/**
27+
* Limit for the BytesRef return of functions.
28+
* <p>
29+
* To be used when there's no CircuitBreaking, as an arbitrary measure to limit memory usage.
30+
* </p>
31+
*/
32+
public static final long MAX_BYTES_REF_RESULT_SIZE = MB.toBytes(1);
33+
2534
protected ScalarFunction(Source source) {
2635
super(source, emptyList());
2736
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Repeat.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
public class Repeat extends EsqlScalarFunction implements OptionalArgument {
4141
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Repeat", Repeat::new);
4242

43-
static final long MAX_REPEATED_LENGTH = MB.toBytes(1);
44-
4543
private final Expression str;
4644
private final Expression number;
4745

@@ -123,9 +121,9 @@ static BytesRef process(
123121

124122
static BytesRef processInner(BreakingBytesRefBuilder scratch, BytesRef str, int number) {
125123
int repeatedLen = str.length * number;
126-
if (repeatedLen > MAX_REPEATED_LENGTH) {
124+
if (repeatedLen > MAX_BYTES_REF_RESULT_SIZE) {
127125
throw new IllegalArgumentException(
128-
"Creating repeated strings with more than [" + MAX_REPEATED_LENGTH + "] bytes is not supported"
126+
"Creating repeated strings with more than [" + MAX_BYTES_REF_RESULT_SIZE + "] bytes is not supported"
129127
);
130128
}
131129
scratch.grow(repeatedLen);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Replace.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.compute.ann.Fixed;
1717
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
1818
import org.elasticsearch.xpack.esql.core.expression.Expression;
19+
import org.elasticsearch.xpack.esql.core.expression.function.scalar.ScalarFunction;
1920
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2021
import org.elasticsearch.xpack.esql.core.tree.Source;
2122
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -41,8 +42,6 @@
4142
public class Replace extends EsqlScalarFunction {
4243
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Replace", Replace::new);
4344

44-
static final long MAX_RESULT_LENGTH = MB.toBytes(1);
45-
4645
private final Expression str;
4746
private final Expression regex;
4847
private final Expression newStr;
@@ -138,7 +137,6 @@ static BytesRef process(BytesRef str, BytesRef regex, BytesRef newStr) {
138137
if (str == null) {
139138
return null;
140139
}
141-
142140
if (regex == null || newStr == null) {
143141
return str;
144142
}
@@ -160,8 +158,8 @@ private static BytesRef safeReplace(BytesRef strBytesRef, Pattern regex, BytesRe
160158
do {
161159
m.appendReplacement(result, newStr);
162160

163-
if (result.length() > MAX_RESULT_LENGTH) {
164-
throw new IllegalArgumentException("Creating strings with more than [" + MAX_RESULT_LENGTH + "] bytes is not supported");
161+
if (result.length() > MAX_BYTES_REF_RESULT_SIZE) {
162+
throw new IllegalArgumentException("Creating strings with more than [" + MAX_BYTES_REF_RESULT_SIZE + "] bytes is not supported");
165163
}
166164
} while (m.find());
167165
m.appendTail(result);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/RepeatStaticTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.compute.test.TestBlockFactory;
2222
import org.elasticsearch.test.ESTestCase;
2323
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
24+
import org.elasticsearch.xpack.esql.core.expression.function.scalar.ScalarFunction;
2425
import org.elasticsearch.xpack.esql.core.tree.Source;
2526
import org.elasticsearch.xpack.esql.core.type.DataType;
2627
import org.elasticsearch.xpack.esql.core.type.EsField;
@@ -45,14 +46,14 @@ public class RepeatStaticTests extends ESTestCase {
4546

4647
public void testAlmostTooBig() {
4748
String str = randomAlphaOfLength(1);
48-
int number = (int) Repeat.MAX_REPEATED_LENGTH;
49+
int number = (int) ScalarFunction.MAX_BYTES_REF_RESULT_SIZE;
4950
String repeated = process(str, number);
5051
assertThat(repeated, equalTo(str.repeat(number)));
5152
}
5253

5354
public void testTooBig() {
5455
String str = randomAlphaOfLength(1);
55-
int number = (int) Repeat.MAX_REPEATED_LENGTH + 1;
56+
int number = (int) ScalarFunction.MAX_BYTES_REF_RESULT_SIZE + 1;
5657
String repeated = process(str, number);
5758
assertNull(repeated);
5859
assertWarnings(

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ReplaceTests.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import org.apache.lucene.util.BytesRef;
1414
import org.elasticsearch.xpack.esql.core.expression.Expression;
15+
import org.elasticsearch.xpack.esql.core.expression.function.scalar.ScalarFunction;
1516
import org.elasticsearch.xpack.esql.core.tree.Source;
1617
import org.elasticsearch.xpack.esql.core.type.DataType;
1718
import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
@@ -22,7 +23,6 @@
2223
import java.util.function.Supplier;
2324
import java.util.regex.PatternSyntaxException;
2425

25-
import static org.elasticsearch.xpack.esql.expression.function.scalar.string.Replace.MAX_RESULT_LENGTH;
2626
import static org.hamcrest.Matchers.equalTo;
2727

2828
public class ReplaceTests extends AbstractScalarFunctionTestCase {
@@ -79,6 +79,24 @@ public static Iterable<Object[]> parameters() {
7979
)
8080
);
8181

82+
// Groups
83+
suppliers.add(fixedCase("Full group", "Cats are awesome", ".+", "<$0>", "<Cats are awesome>"));
84+
suppliers.add(fixedCase(
85+
"Nested groups",
86+
"A cat is great, a cat is awesome",
87+
"\\b([Aa] (\\w+)) is (\\w+)\\b",
88+
"$1$2",
89+
"A catcat, a catcat"
90+
));
91+
suppliers.add(fixedCase(
92+
"Multiple groups",
93+
"Cats are awesome",
94+
"(\\w+) (.+)",
95+
"$0 -> $1 and dogs $2",
96+
"Cats are awesome -> Cats and dogs are awesome"
97+
));
98+
99+
// Errors
82100
suppliers.add(new TestCaseSupplier("syntax error", List.of(DataType.KEYWORD, DataType.KEYWORD, DataType.KEYWORD), () -> {
83101
String text = randomAlphaOfLength(10);
84102
String invalidRegex = "[";
@@ -106,7 +124,7 @@ public static Iterable<Object[]> parameters() {
106124
}));
107125

108126
suppliers.add(new TestCaseSupplier("result too big", List.of(DataType.KEYWORD, DataType.KEYWORD, DataType.KEYWORD), () -> {
109-
String textAndNewStr = randomAlphaOfLength((int) (MAX_RESULT_LENGTH / 10));
127+
String textAndNewStr = randomAlphaOfLength((int) (ScalarFunction.MAX_BYTES_REF_RESULT_SIZE / 10));
110128
String regex = ".";
111129
return new TestCaseSupplier.TestCase(
112130
List.of(
@@ -121,7 +139,7 @@ public static Iterable<Object[]> parameters() {
121139
.withWarning(
122140
"Line 1:1: java.lang.IllegalArgumentException: "
123141
+ "Creating strings with more than ["
124-
+ MAX_RESULT_LENGTH
142+
+ ScalarFunction.MAX_BYTES_REF_RESULT_SIZE
125143
+ "] bytes is not supported"
126144
);
127145
}));

0 commit comments

Comments
 (0)