From 6c961ade72371cddf1fe22ac1ffe660db7752df1 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sun, 25 Aug 2024 16:46:28 +0200 Subject: [PATCH 01/24] Initial implementation of SPACE() function. --- .../src/main/resources/meta.csv-spec | 6 +- .../src/main/resources/string.csv-spec | 47 ++++++ .../scalar/string/SpaceConstantEvaluator.java | 95 ++++++++++++ .../scalar/string/SpaceEvaluator.java | 128 ++++++++++++++++ .../function/EsqlFunctionRegistry.java | 4 +- .../function/scalar/UnaryScalarFunction.java | 2 + .../function/scalar/string/Space.java | 141 ++++++++++++++++++ 7 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java create mode 100644 x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceEvaluator.java create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec index f1f66a9cb990c..b5ec41c8f050f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec @@ -72,6 +72,7 @@ double pi() "double signum(number:double|integer|long|unsigned_long)" "double sin(angle:double|integer|long|unsigned_long)" "double sinh(angle:double|integer|long|unsigned_long)" +"keyword space(number:integer)" "keyword split(string:keyword|text, delim:keyword|text)" "double sqrt(number:double|integer|long|unsigned_long)" "geo_point|cartesian_point st_centroid_agg(field:geo_point|cartesian_point)" @@ -196,6 +197,7 @@ rtrim |string |"keyword|text" signum |number |"double|integer|long|unsigned_long" |"Numeric expression. If `null`, the function returns `null`." sin |angle |"double|integer|long|unsigned_long" |An angle, in radians. If `null`, the function returns `null`. sinh |angle |"double|integer|long|unsigned_long" |An angle, in radians. If `null`, the function returns `null`. +space |number |"integer" |Number of spaces in result. split |[string, delim] |["keyword|text", "keyword|text"] |[String expression. If `null`\, the function returns `null`., Delimiter. Only single byte delimiters are currently supported.] sqrt |number |"double|integer|long|unsigned_long" |"Numeric expression. If `null`, the function returns `null`." st_centroid_ag|field |"geo_point|cartesian_point" |[""] @@ -320,6 +322,7 @@ rtrim |Removes trailing whitespaces from a string. signum |Returns the sign of the given number. It returns `-1` for negative numbers, `0` for `0` and `1` for positive numbers. sin |Returns ths {wikipedia}/Sine_and_cosine[Sine] trigonometric function of an angle. sinh |Returns the {wikipedia}/Hyperbolic_functions[hyperbolic sine] of an angle. +space |Returns a string of `number` spaces. split |Split a single valued string into multiple strings. sqrt |Returns the square root of a number. The input can be any numeric value, the return value is always a double. Square roots of negative numbers and infinities are null. st_centroid_ag|Calculate the spatial centroid over a field with spatial point geometry type. @@ -446,6 +449,7 @@ rtrim |"keyword|text" signum |double |false |false |false sin |double |false |false |false sinh |double |false |false |false +space |keyword |false |false |false split |keyword |[false, false] |false |false sqrt |double |false |false |false st_centroid_ag|"geo_point|cartesian_point" |false |false |true @@ -508,5 +512,5 @@ countFunctions#[skip:-8.15.99] meta functions | stats a = count(*), b = count(*), c = count(*) | mv_expand c; a:long | b:long | c:long -115 | 115 | 115 +116 | 116 | 116 ; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index f85a3bb01ad40..1ee6230442224 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1595,4 +1595,51 @@ emp_no:integer | languages:integer | first_name:keyword 10004 | 5 | ChirstianChirstianChirstianChirstianChirstian ; +space +ROW message = CONCAT("Hello", SPACE(1), "World!"); +message:keyword +Hello World! +; + +spaceLength +ROW len = LENGTH(SPACE(12)); + +len:integer +12 +; + +spaceCalculated +ROW inner_width = 20, title = "Title" +| EVAL lspace = SPACE((inner_width-LENGTH(title))/2), + rspace = SPACE(inner_width-LENGTH(lspace)-LENGTH(title)), + centered_title = CONCAT("<",lspace,title,rspace,">") +| KEEP inner_width, centered_title; + +inner_width:integer | centered_title:keyword +20 | < Title > +; + +spaceZero +ROW s = SPACE(0); + +s:keyword +"" +; + +spaceNull +ROW s = SPACE(null); + +s:keyword +null +; + +spaceNegative +FROM employees | SORT emp_no | LIMIT 1 | EVAL s = SPACE(-LENGTH(first_name)) | KEEP s; + +warning:Line 1:51: evaluation of [SPACE(-LENGTH(first_name))] failed, treating result as null. Only first 20 failures recorded. +warning:Line 1:51: java.lang.IllegalArgumentException: Number parameter cannot be negative, found [-6] + +s:keyword +null +; diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java new file mode 100644 index 0000000000000..23e033efa2be5 --- /dev/null +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java @@ -0,0 +1,95 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License +// 2.0; you may not use this file except in compliance with the Elastic License +// 2.0. +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import java.lang.IllegalArgumentException; +import java.lang.Override; +import java.lang.String; +import java.util.function.Function; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BytesRefBlock; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.EvalOperator; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.expression.function.Warnings; + +/** + * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Space}. + * This class is generated. Do not edit it. + */ +public final class SpaceConstantEvaluator implements EvalOperator.ExpressionEvaluator { + private final Warnings warnings; + + private final BreakingBytesRefBuilder scratch; + + private final int number; + + private final DriverContext driverContext; + + public SpaceConstantEvaluator(Source source, BreakingBytesRefBuilder scratch, int number, + DriverContext driverContext) { + this.scratch = scratch; + this.number = number; + this.driverContext = driverContext; + this.warnings = Warnings.createWarnings(driverContext.warningsMode(), source); + } + + @Override + public Block eval(Page page) { + return eval(page.getPositionCount()); + } + + public BytesRefBlock eval(int positionCount) { + try(BytesRefBlock.Builder result = driverContext.blockFactory().newBytesRefBlockBuilder(positionCount)) { + position: for (int p = 0; p < positionCount; p++) { + try { + result.appendBytesRef(Space.processConstant(this.scratch, this.number)); + } catch (IllegalArgumentException e) { + warnings.registerException(e); + result.appendNull(); + } + } + return result.build(); + } + } + + @Override + public String toString() { + return "SpaceConstantEvaluator[" + "number=" + number + "]"; + } + + @Override + public void close() { + Releasables.closeExpectNoException(scratch); + } + + static class Factory implements EvalOperator.ExpressionEvaluator.Factory { + private final Source source; + + private final Function scratch; + + private final int number; + + public Factory(Source source, Function scratch, + int number) { + this.source = source; + this.scratch = scratch; + this.number = number; + } + + @Override + public SpaceConstantEvaluator get(DriverContext context) { + return new SpaceConstantEvaluator(source, scratch.apply(context), number, context); + } + + @Override + public String toString() { + return "SpaceConstantEvaluator[" + "number=" + number + "]"; + } + } +} diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceEvaluator.java new file mode 100644 index 0000000000000..0252bd85f6ff8 --- /dev/null +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceEvaluator.java @@ -0,0 +1,128 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License +// 2.0; you may not use this file except in compliance with the Elastic License +// 2.0. +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import java.lang.IllegalArgumentException; +import java.lang.Override; +import java.lang.String; +import java.util.function.Function; +import org.elasticsearch.compute.data.Block; +import org.elasticsearch.compute.data.BytesRefBlock; +import org.elasticsearch.compute.data.IntBlock; +import org.elasticsearch.compute.data.IntVector; +import org.elasticsearch.compute.data.Page; +import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; +import org.elasticsearch.compute.operator.DriverContext; +import org.elasticsearch.compute.operator.EvalOperator; +import org.elasticsearch.core.Releasables; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.expression.function.Warnings; + +/** + * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Space}. + * This class is generated. Do not edit it. + */ +public final class SpaceEvaluator implements EvalOperator.ExpressionEvaluator { + private final Warnings warnings; + + private final BreakingBytesRefBuilder scratch; + + private final EvalOperator.ExpressionEvaluator number; + + private final DriverContext driverContext; + + public SpaceEvaluator(Source source, BreakingBytesRefBuilder scratch, + EvalOperator.ExpressionEvaluator number, DriverContext driverContext) { + this.scratch = scratch; + this.number = number; + this.driverContext = driverContext; + this.warnings = Warnings.createWarnings(driverContext.warningsMode(), source); + } + + @Override + public Block eval(Page page) { + try (IntBlock numberBlock = (IntBlock) number.eval(page)) { + IntVector numberVector = numberBlock.asVector(); + if (numberVector == null) { + return eval(page.getPositionCount(), numberBlock); + } + return eval(page.getPositionCount(), numberVector); + } + } + + public BytesRefBlock eval(int positionCount, IntBlock numberBlock) { + try(BytesRefBlock.Builder result = driverContext.blockFactory().newBytesRefBlockBuilder(positionCount)) { + position: for (int p = 0; p < positionCount; p++) { + if (numberBlock.isNull(p)) { + result.appendNull(); + continue position; + } + if (numberBlock.getValueCount(p) != 1) { + if (numberBlock.getValueCount(p) > 1) { + warnings.registerException(new IllegalArgumentException("single-value function encountered multi-value")); + } + result.appendNull(); + continue position; + } + try { + result.appendBytesRef(Space.process(this.scratch, numberBlock.getInt(numberBlock.getFirstValueIndex(p)))); + } catch (IllegalArgumentException e) { + warnings.registerException(e); + result.appendNull(); + } + } + return result.build(); + } + } + + public BytesRefBlock eval(int positionCount, IntVector numberVector) { + try(BytesRefBlock.Builder result = driverContext.blockFactory().newBytesRefBlockBuilder(positionCount)) { + position: for (int p = 0; p < positionCount; p++) { + try { + result.appendBytesRef(Space.process(this.scratch, numberVector.getInt(p))); + } catch (IllegalArgumentException e) { + warnings.registerException(e); + result.appendNull(); + } + } + return result.build(); + } + } + + @Override + public String toString() { + return "SpaceEvaluator[" + "number=" + number + "]"; + } + + @Override + public void close() { + Releasables.closeExpectNoException(scratch, number); + } + + static class Factory implements EvalOperator.ExpressionEvaluator.Factory { + private final Source source; + + private final Function scratch; + + private final EvalOperator.ExpressionEvaluator.Factory number; + + public Factory(Source source, Function scratch, + EvalOperator.ExpressionEvaluator.Factory number) { + this.source = source; + this.scratch = scratch; + this.number = number; + } + + @Override + public SpaceEvaluator get(DriverContext context) { + return new SpaceEvaluator(source, scratch.apply(context), number.get(context), context); + } + + @Override + public String toString() { + return "SpaceEvaluator[" + "number=" + number + "]"; + } + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java index 0d50623fe77eb..bb2ecefabf23e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java @@ -119,6 +119,7 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.string.Repeat; import org.elasticsearch.xpack.esql.expression.function.scalar.string.Replace; import org.elasticsearch.xpack.esql.expression.function.scalar.string.Right; +import org.elasticsearch.xpack.esql.expression.function.scalar.string.Space; import org.elasticsearch.xpack.esql.expression.function.scalar.string.Split; import org.elasticsearch.xpack.esql.expression.function.scalar.string.StartsWith; import org.elasticsearch.xpack.esql.expression.function.scalar.string.Substring; @@ -307,7 +308,8 @@ private FunctionDefinition[][] functions() { def(ToLower.class, ToLower::new, "to_lower"), def(ToUpper.class, ToUpper::new, "to_upper"), def(Locate.class, Locate::new, "locate"), - def(Repeat.class, Repeat::new, "repeat") }, + def(Repeat.class, Repeat::new, "repeat"), + def(Space.class, Space::new, "space") }, // date new FunctionDefinition[] { def(DateDiff.class, DateDiff::new, "date_diff"), diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java index 980d3ab0e7552..bdbc9b649c101 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java @@ -58,6 +58,7 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.string.Length; import org.elasticsearch.xpack.esql.expression.function.scalar.string.RLike; import org.elasticsearch.xpack.esql.expression.function.scalar.string.RTrim; +import org.elasticsearch.xpack.esql.expression.function.scalar.string.Space; import org.elasticsearch.xpack.esql.expression.function.scalar.string.Trim; import org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLike; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Neg; @@ -96,6 +97,7 @@ public static List getNamedWriteables() { entries.add(Signum.ENTRY); entries.add(Sin.ENTRY); entries.add(Sinh.ENTRY); + entries.add(Space.ENTRY); entries.add(Sqrt.ENTRY); entries.add(StX.ENTRY); entries.add(StY.ENTRY); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java new file mode 100644 index 0000000000000..bb0b8a4e0bc58 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -0,0 +1,141 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.compute.ann.Evaluator; +import org.elasticsearch.compute.ann.Fixed; +import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; +import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.NodeInfo; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.expression.function.Example; +import org.elasticsearch.xpack.esql.expression.function.FunctionInfo; +import org.elasticsearch.xpack.esql.expression.function.Param; +import org.elasticsearch.xpack.esql.expression.function.scalar.UnaryScalarFunction; +import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; + +import java.io.IOException; +import java.util.List; +import java.util.function.Function; + +import static org.elasticsearch.common.unit.ByteSizeUnit.MB; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType; + +public class Space extends UnaryScalarFunction { + public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Space", Space::new); + + static final long MAX_LENGTH = MB.toBytes(1); + + private final Expression number; + + @FunctionInfo( + returnType = "keyword", + description = "Returns a string of `number` spaces.", + examples = @Example(file = "string", tag = "space") + ) + public Space( + Source source, + @Param(name = "number", type = { "integer" }, description = "Number of spaces in result.") Expression number + ) { + super(source, number); + this.number = number; + } + + private Space(StreamInput in) throws IOException { + this(Source.readFrom((PlanStreamInput) in), in.readNamedWriteable(Expression.class)); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + source().writeTo(out); + out.writeNamedWriteable(number); + } + + @Override + public String getWriteableName() { + return ENTRY.name; + } + + @Override + public DataType dataType() { + return DataType.KEYWORD; + } + + @Override + protected TypeResolution resolveType() { + return isType(number, dt -> dt == DataType.INTEGER, sourceText(), FIRST, "integer"); + } + + @Override + public boolean foldable() { + return number.foldable(); + } + + @Evaluator(extraName = "Constant", warnExceptions = { IllegalArgumentException.class }) + static BytesRef processConstant(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, @Fixed int number) { + return processInner(scratch, number); + } + + @Evaluator(warnExceptions = { IllegalArgumentException.class }) + static BytesRef process(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, int number) { + if (number < 0) { + throw new IllegalArgumentException("Number parameter cannot be negative, found [" + number + "]"); + } + if (number > MAX_LENGTH) { + throw new IllegalArgumentException("Creating strings longer than [" + MAX_LENGTH + "] bytes is not supported"); + } + return processInner(scratch, number); + } + + static BytesRef processInner(BreakingBytesRefBuilder scratch, int number) { + scratch.grow(number); + scratch.clear(); + for (int i = 0; i < number; ++i) { + scratch.append((byte) ' '); + } + return scratch.bytesRefView(); + } + + @Override + public Expression replaceChildren(List newChildren) { + return new Space(source(), newChildren.get(0)); + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, Space::new, number); + } + + @Override + public ExpressionEvaluator.Factory toEvaluator(Function toEvaluator) { + if (number.foldable()) { + int num = (int) number.fold(); + if (num < 0) { + throw new IllegalArgumentException("Number parameter cannot be negative, found [" + number + "]"); + } + if (num > MAX_LENGTH) { + throw new IllegalArgumentException("Creating strings longer than [" + MAX_LENGTH + "] bytes is not supported"); + } + return new SpaceConstantEvaluator.Factory(source(), context -> new BreakingBytesRefBuilder(context.breaker(), "space"), num); + } + + ExpressionEvaluator.Factory numberExpr = toEvaluator.apply(number); + return new SpaceEvaluator.Factory(source(), context -> new BreakingBytesRefBuilder(context.breaker(), "space"), numberExpr); + } + + Expression number() { + return number; + } +} From 6e6003c94008b7c422171fa6c3248552d543e9ee Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sun, 25 Aug 2024 23:10:00 +0200 Subject: [PATCH 02/24] SpaceTests.java --- .../function/scalar/string/SpaceTests.java | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java new file mode 100644 index 0000000000000..3a0955275527e --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -0,0 +1,77 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase; +import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +public class SpaceTests extends AbstractScalarFunctionTestCase { + public SpaceTests(@Name("TestCase") Supplier testCaseSupplier) { + this.testCase = testCaseSupplier.get(); + } + + @ParametersFactory + public static Iterable parameters() { + + List cases = new ArrayList<>(); + + cases.add(new TestCaseSupplier("Space basic test", List.of(DataType.INTEGER), () -> { + int number = between(0, 10); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), + "SpaceEvaluator[number=Attribute[channel=0]]", + DataType.KEYWORD, + equalTo(new BytesRef(" ".repeat(number))) + ); + })); + + cases.add(new TestCaseSupplier("Space with number zero", List.of(DataType.INTEGER), () -> { + int number = 0; + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), + "SpaceEvaluator[number=Attribute[channel=0]]", + DataType.KEYWORD, + equalTo(new BytesRef("")) + ); + })); + + cases.add(new TestCaseSupplier("Space with negative number", List.of(DataType.INTEGER), () -> { + int number = randomIntBetween(-10, -1); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), + "SpaceEvaluator[number=Attribute[channel=0]]", + DataType.KEYWORD, + nullValue() + ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.") + .withWarning("Line -1:-1: java.lang.IllegalArgumentException: Number parameter cannot be negative, found [" + number + "]") + .withFoldingException(IllegalArgumentException.class, "Number parameter cannot be negative, found [" + number + "]"); + })); + + cases = anyNullIsNull(true, cases); + return parameterSuppliersFromTypedData(cases); + } + + @Override + protected Expression build(Source source, List args) { + return new Space(source, args.get(0)); + } +} From 4c4fa12253e072e0bd1dc8a655f4344178240909 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Mon, 26 Aug 2024 17:25:52 +0200 Subject: [PATCH 03/24] Add docs. --- .../esql/functions/description/space.asciidoc | 5 ++++ .../esql/functions/examples/space.asciidoc | 13 +++++++++++ .../functions/kibana/definition/space.json | 23 +++++++++++++++++++ .../esql/functions/kibana/docs/space.md | 10 ++++++++ .../esql/functions/layout/space.asciidoc | 15 ++++++++++++ .../esql/functions/parameters/space.asciidoc | 6 +++++ .../esql/functions/signature/space.svg | 1 + .../esql/functions/string-functions.asciidoc | 2 ++ .../esql/functions/types/space.asciidoc | 9 ++++++++ .../src/main/resources/string.csv-spec | 4 ++++ 10 files changed, 88 insertions(+) create mode 100644 docs/reference/esql/functions/description/space.asciidoc create mode 100644 docs/reference/esql/functions/examples/space.asciidoc create mode 100644 docs/reference/esql/functions/kibana/definition/space.json create mode 100644 docs/reference/esql/functions/kibana/docs/space.md create mode 100644 docs/reference/esql/functions/layout/space.asciidoc create mode 100644 docs/reference/esql/functions/parameters/space.asciidoc create mode 100644 docs/reference/esql/functions/signature/space.svg create mode 100644 docs/reference/esql/functions/types/space.asciidoc diff --git a/docs/reference/esql/functions/description/space.asciidoc b/docs/reference/esql/functions/description/space.asciidoc new file mode 100644 index 0000000000000..cf85c4a9dfafd --- /dev/null +++ b/docs/reference/esql/functions/description/space.asciidoc @@ -0,0 +1,5 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Description* + +Returns a string of `number` spaces. diff --git a/docs/reference/esql/functions/examples/space.asciidoc b/docs/reference/esql/functions/examples/space.asciidoc new file mode 100644 index 0000000000000..cef3cd6139021 --- /dev/null +++ b/docs/reference/esql/functions/examples/space.asciidoc @@ -0,0 +1,13 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Example* + +[source.merge.styled,esql] +---- +include::{esql-specs}/string.csv-spec[tag=space] +---- +[%header.monospaced.styled,format=dsv,separator=|] +|=== +include::{esql-specs}/string.csv-spec[tag=space-result] +|=== + diff --git a/docs/reference/esql/functions/kibana/definition/space.json b/docs/reference/esql/functions/kibana/definition/space.json new file mode 100644 index 0000000000000..d744e0cf3c201 --- /dev/null +++ b/docs/reference/esql/functions/kibana/definition/space.json @@ -0,0 +1,23 @@ +{ + "comment" : "This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.", + "type" : "eval", + "name" : "space", + "description" : "Returns a string of `number` spaces.", + "signatures" : [ + { + "params" : [ + { + "name" : "number", + "type" : "integer", + "optional" : false, + "description" : "Number of spaces in result." + } + ], + "variadic" : false, + "returnType" : "keyword" + } + ], + "examples" : [ + "ROW message = CONCAT(\"Hello\", SPACE(1), \"World!\");" + ] +} diff --git a/docs/reference/esql/functions/kibana/docs/space.md b/docs/reference/esql/functions/kibana/docs/space.md new file mode 100644 index 0000000000000..11c221a63472a --- /dev/null +++ b/docs/reference/esql/functions/kibana/docs/space.md @@ -0,0 +1,10 @@ + + +### SPACE +Returns a string of `number` spaces. + +``` +ROW message = CONCAT("Hello", SPACE(1), "World!"); +``` diff --git a/docs/reference/esql/functions/layout/space.asciidoc b/docs/reference/esql/functions/layout/space.asciidoc new file mode 100644 index 0000000000000..22355d1e24978 --- /dev/null +++ b/docs/reference/esql/functions/layout/space.asciidoc @@ -0,0 +1,15 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +[discrete] +[[esql-space]] +=== `SPACE` + +*Syntax* + +[.text-center] +image::esql/functions/signature/space.svg[Embedded,opts=inline] + +include::../parameters/space.asciidoc[] +include::../description/space.asciidoc[] +include::../types/space.asciidoc[] +include::../examples/space.asciidoc[] diff --git a/docs/reference/esql/functions/parameters/space.asciidoc b/docs/reference/esql/functions/parameters/space.asciidoc new file mode 100644 index 0000000000000..de4efd34c0ba4 --- /dev/null +++ b/docs/reference/esql/functions/parameters/space.asciidoc @@ -0,0 +1,6 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Parameters* + +`number`:: +Number of spaces in result. diff --git a/docs/reference/esql/functions/signature/space.svg b/docs/reference/esql/functions/signature/space.svg new file mode 100644 index 0000000000000..c506c25dfcb16 --- /dev/null +++ b/docs/reference/esql/functions/signature/space.svg @@ -0,0 +1 @@ +SPACE(number) \ No newline at end of file diff --git a/docs/reference/esql/functions/string-functions.asciidoc b/docs/reference/esql/functions/string-functions.asciidoc index d4b120ad1c45b..ed97769b900e7 100644 --- a/docs/reference/esql/functions/string-functions.asciidoc +++ b/docs/reference/esql/functions/string-functions.asciidoc @@ -19,6 +19,7 @@ * <> * <> * <> +* <> * <> * <> * <> @@ -39,6 +40,7 @@ include::layout/repeat.asciidoc[] include::layout/replace.asciidoc[] include::layout/right.asciidoc[] include::layout/rtrim.asciidoc[] +include::layout/space.asciidoc[] include::layout/split.asciidoc[] include::layout/starts_with.asciidoc[] include::layout/substring.asciidoc[] diff --git a/docs/reference/esql/functions/types/space.asciidoc b/docs/reference/esql/functions/types/space.asciidoc new file mode 100644 index 0000000000000..3f2e89f80d3e5 --- /dev/null +++ b/docs/reference/esql/functions/types/space.asciidoc @@ -0,0 +1,9 @@ +// This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it. + +*Supported types* + +[%header.monospaced.styled,format=dsv,separator=|] +|=== +number | result +integer | keyword +|=== diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index 1ee6230442224..f79aed5519d29 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1596,10 +1596,14 @@ emp_no:integer | languages:integer | first_name:keyword ; space +// tag::space[] ROW message = CONCAT("Hello", SPACE(1), "World!"); +// end::space[] +// tag::space-result[] message:keyword Hello World! +// end::space-result[] ; spaceLength From 43eb5bce5e0673c2aea728f83ed06eb8d5b572d9 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 10:01:43 +0200 Subject: [PATCH 04/24] Remove warnExceptions parameter from the @Evaluator for processConstant. When the function parameter is foldable, all checks of that parameter are done before the ExpressionEvaluator.Factory is returned. --- .../scalar/string/SpaceConstantEvaluator.java | 16 +++++----------- .../expression/function/scalar/string/Space.java | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java index 23e033efa2be5..7e9c4fafadddb 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java @@ -4,12 +4,11 @@ // 2.0. package org.elasticsearch.xpack.esql.expression.function.scalar.string; -import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; import java.util.function.Function; import org.elasticsearch.compute.data.Block; -import org.elasticsearch.compute.data.BytesRefBlock; +import org.elasticsearch.compute.data.BytesRefVector; import org.elasticsearch.compute.data.Page; import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; import org.elasticsearch.compute.operator.DriverContext; @@ -41,18 +40,13 @@ public SpaceConstantEvaluator(Source source, BreakingBytesRefBuilder scratch, in @Override public Block eval(Page page) { - return eval(page.getPositionCount()); + return eval(page.getPositionCount()).asBlock(); } - public BytesRefBlock eval(int positionCount) { - try(BytesRefBlock.Builder result = driverContext.blockFactory().newBytesRefBlockBuilder(positionCount)) { + public BytesRefVector eval(int positionCount) { + try(BytesRefVector.Builder result = driverContext.blockFactory().newBytesRefVectorBuilder(positionCount)) { position: for (int p = 0; p < positionCount; p++) { - try { - result.appendBytesRef(Space.processConstant(this.scratch, this.number)); - } catch (IllegalArgumentException e) { - warnings.registerException(e); - result.appendNull(); - } + result.appendBytesRef(Space.processConstant(this.scratch, this.number)); } return result.build(); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index bb0b8a4e0bc58..86c99f7a09c8b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -83,7 +83,7 @@ public boolean foldable() { return number.foldable(); } - @Evaluator(extraName = "Constant", warnExceptions = { IllegalArgumentException.class }) + @Evaluator(extraName = "Constant") static BytesRef processConstant(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, @Fixed int number) { return processInner(scratch, number); } From b47dc54aaefe22ecf0e352fdcaeb2fde12f10078 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 10:15:06 +0200 Subject: [PATCH 05/24] Extract duplicate checks of number parameter range. --- .../expression/function/scalar/string/Space.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 86c99f7a09c8b..b72aaa7cfc0f6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -90,13 +90,17 @@ static BytesRef processConstant(@Fixed(includeInToString = false, build = true) @Evaluator(warnExceptions = { IllegalArgumentException.class }) static BytesRef process(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, int number) { + checkNumber(number); + return processInner(scratch, number); + } + + static void checkNumber(int number) { if (number < 0) { throw new IllegalArgumentException("Number parameter cannot be negative, found [" + number + "]"); } if (number > MAX_LENGTH) { throw new IllegalArgumentException("Creating strings longer than [" + MAX_LENGTH + "] bytes is not supported"); } - return processInner(scratch, number); } static BytesRef processInner(BreakingBytesRefBuilder scratch, int number) { @@ -122,12 +126,7 @@ protected NodeInfo info() { public ExpressionEvaluator.Factory toEvaluator(Function toEvaluator) { if (number.foldable()) { int num = (int) number.fold(); - if (num < 0) { - throw new IllegalArgumentException("Number parameter cannot be negative, found [" + number + "]"); - } - if (num > MAX_LENGTH) { - throw new IllegalArgumentException("Creating strings longer than [" + MAX_LENGTH + "] bytes is not supported"); - } + checkNumber(num); return new SpaceConstantEvaluator.Factory(source(), context -> new BreakingBytesRefBuilder(context.breaker(), "space"), num); } From 72350cf56f32068f6c5c4363cddbb6ed8fd6f70d Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 11:40:44 +0200 Subject: [PATCH 06/24] Test and fix type resolution. --- .../esql/expression/function/scalar/string/Space.java | 9 ++++++++- .../expression/function/scalar/string/SpaceTests.java | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index b72aaa7cfc0f6..1136626eef28c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -30,8 +30,11 @@ import java.util.function.Function; import static org.elasticsearch.common.unit.ByteSizeUnit.MB; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNumeric; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType; +import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isWholeNumber; public class Space extends UnaryScalarFunction { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Space", Space::new); @@ -75,7 +78,11 @@ public DataType dataType() { @Override protected TypeResolution resolveType() { - return isType(number, dt -> dt == DataType.INTEGER, sourceText(), FIRST, "integer"); + if (childrenResolved() == false) { + return new TypeResolution("Unresolved children"); + } + + return isType(number, dt -> dt == DataType.INTEGER, sourceText(), DEFAULT, "integer"); } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java index 3a0955275527e..19bc12bda04d0 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -67,6 +67,7 @@ public static Iterable parameters() { })); cases = anyNullIsNull(true, cases); + cases = errorsForCasesWithoutExamples(cases, (v, p) -> "integer"); return parameterSuppliersFromTypedData(cases); } From 991dd76a236a978e56a21fe068bd78417e338d1e Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 11:57:46 +0200 Subject: [PATCH 07/24] Add test for requesting space too large. --- .../function/scalar/string/SpaceTests.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java index 19bc12bda04d0..932e0599be1db 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.function.Supplier; +import static org.elasticsearch.common.unit.ByteSizeUnit.MB; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -66,6 +67,19 @@ public static Iterable parameters() { .withFoldingException(IllegalArgumentException.class, "Number parameter cannot be negative, found [" + number + "]"); })); + cases.add(new TestCaseSupplier("Space with number too large", List.of(DataType.INTEGER), () -> { + int max = (int) MB.toBytes(1); + int number = randomIntBetween(max, max+10); + return new TestCaseSupplier.TestCase( + List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), + "SpaceEvaluator[number=Attribute[channel=0]]", + DataType.KEYWORD, + nullValue() + ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.") + .withWarning("Line -1:-1: java.lang.IllegalArgumentException: Creating strings longer than [" + max + "] bytes is not supported") + .withFoldingException(IllegalArgumentException.class, "Creating strings longer than [" + max + "] bytes is not supported"); + })); + cases = anyNullIsNull(true, cases); cases = errorsForCasesWithoutExamples(cases, (v, p) -> "integer"); return parameterSuppliersFromTypedData(cases); From bb3bc07019f1cdb5663204dbef3b4ea112f1983f Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 11:43:13 +0200 Subject: [PATCH 08/24] Improve example for documentation. --- docs/reference/esql/functions/kibana/definition/space.json | 2 +- docs/reference/esql/functions/kibana/docs/space.md | 2 +- .../esql/qa/testFixtures/src/main/resources/string.csv-spec | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reference/esql/functions/kibana/definition/space.json b/docs/reference/esql/functions/kibana/definition/space.json index d744e0cf3c201..9fe5fdf81d97f 100644 --- a/docs/reference/esql/functions/kibana/definition/space.json +++ b/docs/reference/esql/functions/kibana/definition/space.json @@ -18,6 +18,6 @@ } ], "examples" : [ - "ROW message = CONCAT(\"Hello\", SPACE(1), \"World!\");" + "ROW message = CONCAT(\"Hello\", SPACE(3), \"World!\");" ] } diff --git a/docs/reference/esql/functions/kibana/docs/space.md b/docs/reference/esql/functions/kibana/docs/space.md index 11c221a63472a..da5bf170a11fd 100644 --- a/docs/reference/esql/functions/kibana/docs/space.md +++ b/docs/reference/esql/functions/kibana/docs/space.md @@ -6,5 +6,5 @@ This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../READ Returns a string of `number` spaces. ``` -ROW message = CONCAT("Hello", SPACE(1), "World!"); +ROW message = CONCAT("Hello", SPACE(3), "World!"); ``` diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index f79aed5519d29..452c1368de03c 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1597,12 +1597,12 @@ emp_no:integer | languages:integer | first_name:keyword space // tag::space[] -ROW message = CONCAT("Hello", SPACE(1), "World!"); +ROW message = CONCAT("Hello", SPACE(3), "World!"); // end::space[] // tag::space-result[] message:keyword -Hello World! +Hello World! // end::space-result[] ; From 91f62b9280da1aad13d2f9765c360bf1483b36e9 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 13:44:38 +0200 Subject: [PATCH 09/24] Revert "Improve example for documentation." (Doc doesn't render extra spaces in the result) This reverts commit aed785d89137b014082e26c368006e805c2d0df6. --- docs/reference/esql/functions/kibana/definition/space.json | 2 +- docs/reference/esql/functions/kibana/docs/space.md | 2 +- .../esql/qa/testFixtures/src/main/resources/string.csv-spec | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reference/esql/functions/kibana/definition/space.json b/docs/reference/esql/functions/kibana/definition/space.json index 9fe5fdf81d97f..d744e0cf3c201 100644 --- a/docs/reference/esql/functions/kibana/definition/space.json +++ b/docs/reference/esql/functions/kibana/definition/space.json @@ -18,6 +18,6 @@ } ], "examples" : [ - "ROW message = CONCAT(\"Hello\", SPACE(3), \"World!\");" + "ROW message = CONCAT(\"Hello\", SPACE(1), \"World!\");" ] } diff --git a/docs/reference/esql/functions/kibana/docs/space.md b/docs/reference/esql/functions/kibana/docs/space.md index da5bf170a11fd..11c221a63472a 100644 --- a/docs/reference/esql/functions/kibana/docs/space.md +++ b/docs/reference/esql/functions/kibana/docs/space.md @@ -6,5 +6,5 @@ This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../READ Returns a string of `number` spaces. ``` -ROW message = CONCAT("Hello", SPACE(3), "World!"); +ROW message = CONCAT("Hello", SPACE(1), "World!"); ``` diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index 452c1368de03c..f79aed5519d29 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1597,12 +1597,12 @@ emp_no:integer | languages:integer | first_name:keyword space // tag::space[] -ROW message = CONCAT("Hello", SPACE(3), "World!"); +ROW message = CONCAT("Hello", SPACE(1), "World!"); // end::space[] // tag::space-result[] message:keyword -Hello World! +Hello World! // end::space-result[] ; From 1f4ec7a7d1c5e66fd943d6fe8d4f527c0b951bdd Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 14:16:51 +0200 Subject: [PATCH 10/24] Add new capability for function SPACE. --- .../qa/testFixtures/src/main/resources/string.csv-spec | 6 ++++++ .../elasticsearch/xpack/esql/action/EsqlCapabilities.java | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index f79aed5519d29..9fd1eafe814f6 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1596,6 +1596,7 @@ emp_no:integer | languages:integer | first_name:keyword ; space +required_capability: space // tag::space[] ROW message = CONCAT("Hello", SPACE(1), "World!"); // end::space[] @@ -1607,6 +1608,7 @@ Hello World! ; spaceLength +required_capability: space ROW len = LENGTH(SPACE(12)); len:integer @@ -1614,6 +1616,7 @@ len:integer ; spaceCalculated +required_capability: space ROW inner_width = 20, title = "Title" | EVAL lspace = SPACE((inner_width-LENGTH(title))/2), rspace = SPACE(inner_width-LENGTH(lspace)-LENGTH(title)), @@ -1625,6 +1628,7 @@ inner_width:integer | centered_title:keyword ; spaceZero +required_capability: space ROW s = SPACE(0); s:keyword @@ -1632,6 +1636,7 @@ s:keyword ; spaceNull +required_capability: space ROW s = SPACE(null); s:keyword @@ -1639,6 +1644,7 @@ null ; spaceNegative +required_capability: space FROM employees | SORT emp_no | LIMIT 1 | EVAL s = SPACE(-LENGTH(first_name)) | KEEP s; warning:Line 1:51: evaluation of [SPACE(-LENGTH(first_name))] failed, treating result as null. Only first 20 failures recorded. diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 81b2ba71b8808..563da55a5ea80 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -269,7 +269,12 @@ public enum Cap { /** * Allow mixed numeric types in coalesce */ - MIXED_NUMERIC_TYPES_IN_COALESCE; + MIXED_NUMERIC_TYPES_IN_COALESCE, + + /** + * Support for requesting the "SPACE" function. + */ + SPACE; private final boolean snapshotOnly; private final FeatureFlag featureFlag; From d2a452c2aaa619674f3925194fb47008bd3420d3 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 14:38:15 +0200 Subject: [PATCH 11/24] Update docs/changelog/112350.yaml --- docs/changelog/112350.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/112350.yaml diff --git a/docs/changelog/112350.yaml b/docs/changelog/112350.yaml new file mode 100644 index 0000000000000..994cd3a65c633 --- /dev/null +++ b/docs/changelog/112350.yaml @@ -0,0 +1,5 @@ +pr: 112350 +summary: "[ESQL] Add `SPACE` function" +area: ES|QL +type: enhancement +issues: [] From f76cf797ca23ce14c159f194ae2212de5c13a6d5 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 14:52:15 +0200 Subject: [PATCH 12/24] Fix style. --- .../xpack/esql/expression/function/scalar/string/Space.java | 3 --- .../esql/expression/function/scalar/string/SpaceTests.java | 6 ++++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 1136626eef28c..1029201dbabbc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -31,10 +31,7 @@ import static org.elasticsearch.common.unit.ByteSizeUnit.MB; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT; -import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.FIRST; -import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNumeric; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType; -import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isWholeNumber; public class Space extends UnaryScalarFunction { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Space", Space::new); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java index 932e0599be1db..a0346b90e1d85 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -69,14 +69,16 @@ public static Iterable parameters() { cases.add(new TestCaseSupplier("Space with number too large", List.of(DataType.INTEGER), () -> { int max = (int) MB.toBytes(1); - int number = randomIntBetween(max, max+10); + int number = randomIntBetween(max, max + 10); return new TestCaseSupplier.TestCase( List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), "SpaceEvaluator[number=Attribute[channel=0]]", DataType.KEYWORD, nullValue() ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.") - .withWarning("Line -1:-1: java.lang.IllegalArgumentException: Creating strings longer than [" + max + "] bytes is not supported") + .withWarning( + "Line -1:-1: java.lang.IllegalArgumentException: Creating strings longer than [" + max + "] bytes is not supported" + ) .withFoldingException(IllegalArgumentException.class, "Creating strings longer than [" + max + "] bytes is not supported"); })); From 8ec8c6ab4ab9a62d5484b1914df55d4f7d04b9c9 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 18:13:00 +0200 Subject: [PATCH 13/24] Test with input from an index. --- .../src/main/resources/string.csv-spec | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index 9fd1eafe814f6..9660592ecc823 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1627,6 +1627,21 @@ inner_width:integer | centered_title:keyword 20 | < Title > ; +spaceNumberFromIndex +required_capability: space +FROM employees +| EVAL s = CONCAT("<",SPACE(languages),">") +| WHERE emp_no < 10005 +| SORT emp_no +| KEEP emp_no, languages, s; + +emp_no:integer | languages:integer | s:keyword +10001 | 2 | < > +10002 | 5 | < > +10003 | 4 | < > +10004 | 5 | < > +; + spaceZero required_capability: space ROW s = SPACE(0); From b1047ec736045755fbad7e3c2a6b6a64aa7fee57 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 18:13:23 +0200 Subject: [PATCH 14/24] Add serialization tests. --- .../string/SpaceSerializationTests.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java new file mode 100644 index 0000000000000..ea4aa2495778e --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.expression.function.scalar.string; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.expression.AbstractExpressionSerializationTests; + +import java.io.IOException; + +public class SpaceSerializationTests extends AbstractExpressionSerializationTests { + @Override + protected Space createTestInstance() { + Source source = randomSource(); + Expression number = randomChild(); + return new Space(source, number); + } + + @Override + protected Space mutateInstance(Space instance) throws IOException { + Source source = instance.source(); + Expression number = instance.number(); + number = randomValueOtherThan(number, AbstractExpressionSerializationTests::randomChild); + return new Space(source, number); + } +} From f8042326dbcff1984243ac37713514e82a509f72 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 18:14:38 +0200 Subject: [PATCH 15/24] Add more testing advice to the guide to adding a new function. --- .../esql/expression/function/scalar/package-info.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java index b4781c7e41f98..f8b05aea324dc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java @@ -120,15 +120,17 @@ * Rerun the {@code CsvTests}. They should find your function and maybe even pass. Add a * few more tests in the csv-spec tests. They run quickly so it isn't a big deal having * half a dozen of them per function. In fact, it's useful to add more complex combinations - * of things here, just to catch any accidental strange interactions. For example, it is - * probably a good idea to have your function passes as a parameter to another function + * of things here, just to catch any accidental strange interactions. For example, have + * your function take its input from an index like {@code FROM employees | EVAL foo=MY_FUNCTION(emp_no)}. + * It's probably a good idea to have your function passed as a parameter to another function * like {@code EVAL foo=MOST(0, MY_FUNCTION(emp_no))}. And likely useful to try the reverse * like {@code EVAL foo=MY_FUNCTION(MOST(languages + 10000, emp_no)}. * *
  • * Now it's time to make a unit test! The infrastructure for these is under some flux at - * the moment, but it's good to extend from {@code AbstractScalarFunctionTestCase}. All of + * the moment, but it's good to extend {@code AbstractScalarFunctionTestCase}. All of * these tests are parameterized and expect to spend some time finding good parameters. + * Also add serialization tests that extend {@code AbstractExpressionSerializationTests<>}. *
    • *
  • * Once you are happy with the tests run the auto formatter: From 645730a611ca58aa14763640ead072c187395738 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 20:07:29 +0200 Subject: [PATCH 16/24] Remove constant evaluator in favor of building a literal. --- .../scalar/string/SpaceConstantEvaluator.java | 89 ------------------- .../function/scalar/string/Space.java | 27 +++--- 2 files changed, 10 insertions(+), 106 deletions(-) delete mode 100644 x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java deleted file mode 100644 index 7e9c4fafadddb..0000000000000 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceConstantEvaluator.java +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// or more contributor license agreements. Licensed under the Elastic License -// 2.0; you may not use this file except in compliance with the Elastic License -// 2.0. -package org.elasticsearch.xpack.esql.expression.function.scalar.string; - -import java.lang.Override; -import java.lang.String; -import java.util.function.Function; -import org.elasticsearch.compute.data.Block; -import org.elasticsearch.compute.data.BytesRefVector; -import org.elasticsearch.compute.data.Page; -import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; -import org.elasticsearch.compute.operator.DriverContext; -import org.elasticsearch.compute.operator.EvalOperator; -import org.elasticsearch.core.Releasables; -import org.elasticsearch.xpack.esql.core.tree.Source; -import org.elasticsearch.xpack.esql.expression.function.Warnings; - -/** - * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Space}. - * This class is generated. Do not edit it. - */ -public final class SpaceConstantEvaluator implements EvalOperator.ExpressionEvaluator { - private final Warnings warnings; - - private final BreakingBytesRefBuilder scratch; - - private final int number; - - private final DriverContext driverContext; - - public SpaceConstantEvaluator(Source source, BreakingBytesRefBuilder scratch, int number, - DriverContext driverContext) { - this.scratch = scratch; - this.number = number; - this.driverContext = driverContext; - this.warnings = Warnings.createWarnings(driverContext.warningsMode(), source); - } - - @Override - public Block eval(Page page) { - return eval(page.getPositionCount()).asBlock(); - } - - public BytesRefVector eval(int positionCount) { - try(BytesRefVector.Builder result = driverContext.blockFactory().newBytesRefVectorBuilder(positionCount)) { - position: for (int p = 0; p < positionCount; p++) { - result.appendBytesRef(Space.processConstant(this.scratch, this.number)); - } - return result.build(); - } - } - - @Override - public String toString() { - return "SpaceConstantEvaluator[" + "number=" + number + "]"; - } - - @Override - public void close() { - Releasables.closeExpectNoException(scratch); - } - - static class Factory implements EvalOperator.ExpressionEvaluator.Factory { - private final Source source; - - private final Function scratch; - - private final int number; - - public Factory(Source source, Function scratch, - int number) { - this.source = source; - this.scratch = scratch; - this.number = number; - } - - @Override - public SpaceConstantEvaluator get(DriverContext context) { - return new SpaceConstantEvaluator(source, scratch.apply(context), number, context); - } - - @Override - public String toString() { - return "SpaceConstantEvaluator[" + "number=" + number + "]"; - } - } -} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 1029201dbabbc..32304bb01a284 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -16,6 +16,7 @@ import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator; import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -32,6 +33,7 @@ import static org.elasticsearch.common.unit.ByteSizeUnit.MB; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isType; +import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; public class Space extends UnaryScalarFunction { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Space", Space::new); @@ -70,7 +72,7 @@ public String getWriteableName() { @Override public DataType dataType() { - return DataType.KEYWORD; + return KEYWORD; } @Override @@ -87,15 +89,15 @@ public boolean foldable() { return number.foldable(); } - @Evaluator(extraName = "Constant") - static BytesRef processConstant(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, @Fixed int number) { - return processInner(scratch, number); - } - @Evaluator(warnExceptions = { IllegalArgumentException.class }) static BytesRef process(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, int number) { checkNumber(number); - return processInner(scratch, number); + scratch.grow(number); + scratch.clear(); + for (int i = 0; i < number; ++i) { + scratch.append((byte) ' '); + } + return scratch.bytesRefView(); } static void checkNumber(int number) { @@ -107,15 +109,6 @@ static void checkNumber(int number) { } } - static BytesRef processInner(BreakingBytesRefBuilder scratch, int number) { - scratch.grow(number); - scratch.clear(); - for (int i = 0; i < number; ++i) { - scratch.append((byte) ' '); - } - return scratch.bytesRefView(); - } - @Override public Expression replaceChildren(List newChildren) { return new Space(source(), newChildren.get(0)); @@ -131,7 +124,7 @@ public ExpressionEvaluator.Factory toEvaluator(Function new BreakingBytesRefBuilder(context.breaker(), "space"), num); + return toEvaluator.apply(new Literal(source(), " ".repeat(num), KEYWORD)); } ExpressionEvaluator.Factory numberExpr = toEvaluator.apply(number); From 054042334b9340171b24b87e3823a52c1a3c23ac Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Thu, 29 Aug 2024 20:47:18 +0200 Subject: [PATCH 17/24] Use `field` in the superclass rather than adding `number`. --- .../function/scalar/string/Space.java | 23 +++++-------------- .../string/SpaceSerializationTests.java | 2 +- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 32304bb01a284..454ca7b0909b9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -40,8 +40,6 @@ public class Space extends UnaryScalarFunction { static final long MAX_LENGTH = MB.toBytes(1); - private final Expression number; - @FunctionInfo( returnType = "keyword", description = "Returns a string of `number` spaces.", @@ -52,7 +50,6 @@ public Space( @Param(name = "number", type = { "integer" }, description = "Number of spaces in result.") Expression number ) { super(source, number); - this.number = number; } private Space(StreamInput in) throws IOException { @@ -62,7 +59,7 @@ private Space(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { source().writeTo(out); - out.writeNamedWriteable(number); + out.writeNamedWriteable(field); } @Override @@ -81,12 +78,7 @@ protected TypeResolution resolveType() { return new TypeResolution("Unresolved children"); } - return isType(number, dt -> dt == DataType.INTEGER, sourceText(), DEFAULT, "integer"); - } - - @Override - public boolean foldable() { - return number.foldable(); + return isType(field, dt -> dt == DataType.INTEGER, sourceText(), DEFAULT, "integer"); } @Evaluator(warnExceptions = { IllegalArgumentException.class }) @@ -116,22 +108,19 @@ public Expression replaceChildren(List newChildren) { @Override protected NodeInfo info() { - return NodeInfo.create(this, Space::new, number); + return NodeInfo.create(this, Space::new, field); } @Override public ExpressionEvaluator.Factory toEvaluator(Function toEvaluator) { - if (number.foldable()) { - int num = (int) number.fold(); + if (field.foldable()) { + int num = (int) field.fold(); checkNumber(num); return toEvaluator.apply(new Literal(source(), " ".repeat(num), KEYWORD)); } - ExpressionEvaluator.Factory numberExpr = toEvaluator.apply(number); + ExpressionEvaluator.Factory numberExpr = toEvaluator.apply(field); return new SpaceEvaluator.Factory(source(), context -> new BreakingBytesRefBuilder(context.breaker(), "space"), numberExpr); } - Expression number() { - return number; - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java index ea4aa2495778e..bf3b15145e0f3 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceSerializationTests.java @@ -24,7 +24,7 @@ protected Space createTestInstance() { @Override protected Space mutateInstance(Space instance) throws IOException { Source source = instance.source(); - Expression number = instance.number(); + Expression number = instance.field(); number = randomValueOtherThan(number, AbstractExpressionSerializationTests::randomChild); return new Space(source, number); } From bd688eeb172e44e6be84336aebf0f72e1c2b1e30 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Fri, 30 Aug 2024 10:54:31 +0200 Subject: [PATCH 18/24] Fix test. --- .../esql/expression/function/scalar/string/SpaceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java index a0346b90e1d85..4979c7cc92c98 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -69,7 +69,7 @@ public static Iterable parameters() { cases.add(new TestCaseSupplier("Space with number too large", List.of(DataType.INTEGER), () -> { int max = (int) MB.toBytes(1); - int number = randomIntBetween(max, max + 10); + int number = randomIntBetween(max + 1, max + 10); return new TestCaseSupplier.TestCase( List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), "SpaceEvaluator[number=Attribute[channel=0]]", From 74ac4cc43ecc4936f7aac39bc24a3d348065603d Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 14:28:11 +0200 Subject: [PATCH 19/24] Simplify test setup. --- .../function/scalar/string/SpaceTests.java | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java index 4979c7cc92c98..308ce2c9d932f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/SpaceTests.java @@ -22,7 +22,6 @@ import java.util.function.Supplier; import static org.elasticsearch.common.unit.ByteSizeUnit.MB; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; public class SpaceTests extends AbstractScalarFunctionTestCase { @@ -35,25 +34,15 @@ public static Iterable parameters() { List cases = new ArrayList<>(); - cases.add(new TestCaseSupplier("Space basic test", List.of(DataType.INTEGER), () -> { - int number = between(0, 10); - return new TestCaseSupplier.TestCase( - List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), - "SpaceEvaluator[number=Attribute[channel=0]]", - DataType.KEYWORD, - equalTo(new BytesRef(" ".repeat(number))) - ); - })); - - cases.add(new TestCaseSupplier("Space with number zero", List.of(DataType.INTEGER), () -> { - int number = 0; - return new TestCaseSupplier.TestCase( - List.of(new TestCaseSupplier.TypedData(number, DataType.INTEGER, "number")), - "SpaceEvaluator[number=Attribute[channel=0]]", - DataType.KEYWORD, - equalTo(new BytesRef("")) - ); - })); + TestCaseSupplier.forUnaryInt( + cases, + "SpaceEvaluator[number=Attribute[channel=0]]", + DataType.KEYWORD, + i -> new BytesRef(" ".repeat(i)), + 0, + 10, + List.of() + ); cases.add(new TestCaseSupplier("Space with negative number", List.of(DataType.INTEGER), () -> { int number = randomIntBetween(-10, -1); @@ -82,9 +71,7 @@ public static Iterable parameters() { .withFoldingException(IllegalArgumentException.class, "Creating strings longer than [" + max + "] bytes is not supported"); })); - cases = anyNullIsNull(true, cases); - cases = errorsForCasesWithoutExamples(cases, (v, p) -> "integer"); - return parameterSuppliersFromTypedData(cases); + return parameterSuppliersFromTypedDataWithDefaultChecks(true, cases, (v, p) -> "integer"); } @Override From f7d64358fbfba1b35f55e977dd2fb46d0599fbb3 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 14:29:54 +0200 Subject: [PATCH 20/24] Bulk fill with Arrays.fill rather than a loop. --- .../esql/expression/function/scalar/string/Space.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 454ca7b0909b9..3d584d6fb6be1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -27,6 +27,7 @@ import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.function.Function; @@ -85,10 +86,8 @@ protected TypeResolution resolveType() { static BytesRef process(@Fixed(includeInToString = false, build = true) BreakingBytesRefBuilder scratch, int number) { checkNumber(number); scratch.grow(number); - scratch.clear(); - for (int i = 0; i < number; ++i) { - scratch.append((byte) ' '); - } + scratch.setLength(number); + Arrays.fill(scratch.bytes(), 0, number, (byte) ' '); return scratch.bytesRefView(); } From 98e5cefef5c80a44996a3ed22b021cb60ed21f76 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 16:48:48 +0200 Subject: [PATCH 21/24] Update description as suggested. --- docs/reference/esql/functions/description/space.asciidoc | 2 +- docs/reference/esql/functions/kibana/definition/space.json | 2 +- docs/reference/esql/functions/kibana/docs/space.md | 2 +- .../esql/qa/testFixtures/src/main/resources/meta.csv-spec | 2 +- .../xpack/esql/expression/function/scalar/string/Space.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/reference/esql/functions/description/space.asciidoc b/docs/reference/esql/functions/description/space.asciidoc index cf85c4a9dfafd..ee01da64f590f 100644 --- a/docs/reference/esql/functions/description/space.asciidoc +++ b/docs/reference/esql/functions/description/space.asciidoc @@ -2,4 +2,4 @@ *Description* -Returns a string of `number` spaces. +Returns a string made of `number` spaces. diff --git a/docs/reference/esql/functions/kibana/definition/space.json b/docs/reference/esql/functions/kibana/definition/space.json index d744e0cf3c201..acf7466284d3b 100644 --- a/docs/reference/esql/functions/kibana/definition/space.json +++ b/docs/reference/esql/functions/kibana/definition/space.json @@ -2,7 +2,7 @@ "comment" : "This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.", "type" : "eval", "name" : "space", - "description" : "Returns a string of `number` spaces.", + "description" : "Returns a string made of `number` spaces.", "signatures" : [ { "params" : [ diff --git a/docs/reference/esql/functions/kibana/docs/space.md b/docs/reference/esql/functions/kibana/docs/space.md index 11c221a63472a..3112bf953dd65 100644 --- a/docs/reference/esql/functions/kibana/docs/space.md +++ b/docs/reference/esql/functions/kibana/docs/space.md @@ -3,7 +3,7 @@ This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../READ --> ### SPACE -Returns a string of `number` spaces. +Returns a string made of `number` spaces. ``` ROW message = CONCAT("Hello", SPACE(1), "World!"); diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec index b5ec41c8f050f..25ca86d545513 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec @@ -322,7 +322,7 @@ rtrim |Removes trailing whitespaces from a string. signum |Returns the sign of the given number. It returns `-1` for negative numbers, `0` for `0` and `1` for positive numbers. sin |Returns ths {wikipedia}/Sine_and_cosine[Sine] trigonometric function of an angle. sinh |Returns the {wikipedia}/Hyperbolic_functions[hyperbolic sine] of an angle. -space |Returns a string of `number` spaces. +space |Returns a string made of `number` spaces. split |Split a single valued string into multiple strings. sqrt |Returns the square root of a number. The input can be any numeric value, the return value is always a double. Square roots of negative numbers and infinities are null. st_centroid_ag|Calculate the spatial centroid over a field with spatial point geometry type. diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 3d584d6fb6be1..e9c07f5892f95 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -43,7 +43,7 @@ public class Space extends UnaryScalarFunction { @FunctionInfo( returnType = "keyword", - description = "Returns a string of `number` spaces.", + description = "Returns a string made of `number` spaces.", examples = @Example(file = "string", tag = "space") ) public Space( From 8a6c21618306dd266d46522758ede7f156b669f3 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 16:49:53 +0200 Subject: [PATCH 22/24] Make Space.MAX_LENGTH private. --- .../xpack/esql/expression/function/scalar/string/Space.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index e9c07f5892f95..0ee7d2f73fa6b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -39,7 +39,7 @@ public class Space extends UnaryScalarFunction { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Space", Space::new); - static final long MAX_LENGTH = MB.toBytes(1); + private static final long MAX_LENGTH = MB.toBytes(1); @FunctionInfo( returnType = "keyword", From d03fef32660b61c8a7c03172ba64884b7f22355e Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 16:50:34 +0200 Subject: [PATCH 23/24] Add more complex test. --- .../testFixtures/src/main/resources/string.csv-spec | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index 9660592ecc823..7272d8da11f8a 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1668,3 +1668,16 @@ warning:Line 1:51: java.lang.IllegalArgumentException: Number parameter cannot b s:keyword null ; + +spaceComplex +required_capability: space +ROW x = 1+2+3, y = null | EVAL z = x + 5 | LIMIT 1 +| EVAL xs = SPACE(x), xsc = CONCAT("<",xs,">"), + xn = SPACE(null), + xz = SPACE(z), xzc = CONCAT("<",xz,">"), + lxz = LENGTH(xz), lxs = LENGTH(xs), ltr = LENGTH(TRIM(xz)) +| DROP xs, xz; + +x:integer | y:null | z:integer | xsc:keyword | xn:keyword | xzc:keyword | lxz:integer | lxs:integer | ltr:integer +6 | null | 11 | < > | null | < > | 11 | 6 | 0 +; From 8e4f1c5a3c9004f3b8c70a1c8558479cf3112fc4 Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sat, 31 Aug 2024 16:52:22 +0200 Subject: [PATCH 24/24] Return null and warnings for a multi-value parameter. --- .../testFixtures/src/main/resources/string.csv-spec | 11 +++++++++++ .../esql/expression/function/scalar/string/Space.java | 8 +++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index 7272d8da11f8a..ffcceab26bcaf 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -1681,3 +1681,14 @@ ROW x = 1+2+3, y = null | EVAL z = x + 5 | LIMIT 1 x:integer | y:null | z:integer | xsc:keyword | xn:keyword | xzc:keyword | lxz:integer | lxs:integer | ltr:integer 6 | null | 11 | < > | null | < > | 11 | 6 | 0 ; + +spaceMV +required_capability: space +ROW mv = [1,2,3] | EVAL x = space(mv) | KEEP x; + +warning:Line 1:29: evaluation of [space(mv)] failed, treating result as null. Only first 20 failures recorded. +warning:Line 1:29: java.lang.IllegalArgumentException: single-value function encountered multi-value + +x:keyword +null +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java index 0ee7d2f73fa6b..e6225a008fceb 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java @@ -113,9 +113,11 @@ protected NodeInfo info() { @Override public ExpressionEvaluator.Factory toEvaluator(Function toEvaluator) { if (field.foldable()) { - int num = (int) field.fold(); - checkNumber(num); - return toEvaluator.apply(new Literal(source(), " ".repeat(num), KEYWORD)); + Object folded = field.fold(); + if (folded instanceof Integer num) { + checkNumber(num); + return toEvaluator.apply(new Literal(source(), " ".repeat(num), KEYWORD)); + } } ExpressionEvaluator.Factory numberExpr = toEvaluator.apply(field);