Skip to content

Commit e7ccfc2

Browse files
author
MaxKsyunz
committed
Address PR feedback
Remove FunctionProperties from ExpressionConfig and create it when create<LANG>Service is called. This ensures it's created for each query. Created FunctionPropertiesTestConfig to simplify updating unit tests to work with the change above. Removed an infrequently-used version of BuiltinFunctionRepository.compile and @Getter on BuiltinFunctionRepository.functionProperties -- it was only used for a test -- ExpressionConfigTest. Removed the test because it was testing Spring behavior and not very valuable. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
1 parent 703275a commit e7ccfc2

32 files changed

Lines changed: 127 additions & 99 deletions

File tree

‎core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.opensearch.sql.expression.system.SystemFunctions;
2626
import org.opensearch.sql.expression.text.TextFunction;
2727
import org.opensearch.sql.expression.window.WindowFunctions;
28+
import org.springframework.beans.factory.annotation.Autowired;
2829
import org.springframework.context.annotation.Bean;
2930
import org.springframework.context.annotation.Configuration;
3031

@@ -56,10 +57,8 @@ public BuiltinFunctionRepository functionRepository(FunctionProperties functionC
5657
return builtinFunctionRepository;
5758
}
5859

59-
@Bean
60-
public FunctionProperties functionExecutionContext() {
61-
return new FunctionProperties(Instant.now(), ZoneId.systemDefault());
62-
}
60+
@Autowired
61+
FunctionProperties functionProperties;
6362

6463
@Bean
6564
public DSL dsl(BuiltinFunctionRepository repository) {

‎core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ public class BuiltinFunctionRepository {
3636

3737
private final Map<String, Map<FunctionName, FunctionResolver>> namespaceFunctionResolverMap;
3838

39-
@Getter
4039
private final FunctionProperties functionProperties;
4140

4241

@@ -65,12 +64,6 @@ public void register(String namespace, FunctionResolver resolver) {
6564
namespaceFunctionResolverMap.get(namespace).put(resolver.getFunctionName(), resolver);
6665
}
6766

68-
69-
public FunctionImplementation compile(BuiltinFunctionName functionName,
70-
List<Expression> expressions) {
71-
return compile(functionName.getName(), expressions);
72-
}
73-
7467
/**
7568
* Compile FunctionExpression under default namespace.
7669
*

‎core/src/main/java/org/opensearch/sql/expression/function/FunctionProperties.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ public class FunctionProperties implements Serializable {
2323
private final Instant nowInstant;
2424
private final ZoneId currentZoneId;
2525

26+
/**
27+
* By default, use current time and current timezone.
28+
*/
29+
public FunctionProperties() {
30+
nowInstant = Instant.now();
31+
currentZoneId = ZoneId.systemDefault();
32+
}
2633

2734
/**
2835
* Method to access current system clock.

‎core/src/main/java/org/opensearch/sql/expression/function/FunctionPropertiesTestConfig.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.expression.function;
7+
8+
import org.springframework.context.annotation.Bean;
9+
import org.springframework.context.annotation.Configuration;
10+
11+
@Configuration
12+
public class FunctionPropertiesTestConfig {
13+
@Bean
14+
FunctionProperties functionProperties() {
15+
return new FunctionProperties();
16+
}
17+
}

‎core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import org.opensearch.sql.expression.DSL;
8282
import org.opensearch.sql.expression.HighlightExpression;
8383
import org.opensearch.sql.expression.config.ExpressionConfig;
84+
import org.opensearch.sql.expression.function.FunctionPropertiesTestConfig;
8485
import org.opensearch.sql.expression.window.WindowDefinition;
8586
import org.opensearch.sql.planner.logical.LogicalAD;
8687
import org.opensearch.sql.planner.logical.LogicalMLCommons;
@@ -96,7 +97,8 @@
9697

9798
@Configuration
9899
@ExtendWith(SpringExtension.class)
99-
@ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTest.class})
100+
@ContextConfiguration(classes = {FunctionPropertiesTestConfig.class,
101+
ExpressionConfig.class, AnalyzerTest.class})
100102
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
101103
class AnalyzerTest extends AnalyzerTestBase {
102104

‎core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
3434
import org.opensearch.sql.expression.function.FunctionBuilder;
3535
import org.opensearch.sql.expression.function.FunctionName;
36+
import org.opensearch.sql.expression.function.FunctionProperties;
3637
import org.opensearch.sql.expression.function.FunctionResolver;
3738
import org.opensearch.sql.expression.function.FunctionSignature;
3839
import org.opensearch.sql.expression.function.TableFunctionImplementation;

‎core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,17 @@
4848
import org.opensearch.sql.expression.DSL;
4949
import org.opensearch.sql.expression.Expression;
5050
import org.opensearch.sql.expression.FunctionExpression;
51-
import org.opensearch.sql.expression.LiteralExpression;
5251
import org.opensearch.sql.expression.config.ExpressionConfig;
52+
import org.opensearch.sql.expression.function.FunctionPropertiesTestConfig;
5353
import org.opensearch.sql.expression.window.aggregation.AggregateWindowFunction;
5454
import org.springframework.context.annotation.Configuration;
5555
import org.springframework.test.context.ContextConfiguration;
5656
import org.springframework.test.context.junit.jupiter.SpringExtension;
5757

5858
@Configuration
5959
@ExtendWith(SpringExtension.class)
60-
@ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTestBase.class})
60+
@ContextConfiguration(classes = {FunctionPropertiesTestConfig.class, ExpressionConfig.class,
61+
AnalyzerTestBase.class})
6162
class ExpressionAnalyzerTest extends AnalyzerTestBase {
6263

6364
@Test

‎core/src/test/java/org/opensearch/sql/analysis/ExpressionReferenceOptimizerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.opensearch.sql.expression.DSL;
1919
import org.opensearch.sql.expression.Expression;
2020
import org.opensearch.sql.expression.config.ExpressionConfig;
21+
import org.opensearch.sql.expression.function.FunctionPropertiesTestConfig;
2122
import org.opensearch.sql.expression.window.WindowDefinition;
2223
import org.opensearch.sql.planner.logical.LogicalPlan;
2324
import org.opensearch.sql.planner.logical.LogicalPlanDSL;
@@ -27,7 +28,8 @@
2728

2829
@Configuration
2930
@ExtendWith(SpringExtension.class)
30-
@ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTest.class})
31+
@ContextConfiguration(classes = {FunctionPropertiesTestConfig.class, ExpressionConfig.class,
32+
AnalyzerTest.class})
3133
class ExpressionReferenceOptimizerTest extends AnalyzerTestBase {
3234

3335
@Test

‎core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@
1818
import org.opensearch.sql.ast.expression.Literal;
1919
import org.opensearch.sql.expression.NamedExpression;
2020
import org.opensearch.sql.expression.config.ExpressionConfig;
21+
import org.opensearch.sql.expression.function.FunctionPropertiesTestConfig;
2122
import org.springframework.context.annotation.Configuration;
2223
import org.springframework.test.context.ContextConfiguration;
2324
import org.springframework.test.context.junit.jupiter.SpringExtension;
2425

2526
@Configuration
2627
@ExtendWith(SpringExtension.class)
27-
@ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTestBase.class})
28+
@ContextConfiguration(classes = {FunctionPropertiesTestConfig.class, ExpressionConfig.class,
29+
AnalyzerTestBase.class})
2830
class NamedExpressionAnalyzerTest extends AnalyzerTestBase {
2931
@Test
3032
void visit_named_select_item() {

‎core/src/test/java/org/opensearch/sql/analysis/QualifierAnalyzerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
import org.opensearch.sql.common.antlr.SyntaxCheckException;
2020
import org.opensearch.sql.data.type.ExprType;
2121
import org.opensearch.sql.expression.config.ExpressionConfig;
22+
import org.opensearch.sql.expression.function.FunctionPropertiesTestConfig;
2223
import org.springframework.context.annotation.Configuration;
2324
import org.springframework.test.context.ContextConfiguration;
2425
import org.springframework.test.context.junit.jupiter.SpringExtension;
2526

2627
@Configuration
2728
@ExtendWith(SpringExtension.class)
28-
@ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTestBase.class})
29+
@ContextConfiguration(classes = {FunctionPropertiesTestConfig.class, ExpressionConfig.class,
30+
AnalyzerTestBase.class})
2931
class QualifierAnalyzerTest extends AnalyzerTestBase {
3032

3133
private QualifierAnalyzer qualifierAnalyzer;

0 commit comments

Comments
 (0)