addressed PR comments.

Signed-off-by: Mitchell Gale <[email protected]>

build.gradle

@@ -94,7 +94,7 @@ spotless {
 //        licenseHeader("/*\n" +
 //                " * Copyright OpenSearch Contributors\n" +
 //                " * SPDX-License-Identifier: Apache-2.0\n" +
-//                " */\n\n\n")
+//                " */\n\n")
         removeUnusedImports()
         trimTrailingWhitespace()
         endWithNewline()

core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java

@@ -20,8 +20,8 @@
 public interface ExecutionEngine {
 
   /**
-   * Execute physical plan and call back response listener. Todo. deprecated this interface after
-   * finalize {@link ExecutionContext}.
+   * Execute physical plan and call back response listener.<br>
+   * Todo. deprecated this interface after finalize {@link ExecutionContext}.
    *
    * @param plan executable physical plan
    * @param listener response listener

core/src/main/java/org/opensearch/sql/executor/QueryManager.java

@@ -11,8 +11,8 @@
 import org.opensearch.sql.executor.execution.AbstractPlan;
 
 /**
- * QueryManager is the high-level interface of core engine. Frontend submit {@link AbstractPlan} to
- * QueryManager.
+ * QueryManager is the high-level interface of core engine. Frontend submit an {@link AbstractPlan}
+ * to QueryManager.
  */
 public interface QueryManager {
 

core/src/main/java/org/opensearch/sql/executor/QueryService.java

@@ -29,8 +29,8 @@ public class QueryService {
   private final Planner planner;
 
   /**
-   * Execute the {@link UnresolvedPlan}, using {@link ResponseListener} to get response. Todo.
-   * deprecated this interface after finalize {@link PlanContext}.
+   * Execute the {@link UnresolvedPlan}, using {@link ResponseListener} to get response.<br>
+   * Todo. deprecated this interface after finalize {@link PlanContext}.
    *
    * @param plan {@link UnresolvedPlan}
    * @param listener {@link ResponseListener}

core/src/main/java/org/opensearch/sql/executor/pagination/CanPaginateVisitor.java

@@ -46,16 +46,20 @@
  *
  * <pre>plan.accept(new CanPaginateVisitor(...))</pre>
  *
- * returns <em>true</em>,<br>
- * then PaginatedPlanCache.convertToCursor will succeed. Otherwise, it will fail.<br>
+ * returns <em>true</em>, <em>PaginatedPlanCache.convertToCursor</em> will succeed.<br>
+ * Otherwise, it will fail.<br>
  * The purpose of this visitor is to activate legacy engine fallback mechanism.<br>
  * Currently, V2 engine does not support queries with:<br>
- * - aggregation (GROUP BY clause or aggregation functions like min/max)<br>
- * - in memory aggregation (window function)<br>
- * - LIMIT/OFFSET clause(s)<br>
- * - without FROM clause<br>
- * - JOIN<br>
- * - a subquery<br>
+ *
+ * <ul>
+ *   <li>aggregation (GROUP BY clause or aggregation functions like min/max)
+ *   <li>in memory aggregation (window function)
+ *   <li>LIMIT/OFFSET clause(s)
+ *   <li>without FROM clause
+ *   <li>JOIN
+ *   <li>a subquery
+ * </ul>
+ *
  * V2 also requires that the table being queried should be an OpenSearch index.<br>
  * See PaginatedPlanCache.canConvertToCursor for usage.
  */

core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java

@@ -34,9 +34,11 @@
 import org.opensearch.sql.expression.function.FunctionSignature;
 
 /**
- * The definition of aggregator function avg, Accepts two numbers and produces a number. sum,
- * Accepts two numbers and produces a number. max, Accepts two numbers and produces a number. min,
- * Accepts two numbers and produces a number. count, Accepts two numbers and produces a number.
+ * The definition of aggregator functions <em>avg</em>, <em>sum</em>, <em>min</em>, <em>max</em> and
+ * <em>count</em>.<br>
+ * All of them accept a list of numbers and produce a number. <em>avg</em>, <em>min</em> and
+ * <em>max</em> also accept datetime types.<br>
+ * <em>count</em> accepts values of all types.
  */
 @UtilityClass
 public class AggregatorFunction {

core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFormatterUtil.java

@@ -138,10 +138,8 @@ interface DateTimeFormatHandler {
           .put("%Y", (date) -> "0000")
           .put("%y", (date) -> "00")
           .put("%D", (date) -> null)
-          .put(
-              "%f",
-              (date) -> // %f - Microseconds
-              String.format(NANO_SEC_FORMAT, (date.getNano() / 1000)))
+          // %f - Microseconds
+          .put("%f", (date) -> String.format(NANO_SEC_FORMAT, (date.getNano() / 1000)))
           .put("%w", (date) -> null)
           .put("%U", (date) -> null)
           .put("%u", (date) -> null)

core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java

@@ -810,7 +810,8 @@ private DefaultFunctionResolver period_add() {
 
   /**
    * Returns the number of months between periods P1 and P2. P1 and P2 should be in the format YYMM
-   * or YYYYMM. (INTEGER, INTEGER) -> INTEGER
+   * or YYYYMM.<br>
+   * (INTEGER, INTEGER) -> INTEGER
    */
   private DefaultFunctionResolver period_diff() {
     return define(
@@ -1713,10 +1714,13 @@ private ExprValue exprLastDayToday(Clock clock) {
   /**
    * Following MySQL, function receives arguments of type double and rounds them before use.<br>
    * Furthermore:<br>
-   * - zero year interpreted as 2000<br>
-   * - negative year is not accepted<br>
-   * - @dayOfYear should be greater than 1<br>
-   * - if @dayOfYear is greater than 365/366, calculation goes to the next year(s)<br>
+   *
+   * <ul>
+   *   <li>zero year interpreted as 2000
+   *   <li>negative year is not accepted
+   *   <li>@dayOfYear should be greater than 1
+   *   <li>if @dayOfYear is greater than 365/366, calculation goes to the next year(s)
+   * </ul>
    *
    * @param yearExpr year
    * @param dayOfYearExp day of the @year, starting from 1

core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java

@@ -113,8 +113,8 @@ private static DefaultFunctionResolver baseMathFunction(
 
   /**
    * <b>Definition of abs() function.<\b><br>
-   * The supported signature of abs() function are INT -> INT LONG -> LONG FLOAT -> FLOAT DOUBLE ->
-   * DOUBLE
+   * The supported signature of abs() function are<br>
+   * INT -> INT LONG -> LONG FLOAT -> FLOAT DOUBLE -> DOUBLE
    */
   private static DefaultFunctionResolver abs() {
     return define(
@@ -268,8 +268,8 @@ private static DefaultFunctionResolver ln() {
 
   /**
    * <b>Definition of log(b, x) function.</b><br>
-   * Calculate the logarithm of x using b as the base The supported signature of log function is (b:
-   * INTEGER/LONG/FLOAT/DOUBLE, x: INTEGER/LONG/FLOAT/DOUBLE]) -> DOUBLE
+   * Calculate the logarithm of x using b as the base The supported signature of log function is<br>
+   * (b: INTEGER/LONG/FLOAT/DOUBLE, x: INTEGER/LONG/FLOAT/DOUBLE]) -> DOUBLE
    */
   private static DefaultFunctionResolver log() {
     ImmutableList.Builder<
@@ -891,7 +891,8 @@ private static DefaultFunctionResolver radians() {
   /**
    * <b>Definition of sin(x) function.</b><br>
    * Calculates the sine of x, where x is given in radians The supported signature of sin function
-   * is INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
+   * is<br>
+   * INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
    */
   private static DefaultFunctionResolver sin() {
     return baseMathFunction(
@@ -903,7 +904,8 @@ private static DefaultFunctionResolver sin() {
   /**
    * <b>Definition of tan(x) function.</b><br>
    * Calculates the tangent of x, where x is given in radians The supported signature of tan
-   * function is INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
+   * function is<br>
+   * INTEGER/LONG/FLOAT/DOUBLE -> DOUBLE
    */
   private static DefaultFunctionResolver tan() {
     return baseMathFunction(

core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java

@@ -209,7 +209,8 @@ public ExprType type() {
    * Extend to accept variable argument amounts.<br>
    * <br>
    * Concatenates a list of Strings with a separator string. Supports following<br>
-   * signatures: (STRING, STRING, STRING) -> STRING
+   * signatures:<br>
+   * (STRING, STRING, STRING) -> STRING
    */
   private DefaultFunctionResolver concat_ws() {
     return define(