Use query execution start time as the value of now-like functions. (#149

) - Add FunctionProperties interface to capture query metadata and provide to function implementations. - Update FunctionBuilder to take FunctionProperties in addition to function arguments when evaluating a SQL function. - Implement now-like functions using FunctionProperties. - Add FunctionDSL.nullMissingHandlingWithProperties to allow for consistent null and missing value handling across all functions. - Remove constant value caching from ExpressionAnalyzer. It is no longer necessary in ExpressionAnalyzer -- the same behavior is now implemented with FunctionProperties. ### Unit Tests - Adjust getQueryStartClock_differs_from_instantNow unit test. On Windows, getQueryStartClock_differs_from_instantNow fails because Instant.now() in the test returns the same value as Instant.now() called for FunctionProperties construction. - Add unit tests for FunctionDSL. - Use Spring to instantiate dsl in OpenSearchTestBase. Signed-off-by: MaxKsyunz <[email protected]>
opensearch-project · Nov 8, 2022 · 6614306 · 6614306
1 parent 8c91689
commit 6614306
Show file tree

Hide file tree

Showing 76 changed files with 1,903 additions and 1,045 deletions.
diff --git a/core/src/main/java/org/opensearch/sql/analysis/AnalysisContext.java b/core/src/main/java/org/opensearch/sql/analysis/AnalysisContext.java
@@ -7,12 +7,9 @@
 package org.opensearch.sql.analysis;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 import lombok.Getter;
-import org.opensearch.sql.expression.Expression;
 import org.opensearch.sql.expression.NamedExpression;
 
 /**
@@ -26,14 +23,6 @@ public class AnalysisContext {
   @Getter
   private final List<NamedExpression> namedParseExpressions;
 
-  /**
-   * Storage for values of functions which return a constant value.
-   * We are storing the values there to use it in sequential calls to those functions.
-   * For example, `now` function should the same value during processing a query.
-   */
-  @Getter
-  private final Map<String, Expression> constantFunctionValues;
-
   public AnalysisContext() {
     this(new TypeEnvironment(null));
   }
@@ -45,7 +34,6 @@ public AnalysisContext() {
   public AnalysisContext(TypeEnvironment environment) {
     this.environment = environment;
     this.namedParseExpressions = new ArrayList<>();
-    this.constantFunctionValues = new HashMap<>();
   }
 
   /**

diff --git a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java
@@ -22,11 +22,9 @@
 import org.opensearch.sql.ast.expression.AggregateFunction;
 import org.opensearch.sql.ast.expression.AllFields;
 import org.opensearch.sql.ast.expression.And;
-import org.opensearch.sql.ast.expression.Argument;
 import org.opensearch.sql.ast.expression.Case;
 import org.opensearch.sql.ast.expression.Cast;
 import org.opensearch.sql.ast.expression.Compare;
-import org.opensearch.sql.ast.expression.ConstantFunction;
 import org.opensearch.sql.ast.expression.EqualTo;
 import org.opensearch.sql.ast.expression.Field;
 import org.opensearch.sql.ast.expression.Function;
@@ -176,19 +174,6 @@ public Expression visitRelevanceFieldList(RelevanceFieldList node, AnalysisConte
         ImmutableMap.copyOf(node.getFieldList())));
   }
 
-  @Override
-  public Expression visitConstantFunction(ConstantFunction node, AnalysisContext context) {
-    var valueName = node.getFuncName();
-    if (context.getConstantFunctionValues().containsKey(valueName)) {
-      return context.getConstantFunctionValues().get(valueName);
-    }
-
-    var value = visitFunction(node, context);
-    value = DSL.literal(value.valueOf(null));
-    context.getConstantFunctionValues().put(valueName, value);
-    return value;
-  }
-
   @Override
   public Expression visitFunction(Function node, AnalysisContext context) {
     FunctionName functionName = FunctionName.of(node.getFuncName());

diff --git a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java
@@ -15,7 +15,6 @@
 import org.opensearch.sql.ast.expression.Case;
 import org.opensearch.sql.ast.expression.Cast;
 import org.opensearch.sql.ast.expression.Compare;
-import org.opensearch.sql.ast.expression.ConstantFunction;
 import org.opensearch.sql.ast.expression.EqualTo;
 import org.opensearch.sql.ast.expression.Field;
 import org.opensearch.sql.ast.expression.Function;
@@ -123,10 +122,6 @@ public T visitRelevanceFieldList(RelevanceFieldList node, C context) {
     return visitChildren(node, context);
   }
 
-  public T visitConstantFunction(ConstantFunction node, C context) {
-    return visitChildren(node, context);
-  }
-
   public T visitUnresolvedAttribute(UnresolvedAttribute node, C context) {
     return visitChildren(node, context);
   }

diff --git a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java
@@ -19,7 +19,6 @@
 import org.opensearch.sql.ast.expression.Case;
 import org.opensearch.sql.ast.expression.Cast;
 import org.opensearch.sql.ast.expression.Compare;
-import org.opensearch.sql.ast.expression.ConstantFunction;
 import org.opensearch.sql.ast.expression.DataType;
 import org.opensearch.sql.ast.expression.EqualTo;
 import org.opensearch.sql.ast.expression.Field;
@@ -246,10 +245,6 @@ public static Function function(String funcName, UnresolvedExpression... funcArg
     return new Function(funcName, Arrays.asList(funcArgs));
   }
 
-  public static Function constantFunction(String funcName, UnresolvedExpression... funcArgs) {
-    return new ConstantFunction(funcName, Arrays.asList(funcArgs));
-  }
-
   /**
    * CASE
    *     WHEN search_condition THEN result_expr

diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/ConstantFunction.java b/core/src/main/java/org/opensearch/sql/ast/expression/ConstantFunction.java
diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java
@@ -124,7 +124,7 @@ public static NamedArgumentExpression namedArgument(String argName, Expression v
     return new NamedArgumentExpression(argName, value);
   }
 
-  public NamedArgumentExpression namedArgument(String name, String value) {
+  public static NamedArgumentExpression namedArgument(String name, String value) {
     return namedArgument(name, literal(value));
   }
 

diff --git a/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java b/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java
@@ -68,7 +68,7 @@ private static DefaultFunctionResolver avg() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> new AvgAggregator(arguments, DOUBLE))
+                (functionProperties, arguments) -> new AvgAggregator(arguments, DOUBLE))
             .build()
     );
   }
@@ -78,7 +78,7 @@ private static DefaultFunctionResolver count() {
     DefaultFunctionResolver functionResolver = new DefaultFunctionResolver(functionName,
         ExprCoreType.coreTypes().stream().collect(Collectors.toMap(
           type -> new FunctionSignature(functionName, Collections.singletonList(type)),
-          type -> arguments -> new CountAggregator(arguments, INTEGER))));
+          type -> (functionProperties, arguments) -> new CountAggregator(arguments, INTEGER))));
     return functionResolver;
   }
 
@@ -88,13 +88,13 @@ private static DefaultFunctionResolver sum() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(INTEGER)),
-                arguments -> new SumAggregator(arguments, INTEGER))
+                (functionProperties, arguments) -> new SumAggregator(arguments, INTEGER))
             .put(new FunctionSignature(functionName, Collections.singletonList(LONG)),
-                arguments -> new SumAggregator(arguments, LONG))
+                (functionProperties, arguments) -> new SumAggregator(arguments, LONG))
             .put(new FunctionSignature(functionName, Collections.singletonList(FLOAT)),
-                arguments -> new SumAggregator(arguments, FLOAT))
+                (functionProperties, arguments) -> new SumAggregator(arguments, FLOAT))
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> new SumAggregator(arguments, DOUBLE))
+                (functionProperties, arguments) -> new SumAggregator(arguments, DOUBLE))
             .build()
     );
   }
@@ -105,23 +105,23 @@ private static DefaultFunctionResolver min() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(INTEGER)),
-                arguments -> new MinAggregator(arguments, INTEGER))
+                (functionProperties, arguments) -> new MinAggregator(arguments, INTEGER))
             .put(new FunctionSignature(functionName, Collections.singletonList(LONG)),
-                arguments -> new MinAggregator(arguments, LONG))
+                (functionProperties, arguments) -> new MinAggregator(arguments, LONG))
             .put(new FunctionSignature(functionName, Collections.singletonList(FLOAT)),
-                arguments -> new MinAggregator(arguments, FLOAT))
+                (functionProperties, arguments) -> new MinAggregator(arguments, FLOAT))
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> new MinAggregator(arguments, DOUBLE))
+                (functionProperties, arguments) -> new MinAggregator(arguments, DOUBLE))
             .put(new FunctionSignature(functionName, Collections.singletonList(STRING)),
-                arguments -> new MinAggregator(arguments, STRING))
+                (functionProperties, arguments) -> new MinAggregator(arguments, STRING))
             .put(new FunctionSignature(functionName, Collections.singletonList(DATE)),
-                arguments -> new MinAggregator(arguments, DATE))
+                (functionProperties, arguments) -> new MinAggregator(arguments, DATE))
             .put(new FunctionSignature(functionName, Collections.singletonList(DATETIME)),
-                arguments -> new MinAggregator(arguments, DATETIME))
+                (functionProperties, arguments) -> new MinAggregator(arguments, DATETIME))
             .put(new FunctionSignature(functionName, Collections.singletonList(TIME)),
-                arguments -> new MinAggregator(arguments, TIME))
+                (functionProperties, arguments) -> new MinAggregator(arguments, TIME))
             .put(new FunctionSignature(functionName, Collections.singletonList(TIMESTAMP)),
-                arguments -> new MinAggregator(arguments, TIMESTAMP))
+                (functionProperties, arguments) -> new MinAggregator(arguments, TIMESTAMP))
             .build());
   }
 
@@ -131,23 +131,23 @@ private static DefaultFunctionResolver max() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(INTEGER)),
-                arguments -> new MaxAggregator(arguments, INTEGER))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, INTEGER))
             .put(new FunctionSignature(functionName, Collections.singletonList(LONG)),
-                arguments -> new MaxAggregator(arguments, LONG))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, LONG))
             .put(new FunctionSignature(functionName, Collections.singletonList(FLOAT)),
-                arguments -> new MaxAggregator(arguments, FLOAT))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, FLOAT))
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> new MaxAggregator(arguments, DOUBLE))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, DOUBLE))
             .put(new FunctionSignature(functionName, Collections.singletonList(STRING)),
-                arguments -> new MaxAggregator(arguments, STRING))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, STRING))
             .put(new FunctionSignature(functionName, Collections.singletonList(DATE)),
-                arguments -> new MaxAggregator(arguments, DATE))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, DATE))
             .put(new FunctionSignature(functionName, Collections.singletonList(DATETIME)),
-                arguments -> new MaxAggregator(arguments, DATETIME))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, DATETIME))
             .put(new FunctionSignature(functionName, Collections.singletonList(TIME)),
-                arguments -> new MaxAggregator(arguments, TIME))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, TIME))
             .put(new FunctionSignature(functionName, Collections.singletonList(TIMESTAMP)),
-                arguments -> new MaxAggregator(arguments, TIMESTAMP))
+                (functionProperties, arguments) -> new MaxAggregator(arguments, TIMESTAMP))
             .build()
     );
   }
@@ -158,7 +158,7 @@ private static DefaultFunctionResolver varSamp() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> varianceSample(arguments, DOUBLE))
+                (functionProperties, arguments) -> varianceSample(arguments, DOUBLE))
             .build()
     );
   }
@@ -169,7 +169,7 @@ private static DefaultFunctionResolver varPop() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> variancePopulation(arguments, DOUBLE))
+                (functionProperties, arguments) -> variancePopulation(arguments, DOUBLE))
             .build()
     );
   }
@@ -180,7 +180,7 @@ private static DefaultFunctionResolver stddevSamp() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> stddevSample(arguments, DOUBLE))
+                (functionProperties, arguments) -> stddevSample(arguments, DOUBLE))
             .build()
     );
   }
@@ -191,7 +191,7 @@ private static DefaultFunctionResolver stddevPop() {
         functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, Collections.singletonList(DOUBLE)),
-                arguments -> stddevPopulation(arguments, DOUBLE))
+                (functionProperties, arguments) -> stddevPopulation(arguments, DOUBLE))
             .build()
     );
   }
@@ -201,7 +201,7 @@ private static DefaultFunctionResolver take() {
     DefaultFunctionResolver functionResolver = new DefaultFunctionResolver(functionName,
         new ImmutableMap.Builder<FunctionSignature, FunctionBuilder>()
             .put(new FunctionSignature(functionName, ImmutableList.of(STRING, INTEGER)),
-                arguments -> new TakeAggregator(arguments, ARRAY))
+                (functionProperties, arguments) -> new TakeAggregator(arguments, ARRAY))
             .build());
     return functionResolver;
   }

diff --git a/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java b/core/src/main/java/org/opensearch/sql/expression/config/ExpressionConfig.java
@@ -6,12 +6,16 @@
 
 package org.opensearch.sql.expression.config;
 
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
 import java.util.HashMap;
 import org.opensearch.sql.expression.DSL;
 import org.opensearch.sql.expression.aggregation.AggregatorFunction;
 import org.opensearch.sql.expression.datetime.DateTimeFunction;
 import org.opensearch.sql.expression.datetime.IntervalClause;
 import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
+import org.opensearch.sql.expression.function.FunctionProperties;
 import org.opensearch.sql.expression.function.OpenSearchFunctions;
 import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunction;
 import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunction;
@@ -33,9 +37,10 @@ public class ExpressionConfig {
    * BuiltinFunctionRepository constructor.
    */
   @Bean
-  public BuiltinFunctionRepository functionRepository() {
+  public BuiltinFunctionRepository functionRepository(FunctionProperties functionContext) {
+
     BuiltinFunctionRepository builtinFunctionRepository =
-        new BuiltinFunctionRepository(new HashMap<>());
+        new BuiltinFunctionRepository(new HashMap<>(), functionContext);
     ArithmeticFunction.register(builtinFunctionRepository);
     BinaryPredicateOperator.register(builtinFunctionRepository);
     MathematicalFunction.register(builtinFunctionRepository);
@@ -51,8 +56,14 @@ public BuiltinFunctionRepository functionRepository() {
     return builtinFunctionRepository;
   }
 
+  @Bean
+  public FunctionProperties functionExecutionContext() {
+    return new FunctionProperties(Instant.now(), ZoneId.systemDefault());
+  }
+
   @Bean
   public DSL dsl(BuiltinFunctionRepository repository) {
     return new DSL(repository);
   }
+
 }