Skip to content

Commit 321c3c3

Browse files
authored
ESQL: Fix function registry concurrency issues on constructor (elastic#123492) (elastic#123584)
Fixes elastic#123430 There were 2 problems here: - We were filling a static field (used to auto-cast string literals) within a constructor, which is also called in multiple places - The field was only filled with non-snapshot functions, so snapshot function auto-casting wasn't possible Fixed both bugs by making the field non-static instead, and a fix to use the snapshot registry (if available) in the string casting rule.
1 parent 610b844 commit 321c3c3

File tree

4 files changed

+24
-16
lines changed

4 files changed

+24
-16
lines changed

docs/changelog/123492.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 123492
2+
summary: Fix function registry concurrency issues on constructor
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 123430

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,7 @@ private static class ImplicitCasting extends ParameterizedRule<LogicalPlan, Logi
11301130
public LogicalPlan apply(LogicalPlan plan, AnalyzerContext context) {
11311131
return plan.transformExpressionsUp(
11321132
org.elasticsearch.xpack.esql.core.expression.function.Function.class,
1133-
e -> ImplicitCasting.cast(e, context.functionRegistry())
1133+
e -> ImplicitCasting.cast(e, context.functionRegistry().snapshotRegistry())
11341134
);
11351135
}
11361136

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@
188188

189189
public class EsqlFunctionRegistry {
190190

191-
private static final Map<Class<? extends Function>, List<DataType>> DATA_TYPES_FOR_STRING_LITERAL_CONVERSIONS = new LinkedHashMap<>();
192-
193191
private static final Map<DataType, Integer> DATA_TYPE_CASTING_PRIORITY;
194192

195193
static {
@@ -225,6 +223,7 @@ public class EsqlFunctionRegistry {
225223
private final Map<String, FunctionDefinition> defs = new LinkedHashMap<>();
226224
private final Map<String, String> aliases = new HashMap<>();
227225
private final Map<Class<? extends Function>, String> names = new HashMap<>();
226+
private final Map<Class<? extends Function>, List<DataType>> dataTypesForStringLiteralConversions = new LinkedHashMap<>();
228227

229228
private SnapshotFunctionRegistry snapshotRegistry = null;
230229

@@ -711,20 +710,8 @@ private static Constructor<?> constructorFor(Class<? extends Function> clazz) {
711710
return constructors[0];
712711
}
713712

714-
private void buildDataTypesForStringLiteralConversion(FunctionDefinition[]... groupFunctions) {
715-
for (FunctionDefinition[] group : groupFunctions) {
716-
for (FunctionDefinition def : group) {
717-
FunctionDescription signature = description(def);
718-
DATA_TYPES_FOR_STRING_LITERAL_CONVERSIONS.put(
719-
def.clazz(),
720-
signature.args().stream().map(EsqlFunctionRegistry.ArgSignature::targetDataType).collect(Collectors.toList())
721-
);
722-
}
723-
}
724-
}
725-
726713
public List<DataType> getDataTypeForStringLiteralConversion(Class<? extends Function> clazz) {
727-
return DATA_TYPES_FOR_STRING_LITERAL_CONVERSIONS.get(clazz);
714+
return dataTypesForStringLiteralConversions.get(clazz);
728715
}
729716

730717
private static class SnapshotFunctionRegistry extends EsqlFunctionRegistry {
@@ -733,6 +720,7 @@ private static class SnapshotFunctionRegistry extends EsqlFunctionRegistry {
733720
throw new IllegalStateException("build snapshot function registry for non-snapshot build");
734721
}
735722
register(snapshotFunctions());
723+
buildDataTypesForStringLiteralConversion(snapshotFunctions());
736724
}
737725

738726
}
@@ -793,6 +781,18 @@ void register(FunctionDefinition... functions) {
793781
);
794782
}
795783

784+
protected void buildDataTypesForStringLiteralConversion(FunctionDefinition[]... groupFunctions) {
785+
for (FunctionDefinition[] group : groupFunctions) {
786+
for (FunctionDefinition def : group) {
787+
FunctionDescription signature = description(def);
788+
dataTypesForStringLiteralConversions.put(
789+
def.clazz(),
790+
signature.args().stream().map(EsqlFunctionRegistry.ArgSignature::targetDataType).collect(Collectors.toList())
791+
);
792+
}
793+
}
794+
}
795+
796796
protected FunctionDefinition cloneDefinition(String name, FunctionDefinition definition) {
797797
return new FunctionDefinition(name, emptyList(), definition.clazz(), definition.builder());
798798
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.xpack.esql.core.tree.Source;
2222
import org.elasticsearch.xpack.esql.core.type.DataType;
2323
import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper;
24+
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
2425
import org.elasticsearch.xpack.esql.expression.function.Param;
2526
import org.elasticsearch.xpack.esql.expression.function.scalar.UnaryScalarFunction;
2627

@@ -38,6 +39,7 @@
3839
public class Delay extends UnaryScalarFunction {
3940
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Delay", Delay::new);
4041

42+
@FunctionInfo(returnType = { "boolean" }, description = "Sleeps for a duration for every row. For debug purposes only.")
4143
public Delay(Source source, @Param(name = "ms", type = { "time_duration" }, description = "For how long") Expression ms) {
4244
super(source, ms);
4345
}

0 commit comments

Comments
 (0)