ESQL: Drop null columns in text formats (elastic#117643)

This PR resolves the issue where, despite setting `drop_null_columns=true`, columns that are entirely null are still returned when using `format=txt`, `format=csv`, or `format=tsv`.

Closes elastic#116848

docs/changelog/117643.yaml

@@ -0,0 +1,6 @@
+pr: 117643
+summary: Drop null columns in text formats
+area: ES|QL
+type: bug
+issues:
+  - 116848

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

@@ -1119,7 +1119,7 @@ public void testAsyncGetWithoutContentType() throws IOException {
         var json = entityToMap(entity, requestObject.contentType());
         checkKeepOnCompletion(requestObject, json, true);
         String id = (String) json.get("id");
-        // results won't be returned since keepOnCompletion is true
+        // results won't be returned because wait_for_completion is provided a very small interval
         assertThat(id, is(not(emptyOrNullString())));
 
         // issue an "async get" request with no Content-Type
@@ -1274,11 +1274,11 @@ static String runEsqlAsTextWithFormat(RequestObjectBuilder builder, String forma
                 switch (format) {
                     case "txt" -> assertThat(initialValue, emptyOrNullString());
                     case "csv" -> {
-                        assertEquals(initialValue, "\r\n");
+                        assertEquals("\r\n", initialValue);
                         initialValue = "";
                     }
                     case "tsv" -> {
-                        assertEquals(initialValue, "\n");
+                        assertEquals("\n", initialValue);
                         initialValue = "";
                     }
                 }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponse.java

@@ -218,7 +218,7 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params
         });
     }
 
-    private boolean[] nullColumns() {
+    public boolean[] nullColumns() {
         boolean[] nullColumns = new boolean[columns.size()];
         for (int c = 0; c < nullColumns.length; c++) {
             nullColumns[c] = allColumnsAreNull(c);

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

@@ -39,7 +39,8 @@ public enum TextFormat implements MediaType {
     PLAIN_TEXT() {
         @Override
         public Iterator<CheckedConsumer<Writer, IOException>> format(RestRequest request, EsqlQueryResponse esqlResponse) {
-            return new TextFormatter(esqlResponse).format(hasHeader(request));
+            boolean dropNullColumns = request.paramAsBoolean(DROP_NULL_COLUMNS_OPTION, false);
+            return new TextFormatter(esqlResponse, hasHeader(request), dropNullColumns).format();
         }
 
         @Override
@@ -282,15 +283,21 @@ public Set<HeaderValue> headerValues() {
      */
     public static final String URL_PARAM_FORMAT = "format";
     public static final String URL_PARAM_DELIMITER = "delimiter";
+    public static final String DROP_NULL_COLUMNS_OPTION = "drop_null_columns";
 
     public Iterator<CheckedConsumer<Writer, IOException>> format(RestRequest request, EsqlQueryResponse esqlResponse) {
         final var delimiter = delimiter(request);
+        boolean dropNullColumns = request.paramAsBoolean(DROP_NULL_COLUMNS_OPTION, false);
+        boolean[] dropColumns = dropNullColumns ? esqlResponse.nullColumns() : new boolean[esqlResponse.columns().size()];
         return Iterators.concat(
             // if the header is requested return the info
             hasHeader(request) && esqlResponse.columns() != null
-                ? Iterators.single(writer -> row(writer, esqlResponse.columns().iterator(), ColumnInfo::name, delimiter))
+                ? Iterators.single(writer -> row(writer, esqlResponse.columns().iterator(), ColumnInfo::name, delimiter, dropColumns))
                 : Collections.emptyIterator(),
-            Iterators.map(esqlResponse.values(), row -> writer -> row(writer, row, f -> Objects.toString(f, StringUtils.EMPTY), delimiter))
+            Iterators.map(
+                esqlResponse.values(),
+                row -> writer -> row(writer, row, f -> Objects.toString(f, StringUtils.EMPTY), delimiter, dropColumns)
+            )
         );
     }
 
@@ -313,9 +320,14 @@ public String contentType(RestRequest request) {
     }
 
     // utility method for consuming a row.
-    <F> void row(Writer writer, Iterator<F> row, Function<F, String> toString, Character delimiter) throws IOException {
+    <F> void row(Writer writer, Iterator<F> row, Function<F, String> toString, Character delimiter, boolean[] dropColumns)
+        throws IOException {
         boolean firstColumn = true;
-        while (row.hasNext()) {
+        for (int i = 0; row.hasNext(); i++) {
+            if (dropColumns[i]) {
+                row.next();
+                continue;
+            }
             if (firstColumn) {
                 firstColumn = false;
             } else {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/formatter/TextFormatter.java

@@ -30,13 +30,17 @@ public class TextFormatter {
     private final EsqlQueryResponse response;
     private final int[] width;
     private final Function<Object, String> FORMATTER = Objects::toString;
+    private final boolean includeHeader;
+    private final boolean[] dropColumns;
 
     /**
-     * Create a new {@linkplain TextFormatter} for formatting responses.
+     * Create a new {@linkplain TextFormatter} for formatting responses
      */
-    public TextFormatter(EsqlQueryResponse response) {
+    public TextFormatter(EsqlQueryResponse response, boolean includeHeader, boolean dropNullColumns) {
         this.response = response;
         var columns = response.columns();
+        this.includeHeader = includeHeader;
+        this.dropColumns = dropNullColumns ? response.nullColumns() : new boolean[columns.size()];
         // Figure out the column widths:
         // 1. Start with the widths of the column names
         width = new int[columns.size()];
@@ -58,19 +62,22 @@ public TextFormatter(EsqlQueryResponse response) {
     }
 
     /**
-     * Format the provided {@linkplain EsqlQueryResponse} optionally including the header lines.
+     * Format the provided {@linkplain EsqlQueryResponse}
      */
-    public Iterator<CheckedConsumer<Writer, IOException>> format(boolean includeHeader) {
+    public Iterator<CheckedConsumer<Writer, IOException>> format() {
         return Iterators.concat(
             // The header lines
-            includeHeader && response.columns().size() > 0 ? Iterators.single(this::formatHeader) : Collections.emptyIterator(),
+            includeHeader && response.columns().isEmpty() == false ? Iterators.single(this::formatHeader) : Collections.emptyIterator(),
             // Now format the results.
             formatResults()
         );
     }
 
     private void formatHeader(Writer writer) throws IOException {
         for (int i = 0; i < width.length; i++) {
+            if (dropColumns[i]) {
+                continue;
+            }
             if (i > 0) {
                 writer.append('|');
             }
@@ -86,6 +93,9 @@ private void formatHeader(Writer writer) throws IOException {
         writer.append('\n');
 
         for (int i = 0; i < width.length; i++) {
+            if (dropColumns[i]) {
+                continue;
+            }
             if (i > 0) {
                 writer.append('+');
             }
@@ -98,6 +108,10 @@ private Iterator<CheckedConsumer<Writer, IOException>> formatResults() {
         return Iterators.map(response.values(), row -> writer -> {
             for (int i = 0; i < width.length; i++) {
                 assert row.hasNext();
+                if (dropColumns[i]) {
+                    row.next();
+                    continue;
+                }
                 if (i > 0) {
                     writer.append('|');
                 }

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/formatter/TextFormatTests.java

@@ -123,17 +123,17 @@ public void testTsvFormatWithEmptyData() {
     public void testCsvFormatWithRegularData() {
         String text = format(CSV, req(), regularData());
         assertEquals("""
-            string,number,location,location2\r
-            Along The River Bank,708,POINT (12.0 56.0),POINT (1234.0 5678.0)\r
-            Mind Train,280,POINT (-97.0 26.0),POINT (-9753.0 2611.0)\r
+            string,number,location,location2,null_field\r
+            Along The River Bank,708,POINT (12.0 56.0),POINT (1234.0 5678.0),\r
+            Mind Train,280,POINT (-97.0 26.0),POINT (-9753.0 2611.0),\r
             """, text);
     }
 
     public void testCsvFormatNoHeaderWithRegularData() {
         String text = format(CSV, reqWithParam("header", "absent"), regularData());
         assertEquals("""
-            Along The River Bank,708,POINT (12.0 56.0),POINT (1234.0 5678.0)\r
-            Mind Train,280,POINT (-97.0 26.0),POINT (-9753.0 2611.0)\r
+            Along The River Bank,708,POINT (12.0 56.0),POINT (1234.0 5678.0),\r
+            Mind Train,280,POINT (-97.0 26.0),POINT (-9753.0 2611.0),\r
             """, text);
     }
 
@@ -146,14 +146,17 @@ public void testCsvFormatWithCustomDelimiterRegularData() {
             "number",
             "location",
             "location2",
+            "null_field",
             "Along The River Bank",
             "708",
             "POINT (12.0 56.0)",
             "POINT (1234.0 5678.0)",
+            "",
             "Mind Train",
             "280",
             "POINT (-97.0 26.0)",
-            "POINT (-9753.0 2611.0)"
+            "POINT (-9753.0 2611.0)",
+            ""
         );
         List<String> expectedTerms = terms.stream()
             .map(x -> x.contains(String.valueOf(delim)) ? '"' + x + '"' : x)
@@ -167,6 +170,8 @@ public void testCsvFormatWithCustomDelimiterRegularData() {
             sb.append(expectedTerms.remove(0));
             sb.append(delim);
             sb.append(expectedTerms.remove(0));
+            sb.append(delim);
+            sb.append(expectedTerms.remove(0));
             sb.append("\r\n");
         } while (expectedTerms.size() > 0);
         assertEquals(sb.toString(), text);
@@ -175,9 +180,9 @@ public void testCsvFormatWithCustomDelimiterRegularData() {
     public void testTsvFormatWithRegularData() {
         String text = format(TSV, req(), regularData());
         assertEquals("""
-            string\tnumber\tlocation\tlocation2
-            Along The River Bank\t708\tPOINT (12.0 56.0)\tPOINT (1234.0 5678.0)
-            Mind Train\t280\tPOINT (-97.0 26.0)\tPOINT (-9753.0 2611.0)
+            string\tnumber\tlocation\tlocation2\tnull_field
+            Along The River Bank\t708\tPOINT (12.0 56.0)\tPOINT (1234.0 5678.0)\t
+            Mind Train\t280\tPOINT (-97.0 26.0)\tPOINT (-9753.0 2611.0)\t
             """, text);
     }
 
@@ -245,6 +250,24 @@ public void testPlainTextEmptyCursorWithoutColumns() {
         );
     }
 
+    public void testCsvFormatWithDropNullColumns() {
+        String text = format(CSV, reqWithParam("drop_null_columns", "true"), regularData());
+        assertEquals("""
+            string,number,location,location2\r
+            Along The River Bank,708,POINT (12.0 56.0),POINT (1234.0 5678.0)\r
+            Mind Train,280,POINT (-97.0 26.0),POINT (-9753.0 2611.0)\r
+            """, text);
+    }
+
+    public void testTsvFormatWithDropNullColumns() {
+        String text = format(TSV, reqWithParam("drop_null_columns", "true"), regularData());
+        assertEquals("""
+            string\tnumber\tlocation\tlocation2
+            Along The River Bank\t708\tPOINT (12.0 56.0)\tPOINT (1234.0 5678.0)
+            Mind Train\t280\tPOINT (-97.0 26.0)\tPOINT (-9753.0 2611.0)
+            """, text);
+    }
+
     private static EsqlQueryResponse emptyData() {
         return new EsqlQueryResponse(singletonList(new ColumnInfoImpl("name", "keyword")), emptyList(), null, false, false, null);
     }
@@ -256,7 +279,8 @@ private static EsqlQueryResponse regularData() {
             new ColumnInfoImpl("string", "keyword"),
             new ColumnInfoImpl("number", "integer"),
             new ColumnInfoImpl("location", "geo_point"),
-            new ColumnInfoImpl("location2", "cartesian_point")
+            new ColumnInfoImpl("location2", "cartesian_point"),
+            new ColumnInfoImpl("null_field", "keyword")
         );
 
         BytesRefArray geoPoints = new BytesRefArray(2, BigArrays.NON_RECYCLING_INSTANCE);
@@ -274,7 +298,8 @@ private static EsqlQueryResponse regularData() {
                 blockFactory.newBytesRefBlockBuilder(2)
                     .appendBytesRef(CARTESIAN.asWkb(new Point(1234, 5678)))
                     .appendBytesRef(CARTESIAN.asWkb(new Point(-9753, 2611)))
-                    .build()
+                    .build(),
+                blockFactory.newConstantNullBlock(2)
             )
         );
 

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/formatter/TextFormatterTests.java

@@ -85,8 +85,6 @@ public class TextFormatterTests extends ESTestCase {
         new EsqlExecutionInfo(randomBoolean())
     );
 
-    TextFormatter formatter = new TextFormatter(esqlResponse);
-
     /**
      * Tests for {@link TextFormatter#format} with header, values
      * of exactly the minimum column size, column names of exactly
@@ -95,7 +93,7 @@ public class TextFormatterTests extends ESTestCase {
      * column size.
      */
     public void testFormatWithHeader() {
-        String[] result = getTextBodyContent(formatter.format(true)).split("\n");
+        String[] result = getTextBodyContent(new TextFormatter(esqlResponse, true, false).format()).split("\n");
         assertThat(result, arrayWithSize(4));
         assertEquals(
             "      foo      |      bar      |15charwidename!|  null_field1  |superduperwidename!!!|      baz      |"
@@ -119,6 +117,35 @@ public void testFormatWithHeader() {
         );
     }
 
+    /**
+     * Tests for {@link TextFormatter#format} with drop_null_columns and
+     * truncation of long columns.
+     */
+    public void testFormatWithDropNullColumns() {
+        String[] result = getTextBodyContent(new TextFormatter(esqlResponse, true, true).format()).split("\n");
+        assertThat(result, arrayWithSize(4));
+        assertEquals(
+            "      foo      |      bar      |15charwidename!|superduperwidename!!!|      baz      |"
+                + "          date          |     location     |      location2       ",
+            result[0]
+        );
+        assertEquals(
+            "---------------+---------------+---------------+---------------------+---------------+-------"
+                + "-----------------+------------------+----------------------",
+            result[1]
+        );
+        assertEquals(
+            "15charwidedata!|1              |6.888          |12.0                 |rabbit         |"
+                + "1953-09-02T00:00:00.000Z|POINT (12.0 56.0) |POINT (1234.0 5678.0) ",
+            result[2]
+        );
+        assertEquals(
+            "dog            |2              |123124.888     |9912.0               |goat           |"
+                + "2000-03-15T21:34:37.443Z|POINT (-97.0 26.0)|POINT (-9753.0 2611.0)",
+            result[3]
+        );
+    }
+
     /**
      * Tests for {@link TextFormatter#format} without header and
      * truncation of long columns.
@@ -160,7 +187,7 @@ public void testFormatWithoutHeader() {
             new EsqlExecutionInfo(randomBoolean())
         );
 
-        String[] result = getTextBodyContent(new TextFormatter(response).format(false)).split("\n");
+        String[] result = getTextBodyContent(new TextFormatter(response, false, false).format()).split("\n");
         assertThat(result, arrayWithSize(2));
         assertEquals(
             "doggie         |4              |1.0            |null           |77.0                 |wombat         |"
@@ -199,8 +226,10 @@ public void testVeryLongPadding() {
                         randomBoolean(),
                         randomBoolean(),
                         new EsqlExecutionInfo(randomBoolean())
-                    )
-                ).format(false)
+                    ),
+                    false,
+                    false
+                ).format()
             )
         );
     }