From 226e8499d7a898e2f9e9de26e322916901aa98fc Mon Sep 17 00:00:00 2001 From: Radhakrishnan Pachyappan Date: Sun, 5 Jul 2026 11:44:10 +0530 Subject: [PATCH 1/3] Fix SQL explain API rejecting format=json parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isSupportedExplainFormat() only accepted "simple", "standard", "extended", and "cost" — excluding "json". Before OpenSearch 3.0, ?format=json was valid for the explain endpoint, so existing workflows that pass this parameter now receive: "Failed to create executor due to unknown response format: json" The explain endpoint already returns JSON unconditionally regardless of the format parameter, so accepting "json" is a pure backward- compatibility restoration with no behavioral change. Fixes #4373 Signed-off-by: Radhakrishnan Pachyappan --- .../sql/sql/domain/SQLQueryRequest.java | 6 +++++- .../sql/sql/domain/SQLQueryRequestTest.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java b/sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java index 456ea212717..a9d70126873 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java +++ b/sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java @@ -159,7 +159,11 @@ private boolean isSupportedFormat() { } private boolean isSupportedExplainFormat() { - return Stream.of("simple", "standard", "extended", "cost").anyMatch(format::equalsIgnoreCase); + // "json" is accepted for backward compatibility: the explain endpoint always returns JSON + // regardless of this parameter, so treating it as valid avoids the 400 regression + // introduced in OpenSearch 3.0. See https://github.com/opensearch-project/sql/issues/4373 + return Stream.of("simple", "standard", "extended", "cost", "json") + .anyMatch(format::equalsIgnoreCase); } private String getFormat(Map params) { diff --git a/sql/src/test/java/org/opensearch/sql/sql/domain/SQLQueryRequestTest.java b/sql/src/test/java/org/opensearch/sql/sql/domain/SQLQueryRequestTest.java index e5f2400e6cb..5c8b12256dd 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/domain/SQLQueryRequestTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/domain/SQLQueryRequestTest.java @@ -104,6 +104,22 @@ public void should_support_explain_format() { () -> assertTrue(explainRequest.isSupported())); } + @Test + public void should_support_explain_with_json_format() { + // Regression test for https://github.com/opensearch-project/sql/issues/4373. + // ?format=json was accepted before OpenSearch 3.0 and should continue to be valid. + // The explain endpoint always returns JSON regardless of this parameter. + SQLQueryRequest explainRequest = + SQLQueryRequestBuilder.request("SELECT 1") + .path("_plugins/_sql/_explain") + .params(Map.of("format", "json")) + .build(); + + assertAll( + () -> assertTrue(explainRequest.isExplainRequest()), + () -> assertTrue(explainRequest.isSupported())); + } + @Test public void should_not_support_explain_with_unsupported_explain_format() { SQLQueryRequest explainRequest = From 2a4d6a4c08a690138b5eef3201fd0e1e4223907b Mon Sep 17 00:00:00 2001 From: Radhakrishnan P Date: Wed, 2 Sep 2026 09:33:55 +0530 Subject: [PATCH 2/3] Fix explain endpoint crash on format=json at the actual root cause The original fix in this PR only touched SQLQueryRequest#isSupportedExplainFormat() (the V2/sql-module engine's own validation), but that code is never reached for this bug: RestSqlAction#prepareRequest calls SqlRequestParam.getFormat(params) unconditionally, for every request, before SQLQueryRequest is even constructed. That call throws IllegalArgumentException for any format outside the legacy Format enum (jdbc/csv/raw/table) - "json" included - which is exactly the "Failed to create executor due to unknown response format: json" error from #4373. So format=json on /_explain was failing before either engine ever saw the request. Add RestSqlAction#resolveFormat, which falls back to a default Format when parsing fails AND the request is an explain request AND the rejected value is "json" - explain's response body is JSON regardless of this parameter (executeSqlRequest's explain branch calls queryAction.explain().explain() directly, never through a Format-specific executor), so ignoring it here changes nothing else about the response. The earlier SQLQueryRequest#isSupportedExplainFormat() change stays: once a request past this point, it makes the V2 engine treat format=json as supported so it handles the explain natively instead of always falling back to the legacy engine. Signed-off-by: Radhakrishnan P --- .../sql/legacy/plugin/RestSqlAction.java | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java index 4064b73d4a4..cc772ef83ea 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java @@ -143,7 +143,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli LOG.info("[{}] Incoming request {}", QueryContext.getRequestId(), request.uri()); - Format format = SqlRequestParam.getFormat(request.params()); + Format format = resolveFormat(request); SQLQueryRequest newSqlRequest = new SQLQueryRequest( @@ -320,6 +320,29 @@ private static boolean isExplainRequest(final RestRequest request) { return request.path().endsWith("/_explain"); } + /** + * Resolve the response {@link Format} for this request. + * + *

{@code format=json} is accepted for explain requests for backward compatibility: prior to + * OpenSearch 3.0, {@code ?format=json} was a valid way to request the explain plan (see #4373). + * It was never a real member of {@link Format} - {@code SqlRequestParam#getFormat} always + * rejected it - but the explain response body is JSON regardless of this parameter (see {@link + * #executeSqlRequest}, which calls {@code queryAction.explain().explain()} directly instead of + * going through a {@link Format}-specific executor), so the parameter can simply be ignored here + * for explain requests without changing any actual behavior. + */ + private static Format resolveFormat(final RestRequest request) { + try { + return SqlRequestParam.getFormat(request.params()); + } catch (IllegalArgumentException e) { + if (isExplainRequest(request) + && "json".equalsIgnoreCase(request.param(SqlRequestParam.QUERY_PARAMS_FORMAT))) { + return Format.JDBC; + } + throw e; + } + } + private static boolean isClientError(Exception e) { return e instanceof From 26ce5c5146e035a2271b30cb6025d67204c8ffee Mon Sep 17 00:00:00 2001 From: Radhakrishnan P Date: Wed, 2 Sep 2026 09:34:06 +0530 Subject: [PATCH 3/3] Add regression IT for explain endpoint with format=json Per review, cover this with an integration test rather than only a unit test of SQLQueryRequest#isSupportedExplainFormat() in isolation - that unit test alone gave false confidence, since the real bug was in RestSqlAction, a layer up, which the unit test never exercised. There's no YAML REST-spec test coverage for any SQL endpoint yet (only ppl/ppl.explain/ppl.grammar/query.settings action specs exist under integ-test/src/yamlRestTest/resources/rest-api-spec/api/), so this follows the existing Java IT pattern used for the rest of legacy/ExplainIT.java instead of introducing a new sql.explain action spec for a single test. Signed-off-by: Radhakrishnan P --- .../org/opensearch/sql/legacy/ExplainIT.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/ExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/ExplainIT.java index 1b740924c44..96b2c7b4940 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/ExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/ExplainIT.java @@ -13,11 +13,13 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_PEOPLE; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_PHRASE; +import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT; import com.google.common.io.Files; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import org.json.JSONObject; import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; @@ -253,4 +255,25 @@ public void testContentTypeOfExplainRequestShouldBeJson() throws IOException { assertEquals("application/json; charset=UTF-8", response.getHeader("content-type")); } + + /** + * Prior to OpenSearch 3.0, {@code ?format=json} was a valid way to request the explain plan. + * "json" was never a real {@code Format} value the query endpoint accepts (only + * jdbc/csv/raw/table are), so this depends on the explain endpoint specifically tolerating it. + * Regression test for https://github.com/opensearch-project/sql/issues/4373: this used to fail + * with "Failed to create executor due to unknown response format: json" before the explain + * endpoint reached any query-specific logic at all. + */ + @Test + public void testExplainAcceptsJsonFormatForBackwardCompatibility() throws IOException { + String query = makeRequest("SELECT firstname FROM opensearch-sql_test_index_account"); + Request request = new Request("POST", EXPLAIN_API_ENDPOINT + "?format=json"); + request.setJsonEntity(query); + + Response response = client().performRequest(request); + + assertEquals(200, response.getStatusLine().getStatusCode()); + JSONObject explanation = new JSONObject(TestUtils.getResponseBody(response)); + Assert.assertFalse("explain response should not contain an error", explanation.has("error")); + } }