Skip to content

Commit 0234419

Browse files
committed
Fix NPE in case of subsequent scrolled requests for a CSV/TSV formatted response (#43365)
(cherry picked from commit 0ef7bb0)
1 parent 4d73096 commit 0234419

File tree

2 files changed

+55
-41
lines changed

2 files changed

+55
-41
lines changed

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

Lines changed: 54 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -621,46 +621,7 @@ public void testBasicQueryText() throws IOException {
621621
}
622622

623623
public void testNextPageText() throws IOException {
624-
int size = 20;
625-
String[] docs = new String[size];
626-
for (int i = 0; i < size; i++) {
627-
docs[i] = "{\"text\":\"text" + i + "\", \"number\":" + i + "}\n";
628-
}
629-
index(docs);
630-
631-
String request = "{\"query\":\"SELECT text, number, number + 5 AS sum FROM test ORDER BY number\", \"fetch_size\":2}";
632-
633-
String cursor = null;
634-
for (int i = 0; i < 20; i += 2) {
635-
Tuple<String, String> response;
636-
if (i == 0) {
637-
response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), "text/plain");
638-
} else {
639-
response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
640-
ContentType.APPLICATION_JSON), "text/plain");
641-
}
642-
643-
StringBuilder expected = new StringBuilder();
644-
if (i == 0) {
645-
expected.append(" text | number | sum \n");
646-
expected.append("---------------+---------------+---------------\n");
647-
}
648-
expected.append(String.format(Locale.ROOT, "%-15s|%-15d|%-15d\n", "text" + i, i, i + 5));
649-
expected.append(String.format(Locale.ROOT, "%-15s|%-15d|%-15d\n", "text" + (i + 1), i + 1, i + 6));
650-
cursor = response.v2();
651-
assertEquals(expected.toString(), response.v1());
652-
assertNotNull(cursor);
653-
}
654-
Map<String, Object> expected = new HashMap<>();
655-
expected.put("rows", emptyList());
656-
assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
657-
StringUtils.EMPTY));
658-
659-
Map<String, Object> response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
660-
"/close");
661-
assertEquals(true, response.get("succeeded"));
662-
663-
assertEquals(0, getNumberOfSearchContexts("test"));
624+
executeQueryWithNextPage("text/plain", " text | number | sum \n", "%-15s|%-15d|%-15d\n");
664625
}
665626

666627
// CSV/TSV tests
@@ -702,6 +663,10 @@ public void testQueryWithoutHeaderInCSV() throws IOException {
702663
Tuple<String, String> response = runSqlAsText(query, "text/csv; header=absent");
703664
assertEquals(expected, response.v1());
704665
}
666+
667+
public void testNextPageCSV() throws IOException {
668+
executeQueryWithNextPage("text/csv; header=present", "text,number,sum\r\n", "%s,%d,%d\r\n");
669+
}
705670

706671
public void testQueryInTSV() throws IOException {
707672
index("{\"name\":" + toJson("first") + ", \"number\" : 1 }",
@@ -720,6 +685,55 @@ public void testQueryInTSV() throws IOException {
720685
response = runSqlAsTextFormat(query, "tsv");
721686
assertEquals(expected, response.v1());
722687
}
688+
689+
public void testNextPageTSV() throws IOException {
690+
executeQueryWithNextPage("text/tab-separated-values", "text\tnumber\tsum\n", "%s\t%d\t%d\n");
691+
}
692+
693+
private void executeQueryWithNextPage(String format, String expectedHeader, String expectedLineFormat) throws IOException {
694+
int size = 20;
695+
String[] docs = new String[size];
696+
for (int i = 0; i < size; i++) {
697+
docs[i] = "{\"text\":\"text" + i + "\", \"number\":" + i + "}\n";
698+
}
699+
index(docs);
700+
701+
String request = "{\"query\":\"SELECT text, number, number + 5 AS sum FROM test ORDER BY number\", \"fetch_size\":2}";
702+
703+
String cursor = null;
704+
for (int i = 0; i < 20; i += 2) {
705+
Tuple<String, String> response;
706+
if (i == 0) {
707+
response = runSqlAsText(StringUtils.EMPTY, new StringEntity(request, ContentType.APPLICATION_JSON), format);
708+
} else {
709+
response = runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"cursor\":\"" + cursor + "\"}",
710+
ContentType.APPLICATION_JSON), format);
711+
}
712+
713+
StringBuilder expected = new StringBuilder();
714+
if (i == 0) {
715+
expected.append(expectedHeader);
716+
if (format == "text/plain") {
717+
expected.append("---------------+---------------+---------------\n");
718+
}
719+
}
720+
expected.append(String.format(Locale.ROOT, expectedLineFormat, "text" + i, i, i + 5));
721+
expected.append(String.format(Locale.ROOT, expectedLineFormat, "text" + (i + 1), i + 1, i + 6));
722+
cursor = response.v2();
723+
assertEquals(expected.toString(), response.v1());
724+
assertNotNull(cursor);
725+
}
726+
Map<String, Object> expected = new HashMap<>();
727+
expected.put("rows", emptyList());
728+
assertResponse(expected, runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
729+
StringUtils.EMPTY));
730+
731+
Map<String, Object> response = runSql(new StringEntity("{\"cursor\":\"" + cursor + "\"}", ContentType.APPLICATION_JSON),
732+
"/close");
733+
assertEquals(true, response.get("succeeded"));
734+
735+
assertEquals(0, getNumberOfSearchContexts("test"));
736+
}
723737

724738
private Tuple<String, String> runSqlAsText(String sql, String accept) throws IOException {
725739
return runSqlAsText(StringUtils.EMPTY, new StringEntity("{\"query\":\"" + sql + "\"}", ContentType.APPLICATION_JSON), accept);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ String format(Cursor cursor, RestRequest request, SqlQueryResponse response) {
224224

225225
boolean header = hasHeader(request);
226226

227-
if (header) {
227+
if (header && (cursor == null || cursor == Cursor.EMPTY)) {
228228
row(sb, response.columns(), ColumnInfo::name);
229229
}
230230

0 commit comments

Comments
 (0)