QL: Add leniency option to SQL CLI

docs/changelog/83795.yaml

@@ -0,0 +1,6 @@
+pr: 83795
+summary: Add leniency option to SQL CLI
+area: SQL
+type: enhancement
+issues:
+ - 67436

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/cli/EmbeddedCli.java

@@ -28,6 +28,7 @@
 import java.io.PipedOutputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -65,6 +66,15 @@ public class EmbeddedCli implements Closeable {
 
     public EmbeddedCli(String elasticsearchAddress, boolean checkConnectionOnStartup, @Nullable SecurityConfig security)
         throws IOException {
+        this(elasticsearchAddress, checkConnectionOnStartup, security, new String[0]);
+    }
+
+    public EmbeddedCli(
+        String elasticsearchAddress,
+        boolean checkConnectionOnStartup,
+        @Nullable SecurityConfig security,
+        @Nullable String[] additionalArgs
+    ) throws IOException {
         PipedOutputStream outgoing = new PipedOutputStream();
         PipedInputStream cliIn = new PipedInputStream(outgoing);
         PipedInputStream incoming = new PipedInputStream();
@@ -109,6 +119,10 @@ protected boolean addShutdownHook() {
             args.add(Boolean.toString(randomBoolean()));
         }
 
+        if (additionalArgs != null) {
+            Arrays.stream(additionalArgs).forEach(x -> args.add(x));
+        }
+
         exec = new Thread(() -> {
             try {
                 /*

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/cli/SelectTestCase.java

@@ -37,4 +37,45 @@ public void testSelectWithWhere() throws IOException {
         assertThat(readLine(), RegexMatcher.matches("\\s*2\\s*\\|\\s*test_value2\\s*"));
         assertEquals("", readLine());
     }
+
+    public void testLeniency() throws IOException {
+        EmbeddedCli cliWithLeniency = new EmbeddedCli(
+            CliIntegrationTestCase.elasticsearchAddress(),
+            true,
+            securityConfig(),
+            new String[] { "-lenient", "true" }
+        );
+
+        try {
+            index("test", body -> body.field("name", "foo").field("tags", new String[] { "bar", "bar" }));
+
+            assertThat(cliWithLeniency.command("SELECT * FROM test"), RegexMatcher.matches("\\s*name\\s*\\|\\s*tags\\s*"));
+            assertThat(cliWithLeniency.readLine(), containsString("----------"));
+            assertThat(cliWithLeniency.readLine(), RegexMatcher.matches("\\s*foo\\s*\\|\\s*bar\\s*"));
+            assertEquals("", cliWithLeniency.readLine());
+        } finally {
+            cliWithLeniency.close();
+        }
+    }
+
+    public void testLeniency2() throws IOException {
+        EmbeddedCli cliWithLeniency = new EmbeddedCli(
+            CliIntegrationTestCase.elasticsearchAddress(),
+            true,
+            securityConfig(),
+            new String[] { "-l", "true" }
+        );
+
+        try {
+            index("test", body -> body.field("name", "foo").field("tags", new String[] { "bar", "bar" }));
+
+            assertThat(cliWithLeniency.command("SELECT * FROM test"), RegexMatcher.matches("\\s*name\\s*\\|\\s*tags\\s*"));
+            assertThat(cliWithLeniency.readLine(), containsString("----------"));
+            assertThat(cliWithLeniency.readLine(), RegexMatcher.matches("\\s*foo\\s*\\|\\s*bar\\s*"));
+            assertEquals("", cliWithLeniency.readLine());
+        } finally {
+            cliWithLeniency.close();
+        }
+    }
+
 }

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

@@ -40,6 +40,7 @@ public class Cli extends Command {
     private final OptionSpec<Boolean> checkOption;
     private final OptionSpec<String> connectionString;
     private final OptionSpec<Boolean> binaryCommunication;
+    private final OptionSpec<Boolean> lenientOption;
 
     /**
      * Use this VM Options to run in IntelliJ or Eclipse:
@@ -102,6 +103,11 @@ public Cli(CliTerminal cliTerminal) {
             .withRequiredArg()
             .ofType(Boolean.class)
             .defaultsTo(Boolean.parseBoolean(System.getProperty("cli.check", "true")));
+        this.lenientOption = parser.acceptsAll(
+            Arrays.asList("l", "lenient"),
+            "Lenient and return first value for fields with multiple values"
+        ).withRequiredArg().ofType(Boolean.class).defaultsTo(Boolean.parseBoolean(System.getProperty("cli.lenient", "false")));
+
         this.connectionString = parser.nonOptions("uri");
     }
 
@@ -110,6 +116,7 @@ protected void execute(org.elasticsearch.cli.Terminal terminal, OptionSet option
         boolean debug = options.has("d") || options.has("debug");
         boolean binary = binaryCommunication.value(options);
         boolean checkConnection = checkOption.value(options);
+        boolean lenient = this.lenientOption.value(options);
         List<String> args = connectionString.values(options);
         if (args.size() > 1) {
             throw new UserException(ExitCodes.USAGE, "expecting a single uri");
@@ -120,17 +127,18 @@ protected void execute(org.elasticsearch.cli.Terminal terminal, OptionSet option
             throw new UserException(ExitCodes.USAGE, "expecting a single keystore file");
         }
         String keystoreLocationValue = args.size() == 1 ? args.get(0) : null;
-        execute(uri, debug, binary, keystoreLocationValue, checkConnection);
+        execute(uri, debug, binary, keystoreLocationValue, checkConnection, lenient);
     }
 
-    private void execute(String uri, boolean debug, boolean binary, String keystoreLocation, boolean checkConnection) throws Exception {
+    private void execute(String uri, boolean debug, boolean binary, String keystoreLocation, boolean checkConnection, boolean lenient)
+        throws Exception {
         CliCommand cliCommand = new CliCommands(
             new PrintLogoCommand(),
             new ClearScreenCliCommand(),
             new FetchSizeCliCommand(),
             new FetchSeparatorCliCommand(),
             new ServerInfoCliCommand(),
-            new ServerQueryCliCommand()
+            new ServerQueryCliCommand(lenient)
         );
         try {
             ConnectionBuilder connectionBuilder = new ConnectionBuilder(cliTerminal);

x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java

@@ -9,6 +9,7 @@
 import org.elasticsearch.xpack.sql.cli.CliTerminal;
 import org.elasticsearch.xpack.sql.client.HttpClient;
 import org.elasticsearch.xpack.sql.client.JreHttpUrlConnection;
+import org.elasticsearch.xpack.sql.proto.CoreProtocol;
 import org.elasticsearch.xpack.sql.proto.Mode;
 import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;
 import org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter;
@@ -19,14 +20,24 @@
 
 public class ServerQueryCliCommand extends AbstractServerCliCommand {
 
+    protected boolean lenient;
+
+    public ServerQueryCliCommand() {
+        this(CoreProtocol.FIELD_MULTI_VALUE_LENIENCY);
+    }
+
+    public ServerQueryCliCommand(boolean lenient) {
+        this.lenient = lenient;
+    }
+
     @Override
     protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String line) {
         SqlQueryResponse response = null;
         HttpClient cliClient = cliSession.getClient();
         SimpleFormatter formatter;
         String data;
         try {
-            response = cliClient.basicQuery(line, cliSession.getFetchSize());
+            response = cliClient.basicQuery(line, cliSession.getFetchSize(), lenient);
             formatter = new SimpleFormatter(response.columns(), response.rows(), CLI);
             data = formatter.formatWithHeader(response.columns(), response.rows());
             while (true) {

x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java

@@ -32,11 +32,11 @@ public void testExceptionHandling() throws Exception {
         TestTerminal testTerminal = new TestTerminal();
         HttpClient client = mock(HttpClient.class);
         CliSession cliSession = new CliSession(client);
-        when(client.basicQuery("blah", 1000)).thenThrow(new SQLException("test exception"));
+        when(client.basicQuery("blah", 1000, false)).thenThrow(new SQLException("test exception"));
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
         assertTrue(cliCommand.handle(testTerminal, cliSession, "blah"));
         assertEquals("<b>Bad request [</b><i>test exception</i><b>]</b>\n", testTerminal.toString());
-        verify(client, times(1)).basicQuery(eq("blah"), eq(1000));
+        verify(client, times(1)).basicQuery(eq("blah"), eq(1000), eq(false));
         verifyNoMoreInteractions(client);
     }
 
@@ -45,15 +45,15 @@ public void testOnePageQuery() throws Exception {
         HttpClient client = mock(HttpClient.class);
         CliSession cliSession = new CliSession(client);
         cliSession.setFetchSize(10);
-        when(client.basicQuery("test query", 10)).thenReturn(fakeResponse("", true, "foo"));
+        when(client.basicQuery("test query", 10, false)).thenReturn(fakeResponse("", true, "foo"));
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
         assertTrue(cliCommand.handle(testTerminal, cliSession, "test query"));
         assertEquals("""
                  field    \s
             ---------------
             foo           \s
             <flush/>""", testTerminal.toString());
-        verify(client, times(1)).basicQuery(eq("test query"), eq(10));
+        verify(client, times(1)).basicQuery(eq("test query"), eq(10), eq(false));
         verifyNoMoreInteractions(client);
     }
 
@@ -62,7 +62,7 @@ public void testThreePageQuery() throws Exception {
         HttpClient client = mock(HttpClient.class);
         CliSession cliSession = new CliSession(client);
         cliSession.setFetchSize(10);
-        when(client.basicQuery("test query", 10)).thenReturn(fakeResponse("my_cursor1", true, "first"));
+        when(client.basicQuery("test query", 10, false)).thenReturn(fakeResponse("my_cursor1", true, "first"));
         when(client.nextPage("my_cursor1")).thenReturn(fakeResponse("my_cursor2", false, "second"));
         when(client.nextPage("my_cursor2")).thenReturn(fakeResponse("", false, "third"));
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
@@ -74,7 +74,7 @@ public void testThreePageQuery() throws Exception {
             second        \s
             third         \s
             <flush/>""", testTerminal.toString());
-        verify(client, times(1)).basicQuery(eq("test query"), eq(10));
+        verify(client, times(1)).basicQuery(eq("test query"), eq(10), eq(false));
         verify(client, times(2)).nextPage(any());
         verifyNoMoreInteractions(client);
     }
@@ -86,7 +86,7 @@ public void testTwoPageQueryWithSeparator() throws Exception {
         cliSession.setFetchSize(15);
         // Set a separator
         cliSession.setFetchSeparator("-----");
-        when(client.basicQuery("test query", 15)).thenReturn(fakeResponse("my_cursor1", true, "first"));
+        when(client.basicQuery("test query", 15, false)).thenReturn(fakeResponse("my_cursor1", true, "first"));
         when(client.nextPage("my_cursor1")).thenReturn(fakeResponse("", false, "second"));
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
         assertTrue(cliCommand.handle(testTerminal, cliSession, "test query"));
@@ -97,7 +97,7 @@ public void testTwoPageQueryWithSeparator() throws Exception {
             -----
             second        \s
             <flush/>""", testTerminal.toString());
-        verify(client, times(1)).basicQuery(eq("test query"), eq(15));
+        verify(client, times(1)).basicQuery(eq("test query"), eq(15), eq(false));
         verify(client, times(1)).nextPage(any());
         verifyNoMoreInteractions(client);
     }
@@ -107,7 +107,7 @@ public void testCursorCleanupOnError() throws Exception {
         HttpClient client = mock(HttpClient.class);
         CliSession cliSession = new CliSession(client);
         cliSession.setFetchSize(15);
-        when(client.basicQuery("test query", 15)).thenReturn(fakeResponse("my_cursor1", true, "first"));
+        when(client.basicQuery("test query", 15, false)).thenReturn(fakeResponse("my_cursor1", true, "first"));
         when(client.nextPage("my_cursor1")).thenThrow(new SQLException("test exception"));
         when(client.queryClose("my_cursor1", Mode.CLI)).thenReturn(true);
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
@@ -118,7 +118,7 @@ public void testCursorCleanupOnError() throws Exception {
             first         \s
             <b>Bad request [</b><i>test exception</i><b>]</b>
             """, testTerminal.toString());
-        verify(client, times(1)).basicQuery(eq("test query"), eq(15));
+        verify(client, times(1)).basicQuery(eq("test query"), eq(15), eq(false));
         verify(client, times(1)).nextPage(any());
         verify(client, times(1)).queryClose(eq("my_cursor1"), eq(Mode.CLI));
         verifyNoMoreInteractions(client);

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

@@ -61,6 +61,10 @@ public MainResponse serverInfo() throws SQLException {
     }
 
     public SqlQueryResponse basicQuery(String query, int fetchSize) throws SQLException {
+        return basicQuery(query, fetchSize, CoreProtocol.FIELD_MULTI_VALUE_LENIENCY);
+    }
+
+    public SqlQueryResponse basicQuery(String query, int fetchSize, boolean fieldMultiValueLeniency) throws SQLException {
         // TODO allow customizing the time zone - this is what session set/reset/get should be about
         // method called only from CLI
         SqlQueryRequest sqlRequest = new SqlQueryRequest(
@@ -74,7 +78,7 @@ public SqlQueryResponse basicQuery(String query, int fetchSize) throws SQLExcept
             Boolean.FALSE,
             null,
             new RequestInfo(Mode.CLI, ClientVersion.CURRENT),
-            false,
+            fieldMultiValueLeniency,
             false,
             cfg.binaryCommunication()
         );

x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/HttpClientRequestTests.java

@@ -106,7 +106,7 @@ private void assertBinaryRequestForCLI(XContentType xContentType) throws URISynt
 
         prepareMockResponse();
         try {
-            httpClient.basicQuery(query, fetchSize);
+            httpClient.basicQuery(query, fetchSize, false);
         } catch (SQLException e) {
             logger.info("Ignored SQLException", e);
         }