Query context for built-in functions

[MaxKsyunz]

Description

These changes allow creating SQL functions that expose engine metadata.

As an example, now-family of functions is updated to report consistent time across multiple invocations.

Note: sysdate was unmodified. As per MySQL, it returns the time at invocation.

Implementation Details

• Add QueryContext interface -- a way for SQL functions to access SQL plugin metadata.
• FunctionBuilder.apply signature expects QueryContext as well as list of arguments.
• An instance of QueryContext is setup in Expression.Config.functionRepository and passed to BuiltInFunctionRepository.
• Add FunctionDSL.queryContextFunction method. It is similar to FunctionDSL.impl but passes QueryContext to the implementing lambda function.

Issues Resolved

[List any issues this PR will resolve]

Check List

• [ ] New functionality includes testing.
  • [ ] All tests pass, including unit test, integration test and doctest
• [ ] New functionality has been documented.
  • [ ] New functionality has javadoc added
  • [ ] New functionality has user manual doc added
• [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[MaxKsyunz]

SQL Java CI is failing on test coverage for FunctionDSL class. Is it covered implicitly by other test? I'm not finding tests that explicitly test FunctionDSL.

What does the error message refer to?

Execution failed for task ':core:jacocoTestCoverageVerification'.
> Rule violated for class org.opensearch.sql.expression.function.FunctionDSL.1: lines covered ratio is 0.5, but expected minimum is 1.0
  Rule violated for class org.opensearch.sql.expression.function.FunctionDSL.2: lines covered ratio is 0.7, but expected minimum is 1.0

[codecov-commenter]

Codecov Report

Merging #937 (5bed983) into 2.x (be4512e) will decrease coverage by 35.49%. The diff coverage is n/a.

@@              Coverage Diff              @@
##                2.x     #937       +/-   ##
=============================================
- Coverage     98.26%   62.76%   -35.50%     
=============================================
  Files           324       10      -314     
  Lines          8399      658     -7741     
  Branches        553      119      -434     
=============================================
- Hits           8253      413     -7840     
- Misses          142      192       +50     
- Partials          4       53       +49     

Flag	Coverage Δ
query-workbench	62.76% <ø> (?)
sql-engine	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...a/org/opensearch/sql/analysis/AnalysisContext.java
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java
...c/main/java/org/opensearch/sql/expression/DSL.java
...sql/expression/aggregation/AggregatorFunction.java
...search/sql/expression/config/ExpressionConfig.java
...arch/sql/expression/datetime/DateTimeFunction.java
...expression/function/BuiltinFunctionRepository.java
...pensearch/sql/expression/function/FunctionDSL.java
...expression/function/RelevanceFunctionResolver.java
...nsearch/sql/expression/system/SystemFunctions.java
... and 324 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[MaxKsyunz]

@MaxKsyunz is there a test to confirm that multiple calls to now function within the same context return the same value?

[Yury-Fridlyand]

How to access the brand new QueryContext in unit tests? Integration tests?

[MaxKsyunz]

Another way to add this would be to make QueryContext a property of plan execution.

I like that better conceptually since "query execution start time" is related to plan execution, not function definition. It does seem more complex to implement -- the value would have to be passed from ExecutionEngine, to ProjectOperator, to valueOf via Environment. What do you think, @penghuo ?

[MaxKsyunz]

Another way to add this would be to make QueryContext a property of plan execution.

I like that better conceptually since "query execution start time" is related to plan execution, not function definition. It does seem more complex to implement -- the value would have to be passed from ExecutionEngine, to ProjectOperator, to valueOf via Environment. What do you think, @penghuo ?

See #960 for QueryContext as part of plan execution.

[penghuo]

Another way to add this would be to make QueryContext a property of plan execution. I like that better conceptually since "query execution start time" is related to plan execution, not function definition. It does seem more complex to implement -- the value would have to be passed from ExecutionEngine, to ProjectOperator, to valueOf via Environment. What do you think, @penghuo ?

See #960 for QueryContext as part of plan execution.

Thanks for push this PR. This is long pending issue. I did not get the idea why add SessionContext to valueOf interface. i perfer the #937 implementation, In my opinion, expression is compiled with context, and run with env.

[MaxKsyunz]

Thanks for push this PR. This is long pending issue. I did not get the idea why add SessionContext to valueOf interface. i perfer the #937 implementation, In my opinion, expression is compiled with context, and run with env.

Thank you @penghuo! I will clean up this PR and re-submit it.

As for #960, this is what I'm trying to get to:

1. PhysicalPlan.next returns a result set as well as metadata.
2. Expression.valueOf can access this metadata to update the result set.
3. Each PhyislcalPlan.next implementation can easily preserve metadata it received from its children as well as add its own.

It solves this problem but can also be used for other purposes. I'll write up a separate proposal with some examples and cleaner code.