Decouple function repository and DSL from IoC container for use anywhere

[dai-chen]

Description

!!!NOTE!!! Please ignore most file changes as they're all changing instance method to static method call on DSL. The only change matters is in FunctionRepository which becomes a singleton after this.

Problem Statement

The performance issue is improved in #944, this PR is focused on the functionality and our own DSL language adoption problem.

Currently, FunctionRepository is managed by Spring container and inject to class who needs it. DSL depends on the function repository to resolve function signature as well. This causes the function repository or DSL instance passed around and leads to "ugly" code as below:

  private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository());

  dsl.equal(
    DSL.ref("integer_value", INTEGER),
    DSL.literal(integerValue(1)))),

Besides, the bigger problem is it blocks high level physical operator from building custom logic on top of the expression system. For example, the new WindowAssigner needs to calculate the lower bound of a window as below. The current coupling with Spring container make it hard to implement:

  // Assign a numeric or datetime window to the value according to its type
  Window assignWindow(ExprValue value) {
    ...
    ExprValue lowerBound = rounding.round(value);
    ExprValue windowSize = ...
    ExprValue upperBound = 
      lowerBound.isNumber()
        ? DSL.add(lowerBound, windowNumberSize)
        : DSL.adddate(lowerBound, windowIntervalSize)
  }

Solution

After double check, make FunctionRepository singleton as well as DSL. This makes both can be reused anywhere.

TODO

1. May need to move storage engine function register code elsewhere, because it is supposed to be a static list for each storage engine. Added TODO comment for now.

Further Thoughts

1. Will this impact storage engine register its own function in future? <= Probably not.
2. Should basic arithmetic operation built-in ExprValue? ex. val1.add(val2) rather than DSL.add(val1, val2).valueOf(null) <= Not sure. Maybe we should.

Issues Resolved

#944

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.

[codecov-commenter]

Codecov Report

Merging #1085 (b771edc) into 2.x (cdb30ee) will decrease coverage by 1.83%.
The diff coverage is 100.00%.

❗ Current head b771edc differs from pull request most recent head 58b0b5b. Consider uploading reports for the commit 58b0b5b to get more accurate results

@@             Coverage Diff              @@
##                2.x    #1085      +/-   ##
============================================
- Coverage     98.28%   96.44%   -1.84%     
+ Complexity     3435     2636     -799     
============================================
  Files           344      254      -90     
  Lines          8560     6972    -1588     
  Branches        544      521      -23     
============================================
- Hits           8413     6724    -1689     
- Misses          142      194      +52     
- Partials          5       54      +49     

Flag	Coverage Δ
query-workbench	62.76% <ø> (?)
sql-engine	99.95% <100.00%> (+1.66%)	⬆️

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

Impacted Files	Coverage Δ
...ch/sql/planner/optimizer/LogicalPlanOptimizer.java	100.00% <ø> (ø)
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java	100.00% <100.00%> (ø)
...c/main/java/org/opensearch/sql/expression/DSL.java	100.00% <100.00%> (ø)
...expression/function/BuiltinFunctionRepository.java	100.00% <100.00%> (ø)
...l/planner/optimizer/rule/MergeFilterAndFilter.java	100.00% <100.00%> (ø)
.../opensearch/sql/opensearch/response/agg/Utils.java
...ch/sql/opensearch/client/OpenSearchRestClient.java
...age/serialization/DefaultExpressionSerializer.java
...ggregation/ExpressionAggregationScriptFactory.java
...ch/request/system/OpenSearchCatIndicesRequest.java
... and 104 more

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

[acarbonetto]

LGTM

[penghuo]

Thanks

[dai-chen]

@Yury-Fridlyand Please confirm if the previous issues you mentioned in the old PR are gone. Will try to merge this today to unblock others. Thanks!

[derek-ho]

LGTM

[penghuo]

Thanks.