From fe0f9055d8de4626b2eb83ece4b63cdcadfdad75 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 20 Jun 2019 10:11:13 +0300 Subject: [PATCH] Fix NPE in case of subsequent scrolled requests for a CSV/TSV formatted response (#43365) (cherry picked from commit 0ef7bb0f8b07cd0392d37f96ca9360821b19315a) --- .../xpack/sql/qa/rest/RestSqlTestCase.java | 94 +++++++++++-------- .../xpack/sql/plugin/TextFormat.java | 2 +- 2 files changed, 55 insertions(+), 41 deletions(-) diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java index 96d8ffe455c..7c7288d6a35 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java @@ -621,46 +621,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe } public void testNextPageText() throws IOException { - int size = 20; - String[] docs = new String[size]; - for (int i = 0; i < size; i++) { - docs[i] = "{\"text\":\"text" + i + "\", \"number\":" + i + "}\n"; - } - index(docs); - - String request = "{\"query\":\"SELECT text, number, number + 5 AS sum FROM test ORDER BY number\", \"fetch_size\":2}"; - - String cursor = null; - for (int i = 0; i < 20; i += 2) { - Tuple response; - if (i == 0) { - response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain"); - } else { - response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}", - ContentType.APPLICATION_JSON), "text/plain"); - } - - StringBuilder expected = new StringBuilder(); - if (i == 0) { - expected.append(" text | number | sum \n"); - expected.append("---------------+---------------+---------------\n"); - } - expected.append(String.format(Locale.ROOT, "%-15s|%-15d|%-15d\n", "text" + i, i, i + 5)); - expected.append(String.format(Locale.ROOT, "%-15s|%-15d|%-15d\n", "text" + (i + 1), i + 1, i + 6)); - cursor = response.v2(); - assertEquals(expected.toString(), response.v1()); - assertNotNull(cursor); - } - Map expected = new HashMap<>(); - expected.put("rows", emptyList()); - assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), - StringUtils.EMPTY)); - - Map response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), - "/close"); - assertEquals(true, response.get("succeeded")); - - assertEquals(0, getNumberOfSearchContexts("test")); + executeQueryWithNextPage("text/plain", " text | number | sum \n", "%-15s|%-15d|%-15d\n"); } // CSV/TSV tests @@ -702,6 +663,10 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe Tuple response = runSqlAsText(query, "text/csv; header=absent"); assertEquals(expected, response.v1()); } + + public void testNextPageCSV() throws IOException { + executeQueryWithNextPage("text/csv; header=present", "text,number,sum\r\n", "%s,%d,%d\r\n"); + } public void testQueryInTSV() throws IOException { index("{\"name\":" + toJson("first") + ", \"number\" : 1 }", @@ -720,6 +685,55 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe response = runSqlAsTextFormat(query, "tsv"); assertEquals(expected, response.v1()); } + + public void testNextPageTSV() throws IOException { + executeQueryWithNextPage("text/tab-separated-values", "text\tnumber\tsum\n", "%s\t%d\t%d\n"); + } + + private void executeQueryWithNextPage(String format, String expectedHeader, String expectedLineFormat) throws IOException { + int size = 20; + String[] docs = new String[size]; + for (int i = 0; i < size; i++) { + docs[i] = "{\"text\":\"text" + i + "\", \"number\":" + i + "}\n"; + } + index(docs); + + String request = "{\"query\":\"SELECT text, number, number + 5 AS sum FROM test ORDER BY number\", \"fetch_size\":2}"; + + String cursor = null; + for (int i = 0; i < 20; i += 2) { + Tuple response; + if (i == 0) { + response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), format); + } else { + response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}", + ContentType.APPLICATION_JSON), format); + } + + StringBuilder expected = new StringBuilder(); + if (i == 0) { + expected.append(expectedHeader); + if (format == "text/plain") { + expected.append("---------------+---------------+---------------\n"); + } + } + expected.append(String.format(Locale.ROOT, expectedLineFormat, "text" + i, i, i + 5)); + expected.append(String.format(Locale.ROOT, expectedLineFormat, "text" + (i + 1), i + 1, i + 6)); + cursor = response.v2(); + assertEquals(expected.toString(), response.v1()); + assertNotNull(cursor); + } + Map expected = new HashMap<>(); + expected.put("rows", emptyList()); + assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), + StringUtils.EMPTY)); + + Map response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON), + "/close"); + assertEquals(true, response.get("succeeded")); + + assertEquals(0, getNumberOfSearchContexts("test")); + } private Tuple runSqlAsText(String sql, String accept) throws IOException { return runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java index 62963a99b2a..f4e3e006e70 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java @@ -224,7 +224,7 @@ enum TextFormat { boolean header = hasHeader(request); - if (header) { + if (header && (cursor == null || cursor == Cursor.EMPTY)) { row(sb, response.columns(), ColumnInfo::name); }