Remove TextFormat as an instance variable (elastic#69895)

(cherry picked from commit 20f9397)
astefan · Mar 3, 2021 · 77ac0e5 · 77ac0e5
1 parent bc552d6
commit 77ac0e5
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/...ugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java b/...ugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java
@@ -934,6 +934,35 @@ public void testNextPageCSV() throws IOException {
         executeQueryWithNextPage("text/csv; header=present", "text,number,sum\r\n", "%s,%d,%d\r\n");
     }
 
+    public void testCSVWithDelimiterParameter() throws IOException {
+        String format = randomFrom("txt", "tsv", "json", "yaml", "smile", "cbor");
+        String query = "SELECT * FROM test";
+        index("{\"foo\":1}");
+
+        Request badRequest = new Request("POST", SQL_QUERY_REST_ENDPOINT);
+        badRequest.addParameter("format", format);
+        badRequest.addParameter("delimiter", ";");
+        badRequest.setEntity(
+            new StringEntity(
+                query(query).mode(randomValueOtherThan(Mode.JDBC.toString(), BaseRestSqlTestCase::randomMode)).toString(),
+                ContentType.APPLICATION_JSON
+            )
+        );
+        expectBadRequest(() -> {
+            client().performRequest(badRequest);
+            return Collections.emptyMap();
+        }, containsString("request [/_sql] contains unrecognized parameter: [delimiter]"));
+
+        Request csvRequest = new Request("POST", SQL_QUERY_REST_ENDPOINT + "?format=csv&delimiter=%3B");
+        csvRequest.setEntity(
+            new StringEntity(
+                query(query).mode(randomValueOtherThan(Mode.JDBC.toString(), BaseRestSqlTestCase::randomMode)).toString(),
+                ContentType.APPLICATION_JSON
+            )
+        );
+        assertOK(client().performRequest(csvRequest));
+    }
+
     public void testQueryInTSV() throws IOException {
         index(
             "{\"name\":" + toJson("first") + ", \"number\" : 1 }",

diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java
@@ -31,6 +31,7 @@
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
+import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableList;
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
@@ -39,8 +40,6 @@
 
 public class RestSqlQueryAction extends BaseRestHandler {
 
-    TextFormat textFormat;
-
     @Override
     public List<Route> routes() {
         return emptyList();
@@ -106,13 +105,23 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
          * which we turn into a 400 error.
          */
         XContentType xContentType = accept == null ? XContentType.JSON : XContentType.fromMediaTypeOrFormat(accept);
-        textFormat = xContentType == null ? TextFormat.fromMediaTypeOrFormat(accept) : null;
+        TextFormat textFormat = xContentType == null ? TextFormat.fromMediaTypeOrFormat(accept) : null;
 
         if (xContentType == null && sqlRequest.columnar()) {
             throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with "
                     + "txt, csv or tsv formats");
         }
 
+        /*
+         * Special handling for the "delimiter" parameter which should only be
+         * checked for being present or not in the case of CSV format. We cannot
+         * override {@link BaseRestHandler#responseParams()} because this
+         * parameter should only be checked for CSV, not always.
+         */
+        if ((textFormat == null || textFormat != TextFormat.CSV) && request.hasParam(URL_PARAM_DELIMITER)) {
+            throw new IllegalArgumentException(unrecognized(request, Collections.singleton(URL_PARAM_DELIMITER), emptySet(), "parameter"));
+        }
+
         long startNanos = System.nanoTime();
         return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener<SqlQueryResponse>(channel) {
             @Override
@@ -145,7 +154,7 @@ public RestResponse buildResponse(SqlQueryResponse response) throws Exception {
 
     @Override
     protected Set<String> responseParams() {
-        return textFormat == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet();
+        return Collections.singleton(URL_PARAM_DELIMITER);
     }
 
     @Override