From d4413b31ec2cdf37e1af3e8211dbceb9cb42e856 Mon Sep 17 00:00:00 2001 From: Mitchell Gale Date: Wed, 16 Aug 2023 11:01:26 -0700 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Yury-Fridlyand Co-authored-by: Guian Gumpac Signed-off-by: Mitchell Gale --- .../sql/correctness/tests/OpenSearchConnectionTest.java | 4 ++-- .../src/test/java/org/opensearch/sql/legacy/OrderIT.java | 6 +++--- .../org/opensearch/sql/legacy/PrettyFormatResponseIT.java | 8 ++++---- .../java/org/opensearch/sql/legacy/SQLFunctionsIT.java | 4 ++-- .../java/org/opensearch/sql/legacy/SQLIntegTestCase.java | 5 +++-- .../java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java | 6 +++--- .../test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java | 2 +- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/correctness/tests/OpenSearchConnectionTest.java b/integ-test/src/test/java/org/opensearch/sql/correctness/tests/OpenSearchConnectionTest.java index 9e36cc23ae..e5130d8fc1 100644 --- a/integ-test/src/test/java/org/opensearch/sql/correctness/tests/OpenSearchConnectionTest.java +++ b/integ-test/src/test/java/org/opensearch/sql/correctness/tests/OpenSearchConnectionTest.java @@ -66,7 +66,7 @@ public void testInsertData() throws IOException { assertEquals("POST", actual.getMethod()); assertEquals("/test/_bulk?refresh=true", actual.getEndpoint()); assertEquals( - "{\"index\":{}}\n" + "{\"name\":\"John\"}\n" + "{\"index\":{}}\n" + "{\"name\":\"Hank\"}\n", + "{\"index\":{}}\n{\"name\":\"John\"}\n{\"index\":{}}\n{\"name\":\"Hank\"}\n", getBody(actual)); } @@ -81,7 +81,7 @@ public void testInsertNullData() throws IOException { assertEquals("POST", actual.getMethod()); assertEquals("/test/_bulk?refresh=true", actual.getEndpoint()); assertEquals( - "{\"index\":{}}\n" + "{\"age\":30}\n" + "{\"index\":{}}\n" + "{\"name\":\"Hank\"}\n", + "{\"index\":{}}\n{\"age\":30}\n{\"index\":{}}\n{\"name\":\"Hank\"}\n", getBody(actual)); } diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/OrderIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/OrderIT.java index 3e0191c009..01e989e9f0 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/OrderIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/OrderIT.java @@ -77,10 +77,10 @@ public void orderByIsNull() throws IOException { // Another equivalent syntax assertThat( explainQuery( - "SELECT * FROM opensearch-sql_test_index_order " + "ORDER BY id IS NULL, id DESC"), + "SELECT * FROM opensearch-sql_test_index_order ORDER BY id IS NULL, id DESC"), equalTo( explainQuery( - "SELECT * FROM opensearch-sql_test_index_order " + "ORDER BY id IS NULL DESC"))); + "SELECT * FROM opensearch-sql_test_index_order ORDER BY id IS NULL DESC"))); } @Test @@ -100,7 +100,7 @@ public void orderByIsNotNull() throws IOException { // Another equivalent syntax assertThat( explainQuery( - "SELECT id, name FROM opensearch-sql_test_index_order " + "ORDER BY name IS NOT NULL"), + "SELECT id, name FROM opensearch-sql_test_index_order ORDER BY name IS NOT NULL"), equalTo( explainQuery( "SELECT id, name FROM opensearch-sql_test_index_order " diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java index d6bc14b203..70f8a3c433 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/PrettyFormatResponseIT.java @@ -300,7 +300,7 @@ public void testSizeAndTotal() throws IOException { executeQuery( String.format( Locale.ROOT, - "SELECT * " + "FROM %s " + "WHERE balance > 30000 " + "LIMIT 5", + "SELECT * FROM %s WHERE balance > 30000 LIMIT 5", TestsConstants.TEST_INDEX_ACCOUNT)); JSONArray dataRows = getDataRows(response); @@ -427,7 +427,7 @@ public void aggregationFunctionInHaving() throws IOException { executeQuery( String.format( Locale.ROOT, - "SELECT gender " + "FROM %s " + "GROUP BY gender " + "HAVING count(*) > 500", + "SELECT gender FROM %s GROUP BY gender HAVING count(*) > 500", TestsConstants.TEST_INDEX_ACCOUNT)); String ageSum = "gender"; @@ -549,7 +549,7 @@ public void joinQuerySelectOnlyOnOneTable() throws Exception { executeQuery( String.format( Locale.ROOT, - "SELECT b1.age " + "FROM %s b1 JOIN %s b2 ON b1.firstname = b2.firstname", + "SELECT b1.age FROM %s b1 JOIN %s b2 ON b1.firstname = b2.firstname", TestsConstants.TEST_INDEX_ACCOUNT, TestsConstants.TEST_INDEX_ACCOUNT)); @@ -583,7 +583,7 @@ private void testFieldOrder(final String[] expectedFields, final Object[] expect final String query = String.format( Locale.ROOT, - "SELECT %s FROM %s " + "WHERE email='amberduke@pyrami.com'", + "SELECT %s FROM %s WHERE email='amberduke@pyrami.com'", fields, TestsConstants.TEST_INDEX_ACCOUNT); final JSONObject result = executeQuery(query); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java index 877f803189..ebd7084a67 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java @@ -813,7 +813,7 @@ public void literal() throws Exception { @Test public void literalWithDoubleValue() throws Exception { - String query = "SELECT 10.0 " + "from " + TEST_INDEX_ACCOUNT + " limit 1"; + String query = "SELECT 10.0 from " + TEST_INDEX_ACCOUNT + " limit 1"; final SearchHit[] hits = query(query).getHits(); assertThat(hits[0].getFields(), hasValue(contains(10.0))); @@ -821,7 +821,7 @@ public void literalWithDoubleValue() throws Exception { @Test public void literalWithAlias() throws Exception { - String query = "SELECT 10 as key " + "from " + TEST_INDEX_ACCOUNT + " limit 1"; + String query = "SELECT 10 as key from " + TEST_INDEX_ACCOUNT + " limit 1"; final SearchHit[] hits = query(query).getHits(); assertThat(hits.length, is(1)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 4e65f4572b..4479abdcc6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -142,8 +142,9 @@ public static void dumpCoverage() { } /** - * As JUnit JavaDoc says: "The @AfterClass methods declared in superclasses will be run after - * those of the current class." So this method is supposed to run before closeClients() in parent + * As JUnit JavaDoc says:
+ "The @AfterClass methods declared in superclasses will be run after those of the current class."
+ So this method is supposed to run before closeClients() in parent class. * class. */ @AfterClass diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java index cc836b1896..501d4bcb5e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ObjectFieldOperateIT.java @@ -28,7 +28,7 @@ public void select_object_field() throws IOException { JSONObject result = executeQuery( String.format( - "source=%s | " + "fields city.name, city.location.latitude", + "source=%s | fields city.name, city.location.latitude", TEST_INDEX_DEEP_NESTED)); verifySchema(result, schema("city.name", "string"), schema("city.location.latitude", "double")); verifyDataRows(result, rows("Seattle", 10.5)); @@ -51,7 +51,7 @@ public void compare_object_field_in_where() throws IOException { public void group_object_field_in_stats() throws IOException { JSONObject result = executeQuery( - String.format("source=%s " + "| stats count() by city.name", TEST_INDEX_DEEP_NESTED)); + String.format("source=%s | stats count() by city.name", TEST_INDEX_DEEP_NESTED)); verifySchema(result, schema("count()", "integer"), schema("city.name", "string")); verifyDataRows(result, rows(1, "Seattle")); } @@ -61,7 +61,7 @@ public void sort_by_object_field() throws IOException { JSONObject result = executeQuery( String.format( - "source=%s " + "| sort city.name" + "| fields city.name, city.location.latitude", + "source=%s | sort city.name | fields city.name, city.location.latitude", TEST_INDEX_DEEP_NESTED)); verifySchema(result, schema("city.name", "string"), schema("city.location.latitude", "double")); verifyDataRows(result, rows("Seattle", 10.5)); diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java index aaefbbe395..80a89ed9c3 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/QueryAnalysisIT.java @@ -108,7 +108,7 @@ public void unsupportedAggregationFunctionShouldFailSyntaxCheck() { public void nonexistentFieldShouldFailSemanticCheck() { String query = String.format("search source=%s | fields name", TEST_INDEX_ACCOUNT); queryShouldThrowSemanticException( - query, "can't resolve Symbol(namespace=FIELD_NAME, " + "name=name) in type env"); + query, "can't resolve Symbol(namespace=FIELD_NAME, name=name) in type env"); } private void queryShouldPassSyntaxAndSemanticCheck(String query) {