[Spotless] Applying Google Code Format for core/src/main files #1 #312

MitchellGale · 2023-07-25T16:32:54Z

Description

This PR applies Spotless on Java code for:
core/src/main/java/org/opensearch/sql/analysis
core/src/main/java/org/opensearch/sql/ast
core/src/main/java/org/opensearch/sql/data
core/src/main/java/org/opensearch/sql/datasource

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.

codecov · 2023-07-25T16:44:15Z

Codecov Report

❗ No coverage uploaded for pull request base (integ/sl_GoogleJavaFormat1@d2de909). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 7c1b27a differs from pull request most recent head c88dc80. Consider uploading reports for the commit c88dc80 to get more accurate results

@@                      Coverage Diff                      @@
##             integ/sl_GoogleJavaFormat1     #312   +/-   ##
=============================================================
  Coverage                              ?   97.42%           
  Complexity                            ?     4646           
=============================================================
  Files                                 ?      408           
  Lines                                 ?    11550           
  Branches                              ?      838           
=============================================================
  Hits                                  ?    11253           
  Misses                                ?      290           
  Partials                              ?        7

Flag	Coverage Δ
sql-engine	`97.42% <0.00%> (?)`

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

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

core/src/main/java/org/opensearch/sql/analysis/AnalysisContext.java

Yury-Fridlyand · 2023-07-26T19:27:00Z

core/src/main/java/org/opensearch/sql/analysis/ExpressionReferenceOptimizer.java

- * LogicalProject[Ref("abs(age)"), sub(Ref("sum(age)"), Ref("avg(age)"))
- *  LogicalAgg(agg=[sum(age), avg(age)], group=[abs(age)]]
- *   LogicalRelation
+ * The optimizer used to replace the expression referred in the SelectClause e.g. The query SELECT


This javadoc loses info with merging lines. Probably you need to add javadoc tags (pre I suppose) to keep it visual and readable.
Please, go though all changed multiline javadocs to review.

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

core/src/main/java/org/opensearch/sql/ast/expression/DataType.java

core/build.gradle

core/src/main/java/org/opensearch/sql/analysis/SelectExpressionAnalyzer.java

core/src/main/java/org/opensearch/sql/data/type/WideningTypeRule.java

GumpacG · 2023-07-28T14:34:59Z

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

-   *     [WHEN compare_expr THEN result_expr] ...
-   *     [ELSE result_expr]
-   * END
+   * CASE case_value_expr WHEN compare_expr THEN result_expr [WHEN compare_expr THEN result_expr]


Also needs <pre> tag

GumpacG · 2023-07-28T14:37:19Z

core/src/main/java/org/opensearch/sql/data/model/AbstractExprValue.java

-   * NULL    MISSING FALSE
-   * MISSING NULL    FALSE
-   * MISSING MISSING TRUE
+   * The customize equals logic. The table below list the NULL and MISSING handling logic. A B A ==


Also needs <pre> tag

Addressed by adding table in d455533

acarbonetto

@MitchellGale There's one of Yury's commits in here. Would you be able to rebase and remove that commit?

Yury-Fridlyand · 2023-08-02T18:56:10Z

core/src/main/java/org/opensearch/sql/analysis/DataSourceSchemaIdentifierNameResolver.java

 import java.util.List;
-import java.util.Set;


Cleaning imports is a part of another check. Do you want to do it first?

It's on the agenda for being put into upstream, I don't think it'll cause a merge conflict. Need to wait for opensearch-project#1924 to be merged then I can put up the remove unused imports PR up.

Yury-Fridlyand · 2023-08-02T18:56:46Z

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

-                when.getResult()
-            ), context));
+        whens.add(
+            (WhenClause)


Yury-Fridlyand · 2023-08-02T18:58:47Z

core/src/main/java/org/opensearch/sql/analysis/ExpressionReferenceOptimizer.java

+ * LogicalPlan should be<br>
+ * LogicalProject[Ref("abs(age)"), sub(Ref("sum(age)"), Ref("avg(age)"))<br>
+ * &ensp LogicalAgg(agg=[sum(age), avg(age)], group=[abs(age)]]<br>
+ * &emsp LogicalRelation


Is it better than using pre? Or GJF ignores pre and rewrites the comment?

It's properly formatted this way vs using pre tag and just asking it not to be formatted. Both work.

Yury-Fridlyand · 2023-08-02T19:00:22Z

core/src/main/java/org/opensearch/sql/analysis/SelectExpressionAnalyzer.java

-   * Agg(Alias("AVG(age)", aggExpr), Alias("length(name)", groupExpr))
-   * </pre>
+   * The Alias could be<br>
+   * 1. SELECT name, AVG(age) FROM s BY name -><br>


Use ul and li HTML tags for list formatting.

Yury-Fridlyand · 2023-08-02T19:02:24Z

core/src/main/java/org/opensearch/sql/ast/expression/AllFields.java

 @ToString
 @EqualsAndHashCode(callSuper = false)
 public class AllFields extends UnresolvedExpression {
  public static final AllFields INSTANCE = new AllFields();

-  private AllFields() {
-  }
+  private AllFields() {}


This conflicts with checkstyle, but I like it more.
Consider updating checkstyle rules in future.

Checkstyle will be removed in favour of spotless. Currently it is set to "ignore test failures".

Yury-Fridlyand · 2023-08-02T19:07:30Z

core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java

-      throw new SemanticCheckException(String.format("date:%s in unsupported format, please use "
-          + "yyyy-MM-dd", date));
+      throw new SemanticCheckException(
+          String.format("date:%s in unsupported format, please use " + "yyyy-MM-dd", date));


Suggested change

String.format("date:%s in unsupported format, please use " + "yyyy-MM-dd", date));

String.format("date: %s in unsupported format, please use 'yyyy-MM-dd'", date));

Probably you need to update test if you apply this change.

Partly resolved in da35a7e. (no additional quotes)

Yury-Fridlyand · 2023-08-02T19:09:22Z

core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.java

-          + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", datetime));
+      throw new SemanticCheckException(
+          String.format(
+              "datetime:%s in unsupported format, please " + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",


Suggested change

"datetime:%s in unsupported format, please " + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",

"datetime: %s in unsupported format, please 'use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'",

Could be followed by test update

Do we want the single quotes around yyyy-MM-dd instead?

core/src/main/java/org/opensearch/sql/data/model/ExprMissingValue.java

Yury-Fridlyand · 2023-08-02T19:10:50Z

core/src/main/java/org/opensearch/sql/data/model/ExprStringValue.java

+                "datetime:%s in unsupported format, please "
+                    + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",


Suggested change

"datetime:%s in unsupported format, please "

+ "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",

"datetime: %s in unsupported format, please use 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'",

Yury-Fridlyand · 2023-08-02T19:12:16Z

opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java

@@ -34,6 +34,7 @@
 import lombok.SneakyThrows;
 import org.apache.commons.lang3.reflect.FieldUtils;
 import org.apache.lucene.search.TotalHits;
+import org.junit.jupiter.api.BeforeAll;


why?

Suggested change

import org.junit.jupiter.api.BeforeAll;

Thanks! Added as a part of an old fix I was testing... missed removing the import.

# This is the 1st commit message: Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change (opensearch-project#1899) * Added setDefaultMediaType for create_index and create_index_with_IOException Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> (cherry picked from commit 7b932a7) Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs 2 Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> cherry pick 60c0018 Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Addressed PR comment for table format in AbstractExprValue. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from ExpressionReferenceOptimizer. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> fixed java doc in QualifiedName.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removing checkstyle test for core and added spotless for relevant directories. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #2: Fixing spacing around headers in ExpressionReferenceOptimizer.java SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #3: Fix breaking changes. Disable some flaky tests in legacy. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> (cherry picked from commit 809e656) Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #4: Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #5: running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #6: Fixed java doc spelling and improving string concatination. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #7: improving string concatination. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #8: Improving failure format on some functions. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> # This is the commit message #9: spotless apply and fix of build.gradle Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com>

author Mitchell Gale <Mitchell.Gale@improving.com> 1691080710 -0700 committer Mitchell Gale <Mitchell.Gale@improving.com> 1691080766 -0700 Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change (opensearch-project#1899) * Added setDefaultMediaType for create_index and create_index_with_IOException Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> (cherry picked from commit 7b932a7) Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs 2 Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> cherry pick 60c0018 Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Addressed PR comment for table format in AbstractExprValue. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from ExpressionReferenceOptimizer. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removed pre tag from SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> fixed java doc in QualifiedName.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removing checkstyle test for core and added spotless for relevant directories. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Fixing spacing around headers in ExpressionReferenceOptimizer.java SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Fix breaking changes. Disable some flaky tests in legacy. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> (cherry picked from commit 809e656) Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Fixed java doc spelling and improving string concatination. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> improving string concatination. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Improving failure format on some functions. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> spotless apply and fix of build.gradle Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com> Removing unused import in OpenSearchRestClientTest.java Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com>

GumpacG · 2023-08-03T16:45:13Z

core/src/main/java/org/opensearch/sql/analysis/SelectExpressionAnalyzer.java

+   * The Alias could be<br>
+   *
+   * <ol>
+   *   <li>ELECT name, AVG(age) FROM s BY name -><br>


Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com>

MitchellGale requested a review from matthewryanwells July 25, 2023 16:32

MitchellGale requested review from MaxKsyunz, Yury-Fridlyand, forestmvey, acarbonetto and GumpacG as code owners July 25, 2023 16:32

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch from cd0826a to 25bfe09 Compare July 25, 2023 18:20

GumpacG reviewed Jul 25, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java Show resolved Hide resolved

Yury-Fridlyand reviewed Jul 25, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/analysis/AnalysisContext.java Show resolved Hide resolved

core/src/main/java/org/opensearch/sql/analysis/AnalysisContext.java Show resolved Hide resolved

Yury-Fridlyand reviewed Jul 26, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java Outdated Show resolved Hide resolved

MitchellGale mentioned this pull request Jul 26, 2023

[Spotless] Using custom Java formatter from k-nn plugin #315

Closed

6 tasks

MitchellGale commented Jul 26, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/ast/expression/DataType.java Show resolved Hide resolved

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch from c1174c4 to 60c0018 Compare July 26, 2023 22:59

Yury-Fridlyand reviewed Jul 27, 2023

View reviewed changes

core/build.gradle Outdated Show resolved Hide resolved

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch 3 times, most recently from 03127d0 to 613535f Compare July 27, 2023 18:06

matthewryanwells approved these changes Jul 27, 2023

View reviewed changes

MaxKsyunz approved these changes Jul 27, 2023

View reviewed changes

GumpacG reviewed Jul 27, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/analysis/SelectExpressionAnalyzer.java Outdated Show resolved Hide resolved

core/src/main/java/org/opensearch/sql/data/type/WideningTypeRule.java Outdated Show resolved Hide resolved

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch from 7d55f5a to d04dd9f Compare July 28, 2023 08:07

GumpacG reviewed Jul 28, 2023

View reviewed changes

acarbonetto reviewed Jul 31, 2023

View reviewed changes

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch 4 times, most recently from 54cb07d to a43d97d Compare July 31, 2023 21:29

Yury-Fridlyand approved these changes Aug 2, 2023

View reviewed changes

This was referenced Aug 2, 2023

[Spotless] Applying Google Code Format for core/src/main files #2 #313

Closed

[Spotless] Applying Google Code Format for core/src/main files #4 #317

Merged

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch from d6b6aab to 0fae29c Compare August 3, 2023 00:30

MitchellGale changed the base branch from integ/sl_GoogleJavaFormat1 to Failing-CI-Hot-Fix August 3, 2023 07:20

MitchellGale changed the base branch from Failing-CI-Hot-Fix to integ/sl_GoogleJavaFormat1 August 3, 2023 07:20

MitchellGale force-pushed the integ/sl_GoogleJavaFormat1 branch from 7a52c9b to d2de909 Compare August 3, 2023 07:23

MitchellGale and others added 2 commits August 3, 2023 09:27

GumpacG approved these changes Aug 3, 2023

View reviewed changes

MitchellGale force-pushed the dev/sl_GoogleJavaFormat1 branch from 48df392 to 7579925 Compare August 3, 2023 17:33

running spotless apply.

c88dc80

Signed-off-by: Mitchell Gale <Mitchell.Gale@improving.com>

MitchellGale merged commit ec7035e into integ/sl_GoogleJavaFormat1 Aug 3, 2023
12 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spotless] Applying Google Code Format for core/src/main files #1 #312

[Spotless] Applying Google Code Format for core/src/main files #1 #312

MitchellGale commented Jul 25, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading

Yury-Fridlyand Jul 26, 2023

GumpacG Jul 28, 2023

GumpacG Jul 28, 2023

MitchellGale Jul 31, 2023

acarbonetto left a comment

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 2, 2023

Yury-Fridlyand Aug 2, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 2, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 3, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 2, 2023

Yury-Fridlyand Aug 2, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 3, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 2, 2023

Yury-Fridlyand Aug 3, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 3, 2023

Yury-Fridlyand Aug 2, 2023

MitchellGale Aug 3, 2023

MitchellGale Aug 3, 2023

GumpacG Aug 3, 2023

MitchellGale Aug 3, 2023

	String.format("date:%s in unsupported format, please use " + "yyyy-MM-dd", date));
	String.format("date: %s in unsupported format, please use 'yyyy-MM-dd'", date));

	"datetime:%s in unsupported format, please " + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",
	"datetime: %s in unsupported format, please 'use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'",

		"datetime:%s in unsupported format, please "
		+ "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]",

[Spotless] Applying Google Code Format for core/src/main files #1 #312

[Spotless] Applying Google Code Format for core/src/main files #1 #312

Conversation

MitchellGale commented Jul 25, 2023 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acarbonetto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitchellGale commented Jul 25, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading