From 9ffe777e996b621896426cf00fb4dc3193b8aab1 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 08:10:38 -0700 Subject: [PATCH 01/12] Add Explain yaml format Signed-off-by: Peng Huo --- .../opensearch/sql/ast/statement/Explain.java | 4 +- .../opensearch/sql/executor/QueryService.java | 3 +- .../opensearch/sql/utils/YamlFormatter.java | 5 + docs/category.json | 8 +- docs/user/ppl/interfaces/endpoint.rst | 86 ++++++++--- .../sql/calcite/remote/CalciteExplainIT.java | 136 +++++++++--------- .../org/opensearch/sql/ppl/ExplainIT.java | 111 +++++++------- .../opensearch/sql/ppl/PPLIntegTestCase.java | 8 ++ .../org/opensearch/sql/util/MatcherUtils.java | 4 +- .../sql/plugin/rest/RestPPLQueryAction.java | 7 +- .../transport/TransportPPLQueryAction.java | 38 +++-- .../transport/TransportPPLQueryResponse.java | 15 +- .../sql/ppl/domain/PPLQueryRequestTest.java | 7 + .../sql/protocol/response/format/Format.java | 5 +- .../format/YamlResponseFormatter.java | 50 +++++++ .../protocol/response/format/FormatTest.java | 7 + 16 files changed, 326 insertions(+), 168 deletions(-) create mode 100644 protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java diff --git a/core/src/main/java/org/opensearch/sql/ast/statement/Explain.java b/core/src/main/java/org/opensearch/sql/ast/statement/Explain.java index d592d7691cf..dd918a886a4 100644 --- a/core/src/main/java/org/opensearch/sql/ast/statement/Explain.java +++ b/core/src/main/java/org/opensearch/sql/ast/statement/Explain.java @@ -39,7 +39,9 @@ public enum ExplainFormat { SIMPLE, STANDARD, EXTENDED, - COST + COST, + /** Formats explain output in yaml format. */ + YAML } public static ExplainFormat format(String format) { diff --git a/core/src/main/java/org/opensearch/sql/executor/QueryService.java b/core/src/main/java/org/opensearch/sql/executor/QueryService.java index 995d7d55e0d..9fd3d63200d 100644 --- a/core/src/main/java/org/opensearch/sql/executor/QueryService.java +++ b/core/src/main/java/org/opensearch/sql/executor/QueryService.java @@ -189,7 +189,8 @@ public void explainWithLegacy( Explain.ExplainFormat format, Optional calciteFailure) { try { - if (format != null && format != Explain.ExplainFormat.STANDARD) { + if (format != null + && (format != Explain.ExplainFormat.STANDARD && format != Explain.ExplainFormat.YAML)) { throw new UnsupportedOperationException( "Explain mode " + format.name() + " is not supported in v2 engine"); } diff --git a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java index 3ccafb34abd..c5cbfd9df93 100644 --- a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java +++ b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java @@ -5,8 +5,10 @@ package org.opensearch.sql.utils; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; @@ -26,6 +28,9 @@ public class YamlFormatter { YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS); // Quote numeric strings yamlFactory.enable(YAMLGenerator.Feature.INDENT_ARRAYS_WITH_INDICATOR); YAML_MAPPER = new ObjectMapper(yamlFactory); + + YAML_MAPPER.setSerializationInclusion(JsonInclude.Include.NON_NULL); + YAML_MAPPER.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false); } /** diff --git a/docs/category.json b/docs/category.json index cb40f1ebbcd..956faf099a5 100644 --- a/docs/category.json +++ b/docs/category.json @@ -1,12 +1,12 @@ { "bash": [ - "user/ppl/interfaces/endpoint.rst", - "user/ppl/interfaces/protocol.rst", - "user/ppl/admin/settings.rst", "user/optimization/optimization.rst", "user/admin/settings.rst" ], - "ppl_cli": [ + "bash_calcite": [ + "user/ppl/interfaces/endpoint.rst", + "user/ppl/interfaces/protocol.rst", + "user/ppl/admin/settings.rst" ], "sql_cli": [ "user/dql/expressions.rst", diff --git a/docs/user/ppl/interfaces/endpoint.rst b/docs/user/ppl/interfaces/endpoint.rst index 967761caa37..5a2e8471549 100644 --- a/docs/user/ppl/interfaces/endpoint.rst +++ b/docs/user/ppl/interfaces/endpoint.rst @@ -73,28 +73,78 @@ Description You can send HTTP explain request to endpoint **/_plugins/_ppl/_explain** with your query in request body to understand the execution plan for the PPL query. The explain endpoint is useful when user want to get insight how the query is executed in the engine. -Example -------- +Description +----------- + +To translate your query, send it to explain endpoint. The explain output is OpenSearch domain specific language (DSL) in JSON format. You can just copy and paste it to your console to run it against OpenSearch directly. + +Explain output could be set different formats: ``standard`` (the default format), ``simple``, ``extended``, ``dsl``. + + +Example 1 default (standard) format +----------------------------------- -The following PPL query demonstrated that where and stats command were pushed down to OpenSearch DSL aggregation query:: +Explain query:: sh$ curl -sS -H 'Content-Type: application/json' \ ... -X POST localhost:9200/_plugins/_ppl/_explain \ - ... -d '{"query" : "source=accounts | where age > 10 | stats avg(age)"}' + ... -d '{"query" : "source=state_country | where age>30"}' { - "root": { - "name": "ProjectOperator", - "description": { - "fields": "[avg(age)]" - }, - "children": [ - { - "name": "OpenSearchIndexScan", - "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"aggregations\":{\"avg(age)\":{\"avg\":{\"field\":\"age\"}}}}, searchDone=false)" - }, - "children": [] - } - ] + "calcite": { + "logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(name=[$0], country=[$1], state=[$2], month=[$3], year=[$4], age=[$5])\n LogicalFilter(condition=[>($5, 30)])\n CalciteLogicalIndexScan(table=[[OpenSearch, state_country]])\n", + "physical": "CalciteEnumerableIndexScan(table=[[OpenSearch, state_country]], PushDownContext=[[PROJECT->[name, country, state, month, year, age], FILTER->>($5, 30), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"name\",\"country\",\"state\",\"month\",\"year\",\"age\"],\"excludes\":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])\n" + } + } + +Example 2 simple format +----------------------- + +Explain query:: + + sh$ curl -sS -H 'Content-Type: application/json' \ + ... -X POST localhost:9200/_plugins/_ppl/_explain?format=simple \ + ... -d '{"query" : "source=state_country | where age>30"}' + { + "calcite": { + "logical": "LogicalSystemLimit\n LogicalProject\n LogicalFilter\n CalciteLogicalIndexScan\n" } } + +Example 3 extended format +------------------------- + +Explain query:: + + sh$ curl -sS -H 'Content-Type: application/json' \ + ... -X POST localhost:9200/_plugins/_ppl/_explain?format=extended \ + ... -d '{"query" : "source=state_country | where age>30 | dedup age"}' + { + "calcite": { + "logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(name=[$0], country=[$1], state=[$2], month=[$3], year=[$4], age=[$5])\n LogicalFilter(condition=[<=($12, 1)])\n LogicalProject(name=[$0], country=[$1], state=[$2], month=[$3], year=[$4], age=[$5], _id=[$6], _index=[$7], _score=[$8], _maxscore=[$9], _sort=[$10], _routing=[$11], _row_number_dedup_=[ROW_NUMBER() OVER (PARTITION BY $5 ORDER BY $5)])\n LogicalFilter(condition=[IS NOT NULL($5)])\n LogicalFilter(condition=[>($5, 30)])\n CalciteLogicalIndexScan(table=[[OpenSearch, state_country]])\n", + "physical": "EnumerableLimit(fetch=[10000])\n EnumerableCalc(expr#0..6=[{inputs}], expr#7=[1], expr#8=[<=($t6, $t7)], proj#0..5=[{exprs}], $condition=[$t8])\n EnumerableWindow(window#0=[window(partition {5} order by [5] rows between UNBOUNDED PRECEDING and CURRENT ROW aggs [ROW_NUMBER()])])\n CalciteEnumerableIndexScan(table=[[OpenSearch, state_country]], PushDownContext=[[PROJECT->[name, country, state, month, year, age], FILTER->>($5, 30)], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"name\",\"country\",\"state\",\"month\",\"year\",\"age\"],\"excludes\":[]}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n", + "extended": "public org.apache.calcite.linq4j.Enumerable bind(final org.apache.calcite.DataContext root) {\n final org.opensearch.sql.opensearch.storage.scan.CalciteEnumerableIndexScan v1stashed = (org.opensearch.sql.opensearch.storage.scan.CalciteEnumerableIndexScan) root.get(\"v1stashed\");\n int prevStart;\n int prevEnd;\n final java.util.Comparator comparator = new java.util.Comparator(){\n public int compare(Object[] v0, Object[] v1) {\n final int c;\n c = org.apache.calcite.runtime.Utilities.compareNullsLast((Long) v0[5], (Long) v1[5]);\n if (c != 0) {\n return c;\n }\n return 0;\n }\n\n public int compare(Object o0, Object o1) {\n return this.compare((Object[]) o0, (Object[]) o1);\n }\n\n };\n final org.apache.calcite.runtime.SortedMultiMap multiMap = new org.apache.calcite.runtime.SortedMultiMap();\n v1stashed.scan().foreach(new org.apache.calcite.linq4j.function.Function1() {\n public Object apply(Object[] v) {\n Long key = (Long) v[5];\n multiMap.putMulti(key, v);\n return null;\n }\n public Object apply(Object v) {\n return apply(\n (Object[]) v);\n }\n }\n );\n final java.util.Iterator iterator = multiMap.arrays(comparator);\n final java.util.ArrayList _list = new java.util.ArrayList(\n multiMap.size());\n Long a0w0 = (Long) null;\n while (iterator.hasNext()) {\n final Object[] _rows = (Object[]) iterator.next();\n prevStart = -1;\n prevEnd = 2147483647;\n for (int i = 0; i < _rows.length; ++i) {\n final Object[] row = (Object[]) _rows[i];\n if (i != prevEnd) {\n int actualStart = i < prevEnd ? 0 : prevEnd + 1;\n prevEnd = i;\n a0w0 = Long.valueOf(((Number)org.apache.calcite.linq4j.tree.Primitive.of(long.class).numberValueRoundDown((i - 0 + 1))).longValue());\n }\n _list.add(new Object[] {\n row[0],\n row[1],\n row[2],\n row[3],\n row[4],\n row[5],\n a0w0});\n }\n }\n multiMap.clear();\n final org.apache.calcite.linq4j.Enumerable _inputEnumerable = org.apache.calcite.linq4j.Linq4j.asEnumerable(_list);\n final org.apache.calcite.linq4j.AbstractEnumerable child = new org.apache.calcite.linq4j.AbstractEnumerable(){\n public org.apache.calcite.linq4j.Enumerator enumerator() {\n return new org.apache.calcite.linq4j.Enumerator(){\n public final org.apache.calcite.linq4j.Enumerator inputEnumerator = _inputEnumerable.enumerator();\n public void reset() {\n inputEnumerator.reset();\n }\n\n public boolean moveNext() {\n while (inputEnumerator.moveNext()) {\n if (org.apache.calcite.runtime.SqlFunctions.toLong(((Object[]) inputEnumerator.current())[6]) <= $L4J$C$_Number_org_apache_calcite_linq4j_tree_Primitive_of_long_class_358aa52b) {\n return true;\n }\n }\n return false;\n }\n\n public void close() {\n inputEnumerator.close();\n }\n\n public Object current() {\n final Object[] current = (Object[]) inputEnumerator.current();\n final Object input_value = current[0];\n final Object input_value0 = current[1];\n final Object input_value1 = current[2];\n final Object input_value2 = current[3];\n final Object input_value3 = current[4];\n final Object input_value4 = current[5];\n return new Object[] {\n input_value,\n input_value0,\n input_value1,\n input_value2,\n input_value3,\n input_value4};\n }\n\n static final long $L4J$C$_Number_org_apache_calcite_linq4j_tree_Primitive_of_long_class_358aa52b = ((Number)org.apache.calcite.linq4j.tree.Primitive.of(long.class).numberValueRoundDown(1)).longValue();\n };\n }\n\n };\n return child.take(10000);\n}\n\n\npublic Class getElementType() {\n return java.lang.Object[].class;\n}\n\n\n" + } + } + +Example 4 YAML format (experimental) +----------------------------------- + +.. note:: + YAML explain output is an experimental feature and not intended for + production use. The interface and output may change without notice. + +Return Explain response format in In ``yaml`` format. + +Explain query:: + + sh$ curl -sS -H 'Content-Type: application/json' \ + ... -X POST localhost:9200/_plugins/_ppl/_explain?format=yaml \ + ... -d '{"query" : "source=state_country | where age>3"}' + calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + LogicalProject(name=[$0], country=[$1], state=[$2], month=[$3], year=[$4], age=[$5]) + LogicalFilter(condition=[>($5, 30)]) + CalciteLogicalIndexScan(table=[[OpenSearch, state_country]]) + physical: | + CalciteEnumerableIndexScan(table=[[OpenSearch, state_country]], PushDownContext=[[PROJECT->[name, country, state, month, year, age], FILTER->>($5, 30), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"range":{"age":{"from":30,"to":null,"include_lower":false,"include_upper":true,"boost":1.0}}},"_source":{"includes":["name","country","state","month","year","age"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)]) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 0e22507852e..93013da646b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -12,7 +12,7 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_STRINGS; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOGS; import static org.opensearch.sql.util.MatcherUtils.assertJsonEqualsIgnoreId; -import static org.opensearch.sql.util.MatcherUtils.assertYamlEqualsJsonIgnoreId; +import static org.opensearch.sql.util.MatcherUtils.assertYamlEqualsIgnoreId; import java.io.IOException; import java.util.Locale; @@ -42,9 +42,9 @@ public void testExplainModeUnsupportedInV2() throws IOException {} public void supportSearchSargPushDown_singleRange() throws IOException { String query = "source=opensearch-sql_test_index_account | where age >= 1.0 and age < 10 | fields age"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_sarg_filter_push_single_range.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } // Only for Calcite @@ -160,9 +160,9 @@ public void testExplainMultisearchBasic() throws IOException { + " source=opensearch-sql_test_index_account | where age < 30 | eval age_group =" + " 'young'] [search source=opensearch-sql_test_index_account | where age >= 30 | eval" + " age_group = 'adult'] | stats count by age_group"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_multisearch_basic.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test @@ -172,9 +172,9 @@ public void testExplainMultisearchTimestampInterleaving() throws IOException { + "[search source=opensearch-sql_test_index_time_data | where category IN ('A', 'B')] " + "[search source=opensearch-sql_test_index_time_data2 | where category IN ('E', 'F')] " + "| head 5"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_multisearch_timestamp.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } // Only for Calcite @@ -267,9 +267,9 @@ public void testFilterFunctionScriptPushDownExplain() throws Exception { public void testFilterWithSearchCall() throws IOException { enabledOnlyWhenPushdownIsEnabled(); String expected = loadExpectedPlan("explain_filter_with_search.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | where birthdate >= '2023-01-01 00:00:00' and birthdate < '2023-01-03" + " 00:00:00' | stats count() by span(birthdate, 1d)", @@ -292,22 +292,22 @@ public void testExplainWithReverse() throws IOException { @Test public void testExplainWithTimechartAvg() throws IOException { - var result = explainQueryToString("source=events | timechart span=1m avg(cpu_usage) by host"); + var result = explainQueryYaml("source=events | timechart span=1m avg(cpu_usage) by host"); String expected = !isPushdownDisabled() ? loadFromFile("expectedOutput/calcite/explain_timechart.yaml") : loadFromFile("expectedOutput/calcite/explain_timechart_no_pushdown.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test public void testExplainWithTimechartCount() throws IOException { - var result = explainQueryToString("source=events | timechart span=1m count() by host"); + var result = explainQueryYaml("source=events | timechart span=1m count() by host"); String expected = !isPushdownDisabled() ? loadFromFile("expectedOutput/calcite/explain_timechart_count.yaml") : loadFromFile("expectedOutput/calcite/explain_timechart_count_no_pushdown.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test @@ -346,25 +346,23 @@ public void testExplainStatsWithBinsOnTimeField() throws IOException { // TODO: Remove this after addressing https://github.com/opensearch-project/sql/issues/4317 enabledOnlyWhenPushdownIsEnabled(); String expected = loadExpectedPlan("explain_stats_bins_on_time.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( - "source=events | bin @timestamp bins=3 | stats count() by @timestamp")); + explainQueryYaml("source=events | bin @timestamp bins=3 | stats count() by @timestamp")); expected = loadExpectedPlan("explain_stats_bins_on_time2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=events | bin @timestamp bins=3 | stats avg(cpu_usage) by @timestamp")); } @Test public void testExplainBinWithSpan() throws IOException { String expected = loadExpectedPlan("explain_bin_span.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( - "source=opensearch-sql_test_index_account | bin age span=10 | head 5")); + explainQueryYaml("source=opensearch-sql_test_index_account | bin age span=10 | head 5")); } @Test @@ -388,9 +386,9 @@ public void testExplainBinWithStartEnd() throws IOException { @Test public void testExplainBinWithAligntime() throws IOException { String expected = loadExpectedPlan("explain_bin_aligntime.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_time_data | bin @timestamp span=2h aligntime=latest |" + " head 5")); } @@ -439,9 +437,9 @@ public void testEventstatsDistinctCountFunctionExplain() throws IOException { @Test public void testExplainOnAggregationWithSumEnhancement() throws IOException { String expected = loadExpectedPlan("explain_agg_with_sum_enhancement.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | stats sum(balance), sum(balance + 100), sum(balance - 100)," + " sum(balance * 100), sum(balance / 100) by gender", @@ -569,17 +567,17 @@ public void testValuesAggregationExplain() throws IOException { public void testRegexExplain() throws IOException { String query = "source=opensearch-sql_test_index_account | regex lastname='^[A-Z][a-z]+$' | head 5"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_regex.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test public void testRegexNegatedExplain() throws IOException { String query = "source=opensearch-sql_test_index_account | regex lastname!='.*son$' | head 5"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_regex_negated.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test @@ -605,9 +603,9 @@ public void testRexExplain() throws IOException { String query = "source=opensearch-sql_test_index_account | rex field=lastname \\\"(?^[A-Z])\\\" |" + " head 5"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_rex.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); } @Test @@ -639,9 +637,9 @@ public void testPreventLimitPushdown() throws IOException { enabledOnlyWhenPushdownIsEnabled(); setMaxResultWindow("opensearch-sql_test_index_account", 1); String query = "source=opensearch-sql_test_index_account | head 1 from 1"; - var result = explainQueryToString(query); + var result = explainQueryYaml(query); String expected = loadExpectedPlan("explain_prevent_limit_push.yaml"); - assertYamlEqualsJsonIgnoreId(expected, result); + assertYamlEqualsIgnoreId(expected, result); resetMaxResultWindow("opensearch-sql_test_index_account"); } @@ -654,9 +652,9 @@ public void testPushdownLimitIntoAggregation() throws IOException { explainQueryToString("source=opensearch-sql_test_index_account | stats count() by state")); expected = loadExpectedPlan("explain_limit_agg_pushdown2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count() by state | head 100")); expected = loadExpectedPlan("explain_limit_agg_pushdown3.json"); @@ -718,75 +716,75 @@ public void testCountAggPushDownExplain() throws IOException { enabledOnlyWhenPushdownIsEnabled(); // should be optimized by hits.total.value String expected = loadExpectedPlan("explain_count_agg_push1.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString("source=opensearch-sql_test_index_account | stats count() as cnt")); + explainQueryYaml("source=opensearch-sql_test_index_account | stats count() as cnt")); // should be optimized expected = loadExpectedPlan("explain_count_agg_push2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count(lastname) as cnt")); // should be optimized expected = loadExpectedPlan("explain_count_agg_push3.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | eval name = lastname | stats count(name) as" + " cnt")); // should be optimized expected = loadExpectedPlan("explain_count_agg_push4.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count() as c1, count() as c2")); // should be optimized expected = loadExpectedPlan("explain_count_agg_push5.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count(lastname) as c1," + " count(lastname) as c2")); // should be optimized expected = loadExpectedPlan("explain_count_agg_push6.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | eval name = lastname | stats" + " count(lastname), count(name)")); // should not be optimized expected = loadExpectedPlan("explain_count_agg_push7.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count(balance + 1) as cnt")); // should not be optimized expected = loadExpectedPlan("explain_count_agg_push8.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count() as c1, count(lastname) as" + " c2")); // should not be optimized expected = loadExpectedPlan("explain_count_agg_push9.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats count(firstname), count(lastname)")); // should not be optimized expected = loadExpectedPlan("explain_count_agg_push10.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | eval name = lastname | stats" + " count(firstname), count(name)")); } @@ -796,26 +794,26 @@ public void testExplainCountsByAgg() throws IOException { enabledOnlyWhenPushdownIsEnabled(); String expected = loadExpectedPlan("explain_agg_counts_by1.yaml"); // case of only count(): doc_count works - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | stats count(), count() as c1 by gender", TEST_INDEX_ACCOUNT))); // count(FIELD) by: doc_count doesn't work expected = loadExpectedPlan("explain_agg_counts_by2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | stats count(balance) as c1, count(balance) as c2 by gender", TEST_INDEX_ACCOUNT))); // count(FIELD) by: doc_count doesn't work expected = loadExpectedPlan("explain_agg_counts_by3.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | eval account_number_alias = account_number" + " | stats count(account_number), count(account_number_alias) as c2 by gender", @@ -823,26 +821,26 @@ public void testExplainCountsByAgg() throws IOException { // count() + count(FIELD)): doc_count doesn't work expected = loadExpectedPlan("explain_agg_counts_by4.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | stats count(), count(account_number) by gender", TEST_INDEX_ACCOUNT))); // count(FIELD1) + count(FIELD2)) by: doc_count doesn't work expected = loadExpectedPlan("explain_agg_counts_by5.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | stats count(balance), count(account_number) by gender", TEST_INDEX_ACCOUNT))); // case of count(EXPRESSION) by: doc_count doesn't work expected = loadExpectedPlan("explain_agg_counts_by6.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | eval b_1 = balance + 1" + " | stats count(b_1), count(pow(balance, 2)) as c3 by gender", @@ -940,9 +938,9 @@ public void testExplainPushDownScriptsContainingUDT() throws IOException { @Test public void testFillNullValueSyntaxExplain() throws IOException { String expected = loadExpectedPlan("explain_fillnull_value_syntax.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s | fields age, balance | fillnull value=0", TEST_INDEX_ACCOUNT))); } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java index adc160748e7..83d370f3849 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java @@ -12,7 +12,7 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_TIME_DATA; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOGS; import static org.opensearch.sql.util.MatcherUtils.assertJsonEqualsIgnoreId; -import static org.opensearch.sql.util.MatcherUtils.assertYamlEqualsJsonIgnoreId; +import static org.opensearch.sql.util.MatcherUtils.assertYamlEqualsIgnoreId; import java.io.IOException; import java.util.Locale; @@ -38,9 +38,9 @@ public void init() throws Exception { @Test public void testExplain() throws IOException { String expected = loadExpectedPlan("explain_output.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| where age > 30 " + "| stats avg(age) AS avg_age by state, city " @@ -54,9 +54,9 @@ public void testExplain() throws IOException { @Test public void testFilterPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| where age > 30 " + "| where age < 40 " @@ -67,9 +67,9 @@ public void testFilterPushDownExplain() throws IOException { @Test public void testFilterByCompareStringTimestampPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_push_compare_timestamp_string.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_bank" + "| where birthdate > '2016-12-08 00:00:00.000000000' " + "| where birthdate < '2018-11-09 00:00:00.000000000' ")); @@ -78,9 +78,9 @@ public void testFilterByCompareStringTimestampPushDownExplain() throws IOExcepti @Test public void testFilterByCompareStringDatePushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_push_compare_date_string.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_date_formats | fields yyyy-MM-dd" + "| where yyyy-MM-dd > '2016-12-08 00:00:00.123456789' " + "| where yyyy-MM-dd < '2018-11-09 00:00:00.000000000' ")); @@ -89,9 +89,9 @@ public void testFilterByCompareStringDatePushDownExplain() throws IOException { @Test public void testFilterByCompareStringTimePushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_push_compare_time_string.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_date_formats | fields custom_time" + "| where custom_time > '2016-12-08 12:00:00.123456789' " + "| where custom_time < '2018-11-09 19:00:00.123456789' ")); @@ -141,9 +141,9 @@ public void testWeekArgumentCoercion() throws IOException { @Test public void testFilterAndAggPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_agg_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| where age > 30 " + "| stats avg(age) AS avg_age by state, city")); @@ -172,9 +172,9 @@ public void testSortPushDownExplain() throws IOException { @Test public void testSortWithCountPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_sort_count_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString("source=opensearch-sql_test_index_account | sort 5 age | fields age")); + explainQueryYaml("source=opensearch-sql_test_index_account | sort 5 age | fields age")); } @Test @@ -256,9 +256,9 @@ public void testSortWithRenameExplain() throws IOException { @Test public void testSortThenLimitExplain() throws IOException { String expected = loadExpectedPlan("explain_sort_then_limit_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| sort age " + "| head 5 " @@ -274,9 +274,9 @@ public void testLimitThenSortExplain() throws IOException { // TODO: Fix the expected output in expectedOutput/ppl/explain_limit_then_sort_push.json (v2) // limit-then-sort should not be pushed down. String expected = loadExpectedPlan("explain_limit_then_sort_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 5 " + "| sort age " @@ -286,9 +286,9 @@ public void testLimitThenSortExplain() throws IOException { @Test public void testLimitPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_limit_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| eval ageMinus = age - 30 " + "| head 5 " @@ -298,9 +298,9 @@ public void testLimitPushDownExplain() throws IOException { @Test public void testLimitWithFilterPushdownExplain() throws IOException { String expectedFilterThenLimit = loadExpectedPlan("explain_filter_then_limit_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expectedFilterThenLimit, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| where age > 30 " + "| head 5 " @@ -309,9 +309,9 @@ public void testLimitWithFilterPushdownExplain() throws IOException { // The filter in limit-then-filter queries should not be pushed since the current DSL will // execute it as filter-then-limit String expectedLimitThenFilter = loadExpectedPlan("explain_limit_then_filter_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expectedLimitThenFilter, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 5 " + "| where age > 30 " @@ -321,27 +321,27 @@ public void testLimitWithFilterPushdownExplain() throws IOException { @Test public void testMultipleLimitExplain() throws IOException { String expected5Then10 = loadExpectedPlan("explain_limit_5_10_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected5Then10, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 5 " + "| head 10 " + "| fields age")); String expected10Then5 = loadExpectedPlan("explain_limit_10_5_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected10Then5, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 10 " + "| head 5 " + "| fields age")); String expected10from1then10from2 = loadExpectedPlan("explain_limit_10from1_10from2_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected10from1then10from2, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 10 from 1 " + "| head 10 from 2 " @@ -349,9 +349,9 @@ public void testMultipleLimitExplain() throws IOException { // The second limit should not be pushed down for limit-filter-limit queries String expected10ThenFilterThen5 = loadExpectedPlan("explain_limit_10_filter_5_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected10ThenFilterThen5, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 10 " + "| where age > 30 " @@ -362,9 +362,9 @@ public void testMultipleLimitExplain() throws IOException { @Test public void testLimitWithMultipleOffsetPushdownExplain() throws IOException { String expected = loadExpectedPlan("explain_limit_offsets_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 10 from 1 " + "| head 5 from 2 " @@ -384,9 +384,9 @@ public void testFillNullPushDownExplain() throws IOException { @Test public void testTrendlinePushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_trendline_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 5 " + "| trendline sma(2, age) as ageTrend " @@ -397,9 +397,9 @@ public void testTrendlinePushDownExplain() throws IOException { public void testTrendlineWithSortPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_trendline_sort_push.yaml"); // Sort will not be pushed down because there's a head before it. - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| head 5 " + "| trendline sort age sma(2, age) as ageTrend " @@ -422,17 +422,16 @@ public void testExplainModeUnsupportedInV2() throws IOException { public void testPatternsSimplePatternMethodWithoutAggExplain() throws IOException { // TODO: Correct calcite expected result once pushdown is supported String expected = loadExpectedPlan("explain_patterns_simple_pattern.yaml"); - assertYamlEqualsJsonIgnoreId( - expected, - explainQueryToString("source=opensearch-sql_test_index_account | patterns email")); + assertYamlEqualsIgnoreId( + expected, explainQueryYaml("source=opensearch-sql_test_index_account | patterns email")); } @Test public void testPatternsSimplePatternMethodWithAggPushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_patterns_simple_pattern_agg_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | patterns email mode=aggregation" + " show_numbered_token=true")); } @@ -441,9 +440,9 @@ public void testPatternsSimplePatternMethodWithAggPushDownExplain() throws IOExc public void testPatternsBrainMethodWithAggPushDownExplain() throws IOException { // TODO: Correct calcite expected result once pushdown is supported String expected = loadExpectedPlan("explain_patterns_brain_agg_push.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account" + "| patterns email method=brain mode=aggregation show_numbered_token=true")); } @@ -641,9 +640,9 @@ public void testExplainOnAggregationWithFunction() throws IOException { @Test public void testSearchCommandWithAbsoluteTimeRange() throws IOException { String expected = loadExpectedPlan("search_with_absolute_time_range.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format( "source=%s earliest='2022-12-10 13:11:04' latest='2025-09-03 15:10:00'", TEST_INDEX_TIME_DATA))); @@ -651,32 +650,32 @@ public void testSearchCommandWithAbsoluteTimeRange() throws IOException { @Test public void testSearchCommandWithRelativeTimeRange() throws IOException { - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( loadExpectedPlan("search_with_relative_time_range.yaml"), // "", - explainQueryToString( + explainQueryYaml( String.format("source=%s earliest=-1q latest=+30d", TEST_INDEX_TIME_DATA))); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( loadExpectedPlan("search_with_relative_time_snap.yaml"), - explainQueryToString( + explainQueryYaml( String.format("source=%s earliest='-1q@year' latest=now", TEST_INDEX_TIME_DATA))); } @Test public void testSearchCommandWithNumericTimeRange() throws IOException { String expected = loadExpectedPlan("search_with_numeric_time_range.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( String.format("source=%s earliest=1 latest=1754020061.123456", TEST_INDEX_TIME_DATA))); } @Test public void testSearchCommandWithChainedTimeModifier() throws IOException { - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( loadExpectedPlan("search_with_chained_time_modifier.yaml"), - explainQueryToString( + explainQueryYaml( String.format( "source=%s earliest='-3d@d-2h+10m' latest='-1d+1y@mon'", TEST_INDEX_TIME_DATA))); } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java index 0ac42564192..7c8304b2bfb 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java @@ -37,6 +37,7 @@ public abstract class PPLIntegTestCase extends SQLIntegTestCase { private static final String EXTENDED_EXPLAIN_API_ENDPOINT = "/_plugins/_ppl/_explain?format=extended"; + private static final String YAML_EXPLAIN_API_ENDPOINT = "/_plugins/_ppl/_explain?format=yaml"; private static final Logger LOG = LogManager.getLogger(); @Rule public final RetryProcessor retryProcessor = new RetryProcessor(); @@ -61,6 +62,13 @@ protected String explainQueryToString(String query) throws IOException { return explainQueryToString(query, false); } + protected String explainQueryYaml(String query) throws IOException { + Response response = client().performRequest(buildRequest(query, YAML_EXPLAIN_API_ENDPOINT)); + Assert.assertEquals(200, response.getStatusLine().getStatusCode()); + String responseBody = getResponseBody(response, true); + return responseBody.replace("\\r\\n", "\\n"); + } + protected String explainQueryToYaml(String query) throws IOException { String jsonResponse = explainQueryToString(query); JSONObject jsonObject = jsonify(jsonResponse); diff --git a/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java b/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java index 929397594f0..4e7d72ae530 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java @@ -426,8 +426,8 @@ private static String eliminatePid(String s) { return s.replaceAll("pitId=[^,]+,", "pitId=*,"); } - public static void assertYamlEqualsJsonIgnoreId(String expectedYaml, String actualJson) { - String cleanedYaml = cleanUpYaml(jsonToYaml(actualJson)); + public static void assertYamlEqualsIgnoreId(String expectedYaml, String actualYaml) { + String cleanedYaml = cleanUpYaml(actualYaml); assertYamlEquals(expectedYaml, cleanedYaml); } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java index 94cc8c2fe0f..81b2b5d26dc 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java @@ -93,7 +93,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient nod new ActionListener<>() { @Override public void onResponse(TransportPPLQueryResponse response) { - sendResponse(channel, OK, response.getResult()); + sendResponse(channel, OK, response.getContentType(), response.getResult()); } @Override @@ -129,8 +129,9 @@ public void onFailure(Exception e) { }); } - private void sendResponse(RestChannel channel, RestStatus status, String content) { - channel.sendResponse(new BytesRestResponse(status, "application/json; charset=UTF-8", content)); + private void sendResponse( + RestChannel channel, RestStatus status, String contentType, String content) { + channel.sendResponse(new BytesRestResponse(status, contentType, content)); } private void reportError(final RestChannel channel, final Exception e, final RestStatus status) { diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index fc257931c2a..56fbb4f7953 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -41,6 +41,7 @@ import org.opensearch.sql.protocol.response.format.ResponseFormatter; import org.opensearch.sql.protocol.response.format.SimpleJsonResponseFormatter; import org.opensearch.sql.protocol.response.format.VisualizationResponseFormatter; +import org.opensearch.sql.protocol.response.format.YamlResponseFormatter; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; import org.opensearch.transport.client.node.NodeClient; @@ -109,12 +110,13 @@ protected void doExecute( PPLQueryRequest transformedRequest = transportRequest.toPPLQueryRequest(); if (transformedRequest.isExplainRequest()) { - pplService.explain(transformedRequest, createExplainResponseListener(listener)); + pplService.explain( + transformedRequest, createExplainResponseListener(transformedRequest, listener)); } else { pplService.execute( transformedRequest, createListener(transformedRequest, listener), - createExplainResponseListener(listener)); + createExplainResponseListener(transformedRequest, listener)); } } @@ -124,18 +126,32 @@ protected void doExecute( * legacy module. */ private ResponseListener createExplainResponseListener( - ActionListener listener) { + PPLQueryRequest request, ActionListener listener) { return new ResponseListener() { @Override public void onResponse(ExecutionEngine.ExplainResponse response) { - String responseContent = - new JsonResponseFormatter(PRETTY) { - @Override - protected Object buildJsonObject(ExecutionEngine.ExplainResponse response) { - return response; - } - }.format(response); - listener.onResponse(new TransportPPLQueryResponse(responseContent)); + Optional isYamlFormat = + Format.ofExplain(request.getFormat()).filter(format -> format.equals(Format.YAML)); + ResponseFormatter formatter; + if (isYamlFormat.isPresent()) { + formatter = + new YamlResponseFormatter<>() { + @Override + protected Object buildYamlObject(ExecutionEngine.ExplainResponse response) { + return response; + } + }; + } else { + formatter = + new JsonResponseFormatter<>(PRETTY) { + @Override + protected Object buildJsonObject(ExecutionEngine.ExplainResponse response) { + return response; + } + }; + } + listener.onResponse( + new TransportPPLQueryResponse(formatter.format(response), formatter.contentType())); } @Override diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java index a8d06fa6264..9f3adf95d1f 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java @@ -10,25 +10,36 @@ import java.io.IOException; import java.io.UncheckedIOException; import lombok.Getter; -import lombok.RequiredArgsConstructor; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.InputStreamStreamInput; import org.opensearch.core.common.io.stream.OutputStreamStreamOutput; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -@RequiredArgsConstructor public class TransportPPLQueryResponse extends ActionResponse { @Getter private final String result; + @Getter private final String contentType; + + public TransportPPLQueryResponse(String result) { + this.result = result; + this.contentType = "application/json; charset=UTF-8"; + } + + public TransportPPLQueryResponse(String result, String contentType) { + this.result = result; + this.contentType = contentType; + } public TransportPPLQueryResponse(StreamInput in) throws IOException { super(in); result = in.readString(); + contentType = in.readString(); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(result); + out.writeString(result); } public static TransportPPLQueryResponse fromActionResponse(ActionResponse actionResponse) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java index f4e90395cb5..3a78346ab61 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java @@ -41,6 +41,13 @@ public void testJDBCFormat() { assertEquals(request.format(), Format.JDBC); } + @Test + public void testYAMLFormat() { + PPLQueryRequest request = + new PPLQueryRequest("source=test", null, "/_plugins/_ppl/_explain", "yaml"); + assertEquals(request.format(), Format.YAML); + } + @Test public void testCSVFormat() { PPLQueryRequest request = new PPLQueryRequest("source=test", null, "/_plugins/_ppl", "csv"); diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/Format.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/Format.java index d5537ff5556..14204554ff3 100644 --- a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/Format.java +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/Format.java @@ -22,7 +22,9 @@ public enum Format { SIMPLE("simple"), STANDARD("standard"), EXTENDED("extended"), - COST("cost"); + COST("cost"), + /** Returns explain output in yaml format */ + YAML("yaml"); @Getter private final String formatName; @@ -44,6 +46,7 @@ public enum Format { builder.put(STANDARD.formatName, STANDARD); builder.put(EXTENDED.formatName, EXTENDED); builder.put(COST.formatName, COST); + builder.put(YAML.formatName, YAML); EXPLAIN_FORMATS = builder.build(); } diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java new file mode 100644 index 00000000000..6de39f5bc6a --- /dev/null +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.protocol.response.format; + +import java.security.AccessController; +import java.security.PrivilegedAction; +import lombok.RequiredArgsConstructor; +import org.opensearch.sql.utils.YamlFormatter; + +/** + * Abstract class for all YAML formatter. + * + * @param response generic type which could be DQL or DML response + */ +@RequiredArgsConstructor +public abstract class YamlResponseFormatter implements ResponseFormatter { + + public static final String CONTENT_TYPE = "application/yaml; charset=UTF-8"; + + @Override + public String format(R response) { + return yamlify(buildYamlObject(response)); + } + + @Override + public String format(Throwable t) { + return AccessController.doPrivileged( + (PrivilegedAction) () -> YamlFormatter.formatToYaml(t)); + } + + public String contentType() { + return CONTENT_TYPE; + } + + /** + * Build YAML object to generate response yaml string. + * + * @param response response + * @return yaml object for response + */ + protected abstract Object buildYamlObject(R response); + + protected String yamlify(Object yamlObject) { + return AccessController.doPrivileged( + (PrivilegedAction) () -> YamlFormatter.formatToYaml(yamlObject)); + } +} diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/FormatTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/FormatTest.java index e37823ce829..fbe12ef44b3 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/FormatTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/FormatTest.java @@ -50,6 +50,13 @@ void extended() { assertEquals(Format.EXTENDED, format.get()); } + @Test + void yaml() { + Optional format = Format.ofExplain("yaml"); + assertTrue(format.isPresent()); + assertEquals(Format.YAML, format.get()); + } + @Test void defaultExplainFormat() { Optional format = Format.ofExplain(""); From 6b1de41a671d7101a7ed7f83fecf21ca217c78e6 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 08:43:33 -0700 Subject: [PATCH 02/12] update Signed-off-by: Peng Huo --- docs/user/ppl/interfaces/endpoint.rst | 2 +- .../sql/plugin/transport/TransportPPLQueryResponse.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user/ppl/interfaces/endpoint.rst b/docs/user/ppl/interfaces/endpoint.rst index 5a2e8471549..1fc06639113 100644 --- a/docs/user/ppl/interfaces/endpoint.rst +++ b/docs/user/ppl/interfaces/endpoint.rst @@ -139,7 +139,7 @@ Explain query:: sh$ curl -sS -H 'Content-Type: application/json' \ ... -X POST localhost:9200/_plugins/_ppl/_explain?format=yaml \ - ... -d '{"query" : "source=state_country | where age>3"}' + ... -d '{"query" : "source=state_country | where age>30"}' calcite: logical: | LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java index 9f3adf95d1f..8411b974a2f 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryResponse.java @@ -39,7 +39,7 @@ public TransportPPLQueryResponse(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(result); - out.writeString(result); + out.writeString(contentType); } public static TransportPPLQueryResponse fromActionResponse(ActionResponse actionResponse) { From 071db7ae45c379e8588f23cd14cae03b982b0db2 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 08:57:26 -0700 Subject: [PATCH 03/12] Fix compile issue Signed-off-by: Peng Huo --- .../sql/calcite/remote/CalciteExplainIT.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 15f63a8774d..40a5929abe6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -361,16 +361,16 @@ public void testExplainStatsWithBinsOnTimeField() throws IOException { public void testExplainStatsWithSubAggregation() throws IOException { enabledOnlyWhenPushdownIsEnabled(); String expected = loadExpectedPlan("explain_stats_bins_on_time_and_term.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=events | bin @timestamp bins=3 | stats bucket_nullable=false count() by" + " @timestamp, region")); expected = loadExpectedPlan("explain_stats_bins_on_time_and_term2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=events | bin @timestamp bins=3 | stats bucket_nullable=false avg(cpu_usage) by" + " @timestamp, region")); } @@ -706,14 +706,14 @@ public void testPushdownLimitIntoAggregation() throws IOException { + " 100 | head 10 from 10 ")); expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable1.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, explainQueryToString( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" + " state | head 100 | head 10 from 10 ")); expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, explainQueryToString( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" @@ -886,16 +886,16 @@ public void testExplainSortOnMetricsNoBucketNullable() throws IOException { // TODO enhancement later: https://github.com/opensearch-project/sql/issues/4282 enabledOnlyWhenPushdownIsEnabled(); String expected = loadExpectedPlan("explain_agg_sort_on_metrics1.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" + " state | sort `count()`")); expected = loadExpectedPlan("explain_agg_sort_on_metrics2.yaml"); - assertYamlEqualsJsonIgnoreId( + assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" + " gender, state | sort `count()`")); } From 0464207201828eb3112d56ac25f5d684e2b397a6 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 10:11:15 -0700 Subject: [PATCH 04/12] Fix UT Signed-off-by: Peng Huo --- .../org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java index 3a78346ab61..f4e90395cb5 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/domain/PPLQueryRequestTest.java @@ -41,13 +41,6 @@ public void testJDBCFormat() { assertEquals(request.format(), Format.JDBC); } - @Test - public void testYAMLFormat() { - PPLQueryRequest request = - new PPLQueryRequest("source=test", null, "/_plugins/_ppl/_explain", "yaml"); - assertEquals(request.format(), Format.YAML); - } - @Test public void testCSVFormat() { PPLQueryRequest request = new PPLQueryRequest("source=test", null, "/_plugins/_ppl", "csv"); From f66beb78372515c3fda2aa248925739d2d7eb52e Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 10:56:01 -0700 Subject: [PATCH 05/12] Fix UT Signed-off-by: Peng Huo --- .../format/YamlResponseFormatterTest.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 protocol/src/test/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatterTest.java diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatterTest.java new file mode 100644 index 00000000000..9710dde0a88 --- /dev/null +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatterTest.java @@ -0,0 +1,55 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.protocol.response.format; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.utils.YamlFormatter; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +class YamlResponseFormatterTest { + + private final YamlResponseFormatter formatter = + new YamlResponseFormatter<>() { + @Override + protected Object buildYamlObject(Object response) { + // Pass-through for testing: return the response directly + return response; + } + }; + + @Test + void content_type_matches_yaml() { + assertEquals(YamlResponseFormatter.CONTENT_TYPE, formatter.contentType()); + } + + @Test + void formats_response_via_yaml_formatter() { + Map payload = new LinkedHashMap<>(); + payload.put("b", 2); + payload.put("a", "1"); + + String expected = YamlFormatter.formatToYaml(payload); + String actual = formatter.format(payload); + + assertEquals(expected, actual); + } + + @Test + void formats_throwable_via_yaml_formatter() { + Exception e = new Exception("boom", new RuntimeException("root-cause")); + + String expected = YamlFormatter.formatToYaml(e); + String actual = formatter.format(e); + + assertEquals(expected, actual); + } +} From 2fa0d3f6b793cd7c6e83df5958aba859326d5134 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 14:01:36 -0700 Subject: [PATCH 06/12] Fix IT Signed-off-by: Peng Huo --- .../org/opensearch/sql/calcite/remote/CalciteExplainIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 40a5929abe6..28c2e457574 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -708,14 +708,14 @@ public void testPushdownLimitIntoAggregation() throws IOException { expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable1.yaml"); assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" + " state | head 100 | head 10 from 10 ")); expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable2.yaml"); assertYamlEqualsIgnoreId( expected, - explainQueryToString( + explainQueryYaml( "source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by" + " state | sort state | head 100 | head 10 from 10 ")); From c03facf346b241c62e8bac830760bf63e83b6586 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 14:22:38 -0700 Subject: [PATCH 07/12] Update DocTest Signed-off-by: Peng Huo --- docs/category.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/category.json b/docs/category.json index 56490edfabe..b3b61a538d5 100644 --- a/docs/category.json +++ b/docs/category.json @@ -5,8 +5,7 @@ ], "bash_calcite": [ "user/ppl/interfaces/endpoint.rst", - "user/ppl/interfaces/protocol.rst", - "user/ppl/admin/settings.rst" + "user/ppl/interfaces/protocol.rst" ], "sql_cli": [ "user/dql/expressions.rst", From e8f02779846b19f48115c6e316a27188ad05668a Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 15:31:22 -0700 Subject: [PATCH 08/12] Update Signed-off-by: Peng Huo --- .../java/org/opensearch/sql/ppl/PPLIntegTestCase.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java index 7c8304b2bfb..884931bc05e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java @@ -31,7 +31,6 @@ import org.opensearch.sql.common.setting.Settings.Key; import org.opensearch.sql.legacy.SQLIntegTestCase; import org.opensearch.sql.util.RetryProcessor; -import org.opensearch.sql.utils.YamlFormatter; /** OpenSearch Rest integration test base for PPL testing. */ public abstract class PPLIntegTestCase extends SQLIntegTestCase { @@ -66,13 +65,7 @@ protected String explainQueryYaml(String query) throws IOException { Response response = client().performRequest(buildRequest(query, YAML_EXPLAIN_API_ENDPOINT)); Assert.assertEquals(200, response.getStatusLine().getStatusCode()); String responseBody = getResponseBody(response, true); - return responseBody.replace("\\r\\n", "\\n"); - } - - protected String explainQueryToYaml(String query) throws IOException { - String jsonResponse = explainQueryToString(query); - JSONObject jsonObject = jsonify(jsonResponse); - return YamlFormatter.formatToYaml(jsonObject); + return responseBody; } protected String explainQueryToString(String query, boolean extended) throws IOException { From aa3b21fc7a4480fc0f13f8f8196e5b7675c9ff6f Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 16:08:24 -0700 Subject: [PATCH 09/12] Update Signed-off-by: Peng Huo --- core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java index c5cbfd9df93..04c3773c5f0 100644 --- a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java +++ b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java @@ -23,6 +23,7 @@ public class YamlFormatter { static { YAMLFactory yamlFactory = new YAMLFactory(); yamlFactory.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER); + yamlFactory.disable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS); yamlFactory.enable(YAMLGenerator.Feature.MINIMIZE_QUOTES); // Enable smart quoting yamlFactory.enable( YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS); // Quote numeric strings From 454bf833c6cfd1c6da29408a2df5d37aea420666 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 16:40:23 -0700 Subject: [PATCH 10/12] Update Signed-off-by: Peng Huo --- core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java index 04c3773c5f0..e0f339abb48 100644 --- a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java +++ b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java @@ -24,6 +24,7 @@ public class YamlFormatter { YAMLFactory yamlFactory = new YAMLFactory(); yamlFactory.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER); yamlFactory.disable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS); + yamlFactory.enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE); yamlFactory.enable(YAMLGenerator.Feature.MINIMIZE_QUOTES); // Enable smart quoting yamlFactory.enable( YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS); // Quote numeric strings From 64bcd968e607f7e2ab853eeea4ef8fe558e204e3 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 17:39:42 -0700 Subject: [PATCH 11/12] Update Signed-off-by: Peng Huo --- core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java | 2 +- .../sql/protocol/response/format/YamlResponseFormatter.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java index e0f339abb48..c50d04f8217 100644 --- a/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java +++ b/core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java @@ -23,7 +23,7 @@ public class YamlFormatter { static { YAMLFactory yamlFactory = new YAMLFactory(); yamlFactory.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER); - yamlFactory.disable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS); + yamlFactory.enable(YAMLGenerator.Feature.USE_PLATFORM_LINE_BREAKS); yamlFactory.enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE); yamlFactory.enable(YAMLGenerator.Feature.MINIMIZE_QUOTES); // Enable smart quoting yamlFactory.enable( diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java index 6de39f5bc6a..da3022fbc82 100644 --- a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/YamlResponseFormatter.java @@ -27,8 +27,7 @@ public String format(R response) { @Override public String format(Throwable t) { - return AccessController.doPrivileged( - (PrivilegedAction) () -> YamlFormatter.formatToYaml(t)); + return yamlify(t); } public String contentType() { From 0e01e7bb21f7c4516541644213f0f5e314412336 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 10 Oct 2025 19:12:12 -0700 Subject: [PATCH 12/12] Update Signed-off-by: Peng Huo --- .../opensearch/sql/executor/ExecutionEngine.java | 16 ++++++++++++++++ .../transport/TransportPPLQueryAction.java | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java b/core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java index 78b0ead1c8c..ec1e427e365 100644 --- a/core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java +++ b/core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java @@ -109,6 +109,22 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(root, calcite); } + + public static ExplainResponse normalizeLf(ExplainResponse response) { + ExecutionEngine.ExplainResponseNodeV2 calcite = response.getCalcite(); + if (calcite != null) { + return new ExplainResponse( + new ExecutionEngine.ExplainResponseNodeV2( + normalizeLf(calcite.getLogical()), + normalizeLf(calcite.getPhysical()), + normalizeLf(calcite.getExtended()))); + } + return response; + } + + private static String normalizeLf(String value) { + return value == null ? null : value.replace("\r\n", "\n"); + } } @AllArgsConstructor diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index c7632b7900e..b53e41b0ee4 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -6,6 +6,7 @@ package org.opensearch.sql.plugin.transport; import static org.opensearch.rest.BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX; +import static org.opensearch.sql.executor.ExecutionEngine.ExplainResponse.normalizeLf; import static org.opensearch.sql.lang.PPLLangSpec.PPL_SPEC; import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; @@ -139,7 +140,7 @@ public void onResponse(ExecutionEngine.ExplainResponse response) { new YamlResponseFormatter<>() { @Override protected Object buildYamlObject(ExecutionEngine.ExplainResponse response) { - return response; + return normalizeLf(response); } }; } else {