SQL: Move Exception classes to follow ES format (elastic/x-pack-elasticsearch#3721)

This commit moves the exception classes that SQL uses to follow the
Elasticsearch convention. In the places where varargs were used, the
`LoggerMessageFormat` (`{}`) standard is used instead.

In also removes on Exception - `ExecutionException` since it seemed to not ever
be beefed up, it can be re-added later if needed.

This removes the varargs version of `ClientException` to push the formatting
back on the caller, since `ClientException` cannot depend on Elasticsearch for
formatting the arguments.

There were also a couple of places where we incorrectly passed the Throwable
cause as a var-arg and were unintentionally swallowing it since `String.format`
discards unused arguments.

Relates to elastic/x-pack-elasticsearch#2880

Original commit: elastic/x-pack-elasticsearch@5f5d580e57

plugin/sql/sql-shared-client/src/main/java/org/elasticsearch/xpack/sql/client/shared/ClientException.java

@@ -5,27 +5,19 @@
  */
 package org.elasticsearch.xpack.sql.client.shared;
 
-import java.util.Locale;
+/**
-
+ * A general-purpose exception to be used on the client-side code. Does not support var-args formatting.
-import static java.lang.String.format;
+ */
-
 public class ClientException extends RuntimeException {
-    public ClientException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
+
-        super(message, cause, enableSuppression, writableStackTrace);
+    public ClientException(String message, Throwable cause) {
+        super(message, cause);
     }
 
     public ClientException(String message) {
         super(message);
     }
 
-    public ClientException(String message, Object... args) { // TODO remove these ctors
-        super(format(Locale.ROOT, message, args));
-    }
-
-    public ClientException(Throwable cause, String message, Object... args) {
-        super(format(Locale.ROOT, message, args), cause);
-    }
-
     public ClientException(Throwable cause) {
         super(cause);
     }

plugin/sql/sql-shared-client/src/main/java/org/elasticsearch/xpack/sql/client/shared/JreHttpUrlConnection.java

@@ -50,7 +50,7 @@ public class JreHttpUrlConnection implements Closeable {
             url = new URI(uriPath.getScheme(), null, uriPath.getHost(), uriPath.getPort(), uriPath.getPath(), uriQuery,
                     uriPath.getFragment()).toURL();
         } catch (URISyntaxException | MalformedURLException ex) {
-            throw new ClientException(ex, "Cannot build url using base: [" + uriPath + "] query: [" + query + "] path: [" + path + "]");
+            throw new ClientException("Cannot build url using base: [" + uriPath + "] query: [" + query + "] path: [" + path + "]", ex);
         }
         try (JreHttpUrlConnection con = new JreHttpUrlConnection(url, cfg)) {
             return handler.apply(con);
@@ -69,7 +69,7 @@ public class JreHttpUrlConnection implements Closeable {
             Proxy p = cfg.proxyConfig().proxy();
             con = (HttpURLConnection) (p != null ? url.openConnection(p) : url.openConnection());
         } catch (IOException ex) {
-            throw new ClientException(ex, "Cannot setup connection to %s (%s)", url, ex.getMessage());
+            throw new ClientException("Cannot setup connection to " + url + " (" + ex.getMessage() + ")", ex);
         }
 
         // the rest of the connection setup
@@ -122,7 +122,7 @@ public class JreHttpUrlConnection implements Closeable {
             int responseCode = con.getResponseCode();
             return responseCode == HttpURLConnection.HTTP_OK;
         } catch (IOException ex) {
-            throw new ClientException(ex, "Cannot HEAD address %s (%s)", url, ex.getMessage());
+            throw new ClientException("Cannot HEAD address " + url + " (" + ex.getMessage() + ")", ex);
         }
     }
 
@@ -151,7 +151,7 @@ public class JreHttpUrlConnection implements Closeable {
             }
             return parserError();
         } catch (IOException ex) {
-            throw new ClientException(ex, "Cannot POST address %s (%s)", url, ex.getMessage());
+            throw new ClientException("Cannot POST address " + url + " (" + ex.getMessage() + ")", ex);
         }
     }
 
@@ -214,7 +214,7 @@ public class JreHttpUrlConnection implements Closeable {
         try {
             con.connect();
         } catch (IOException ex) {
-            throw new ClientException(ex, "Cannot open connection to %s (%s)", url, ex.getMessage());
+            throw new ClientException("Cannot open connection to " + url + " (" + ex.getMessage() + ")", ex);
         }
     }
 

plugin/sql/sql-shared-client/src/main/java/org/elasticsearch/xpack/sql/client/shared/ProxyConfig.java

@@ -61,7 +61,7 @@ class ProxyConfig {
             Object[] results = { uri.getHost(), uri.getPort() > 0 ? uri.getPort() : defaultPort };
             return results;
         } catch (URISyntaxException ex) {
-            throw new ClientException("Unrecognized address format %s", address);
+            throw new ClientException("Unrecognized address format " + address, ex);
         }
     }
 }

plugin/sql/sql-shared-client/src/main/java/org/elasticsearch/xpack/sql/client/shared/SslConfig.java

@@ -90,7 +90,7 @@ public class SslConfig {
             ctx = SSLContext.getInstance(protocol);
             ctx.init(loadKeyManagers(), loadTrustManagers(), null);
         } catch (Exception ex) {
-            throw new ClientException(ex, "Failed to initialize SSL - %s", ex.getMessage());
+            throw new ClientException("Failed to initialize SSL - " + ex.getMessage(), ex);
         }
 
         return ctx;
@@ -115,13 +115,13 @@ public class SslConfig {
 
         if (!Files.exists(path)) {
            throw new ClientException(
-                   "Expected to find keystore file at [%s] but was unable to. Make sure you have specified a valid URI.", location);
+                   "Expected to find keystore file at [" + location + "] but was unable to. Make sure you have specified a valid URI.");
         }
 
         try (InputStream in = Files.newInputStream(Paths.get(location), StandardOpenOption.READ)) {
             keyStore.load(in, pass);
         } catch (Exception ex) {
-            throw new ClientException(ex, "Cannot open keystore [%s] - %s", location, ex.getMessage());
+            throw new ClientException("Cannot open keystore [" + location + "] - " + ex.getMessage(), ex);
         } finally {
 
         }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/SqlException.java

@@ -7,11 +7,7 @@ package org.elasticsearch.xpack.sql;
 
 import org.elasticsearch.ElasticsearchException;
 
-import java.util.Locale;
+public class SqlException extends ElasticsearchException {
-
-import static java.lang.String.format;
-
-public abstract class SqlException extends ElasticsearchException {
     public SqlException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
         super(message, cause, enableSuppression, writableStackTrace);
     }
@@ -21,11 +17,11 @@ public abstract class SqlException extends ElasticsearchException {
     }
 
     public SqlException(String message, Object... args) {
-        this(null, message, args);
+        super(message, args);
     }
 
     public SqlException(Throwable cause, String message, Object... args) {
-        super(format(Locale.ROOT, message, args), cause);
+        super(message, cause, args);
     }
 
     public SqlException(Throwable cause) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/SqlIllegalArgumentException.java

@@ -15,7 +15,7 @@ public class SqlIllegalArgumentException extends ServerSqlException {
     }
 
     public SqlIllegalArgumentException(String message, Object... args) {
-        this(null, message, args);
+        super(message, args);
     }
 
     public SqlIllegalArgumentException(Throwable cause, String message, Object... args) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/AnalysisException.java

@@ -12,8 +12,6 @@ import org.elasticsearch.xpack.sql.tree.Node;
 
 import java.util.Locale;
 
-import static java.lang.String.format;
-
 public class AnalysisException extends ClientSqlException {
 
     private final int line;
@@ -56,6 +54,6 @@ public class AnalysisException extends ClientSqlException {
 
     @Override
     public String getMessage() {
-        return format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
+        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/ExecutionException.java

@@ -1,20 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.sql.execution;
-
-import org.elasticsearch.xpack.sql.SqlException;
-
-//TODO: beef up the exception or remove it
-public class ExecutionException extends SqlException {
-
-    public ExecutionException(String message, Object ...args) {
-        super(message, args);
-    }
-
-    public ExecutionException(String message, Throwable cause) {
-        super(message, cause);
-    }
-}

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java

@@ -52,7 +52,7 @@ public class PlanExecutor {
                 EsQueryExec e = (EsQueryExec) exec;
                 listener.onResponse(SourceGenerator.sourceBuilder(e.queryContainer(), settings.filter(), settings.pageSize()));
             } else {
-                listener.onFailure(new PlanningException("Cannot generate a query DSL for %s", sql));
+                listener.onFailure(new PlanningException("Cannot generate a query DSL for {}", sql));
             }
         }, listener::onFailure));
     }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/AbstractSearchHitRowSet.java

@@ -55,7 +55,7 @@ abstract class AbstractSearchHitRowSet extends AbstractRowSet {
         int maxDepth = 0;
         if (!innerHits.isEmpty()) {
             if (innerHits.size() > 1) {
-                throw new SqlIllegalArgumentException("Multi-nested docs not yet supported %s", innerHits);
+                throw new SqlIllegalArgumentException("Multi-nested docs not yet supported {}", innerHits);
             }
             maxDepth = 1;
 

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Scroller.java

@@ -23,8 +23,8 @@ import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation;
 import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
 import org.elasticsearch.search.aggregations.support.AggregationPath;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
+import org.elasticsearch.xpack.sql.SqlException;
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
-import org.elasticsearch.xpack.sql.execution.ExecutionException;
 import org.elasticsearch.xpack.sql.execution.search.extractor.ComputingHitExtractor;
 import org.elasticsearch.xpack.sql.execution.search.extractor.FieldHitExtractor;
 import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractor;
@@ -198,7 +198,7 @@ public class Scroller {
 
                 return arr;
             }
-            throw new SqlIllegalArgumentException("Unexpected non-agg/grouped column specified; %s", col.getClass());
+            throw new SqlIllegalArgumentException("Unexpected non-agg/grouped column specified; {}", col.getClass());
         }
 
         private static Object getAggProperty(Aggregations aggs, String path) {
@@ -206,7 +206,7 @@ public class Scroller {
             String aggName = list.get(0);
             InternalAggregation agg = aggs.get(aggName);
             if (agg == null) {
-                throw new ExecutionException("Cannot find an aggregation named %s", aggName);
+                throw new SqlException("Cannot find an aggregation named {}", aggName);
             }
             return agg.getProperty(list.subList(1, list.size()));
         }
@@ -285,7 +285,7 @@ public class Scroller {
                     hitNames.add(he.hitName());
 
                     if (hitNames.size() > 1) {
-                        throw new SqlIllegalArgumentException("Multi-level nested fields [%s] not supported yet", hitNames);
+                        throw new SqlIllegalArgumentException("Multi-level nested fields [{}] not supported yet", hitNames);
                     }
 
                     return new HitExtractorInput(l.location(), l.expression(), he);
@@ -297,7 +297,7 @@ public class Scroller {
                 return new ComputingHitExtractor(proc.asProcessor(), hitName);
             }
 
-            throw new SqlIllegalArgumentException("Unexpected ValueReference %s", ref.getClass());
+            throw new SqlIllegalArgumentException("Unexpected ValueReference {}", ref.getClass());
         }
     }
 
@@ -323,7 +323,7 @@ public class Scroller {
             try {
                 ShardSearchFailure[] failure = response.getShardFailures();
                 if (!CollectionUtils.isEmpty(failure)) {
-                    cleanupScroll(response, new ExecutionException(failure[0].reason(), failure[0].getCause()));
+                    cleanupScroll(response, new SqlException(failure[0].reason(), failure[0].getCause()));
                 } else {
                     handleResponse(response, ActionListener.wrap(listener::onResponse, e -> cleanupScroll(response, e)));
                 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java

@@ -10,8 +10,8 @@ import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.search.SearchHit;
+import org.elasticsearch.xpack.sql.SqlException;
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
-import org.elasticsearch.xpack.sql.execution.ExecutionException;
 import org.joda.time.ReadableDateTime;
 
 import java.io.IOException;
@@ -56,7 +56,7 @@ public class FieldHitExtractor implements HitExtractor {
 
         if (hitName != null) {
             if (!name.contains(hitName)) {
-                throw new SqlIllegalArgumentException("Hitname [%s] specified but not part of the name [%s]", hitName, name);
+                throw new SqlIllegalArgumentException("Hitname [{}] specified but not part of the name [{}]", hitName, name);
             }
         }
 
@@ -111,18 +111,18 @@ public class FieldHitExtractor implements HitExtractor {
                 if (ARRAYS_LENIENCY || list.size() == 1) {
                     return unwrapMultiValue(list.get(0));
                 } else {
-                    throw new ExecutionException("Arrays (returned by [%s]) are not supported", fieldName);
+                    throw new SqlException("Arrays (returned by [{}]) are not supported", fieldName);
                 }
             }
         }
         if (values instanceof Map) {
-            throw new ExecutionException("Objects (returned by [%s]) are not supported", fieldName);
+            throw new SqlException("Objects (returned by [{}]) are not supported", fieldName);
         }
         if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean
                 || values instanceof ReadableDateTime) {
             return values;
         }
-        throw new ExecutionException("Type %s (returned by [%s]) is not supported", values.getClass().getSimpleName(), fieldName);
+        throw new SqlException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName);
     }
 
     @SuppressWarnings("unchecked")
@@ -137,7 +137,7 @@ public class FieldHitExtractor implements HitExtractor {
                 first = false;
                 value = ((Map<String, Object>) value).get(node);
             } else {
-                throw new ExecutionException("Cannot extract value [%s] from source", fieldName);
+                throw new SqlException("Cannot extract value [{}] from source", fieldName);
             }
         }
         return unwrapMultiValue(value);

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Foldables.java

@@ -20,14 +20,14 @@ public abstract class Foldables {
         if (e.foldable()) {
             return (T) DataTypeConversion.conversionFor(e.dataType(), to).convert(e.fold());
         }
-        throw new SqlIllegalArgumentException("Cannot determine value for %s", e);
+        throw new SqlIllegalArgumentException("Cannot determine value for {}", e);
     }
 
     public static Object valueOf(Expression e) {
         if (e.foldable()) {
             return e.fold();
         }
-        throw new SqlIllegalArgumentException("Cannot determine value for %s", e);
+        throw new SqlIllegalArgumentException("Cannot determine value for {}", e);
     }
 
     public static String stringValueOf(Expression e) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java

@@ -167,7 +167,7 @@ public class FunctionRegistry {
     public Function resolveFunction(UnresolvedFunction ur, DateTimeZone timeZone) {
         FunctionDefinition def = defs.get(normalize(ur.name()));
         if (def == null) {
-            throw new SqlIllegalArgumentException("Cannot find function %s; this should have been caught during analysis", ur.name());
+            throw new SqlIllegalArgumentException("Cannot find function {}; this should have been caught during analysis", ur.name());
         }
         return def.builder().apply(ur, timeZone);
     }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionType.java

@@ -28,6 +28,6 @@ public enum FunctionType {
                 return type;
             }
         }
-        throw new SqlIllegalArgumentException("Cannot identify the function type for %s", clazz);
+        throw new SqlIllegalArgumentException("Cannot identify the function type for {}", clazz);
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ScalarFunction.java

@@ -65,7 +65,7 @@ public abstract class ScalarFunction extends Function {
             // fall-back to
             return asScriptFrom((FieldAttribute) attr);
         }
-        throw new SqlIllegalArgumentException("Cannot evaluate script for expression %s", exp);
+        throw new SqlIllegalArgumentException("Cannot evaluate script for expression {}", exp);
     }
 
     protected ScriptTemplate asScriptFrom(ScalarFunctionAttribute scalar) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/arithmetic/BinaryArithmeticProcessor.java

@@ -16,8 +16,6 @@ import java.util.Locale;
 import java.util.Objects;
 import java.util.function.BiFunction;
 
-import static java.lang.String.format;
-
 public class BinaryArithmeticProcessor extends BinaryProcessor {
     
     public enum BinaryArithmeticOperation {
@@ -80,10 +78,10 @@ public class BinaryArithmeticProcessor extends BinaryProcessor {
             return null;
         }
         if (!(left instanceof Number)) {
-            throw new SqlIllegalArgumentException("A number is required; received %s", left);
+            throw new SqlIllegalArgumentException("A number is required; received {}", left);
         }
         if (!(right instanceof Number)) {
-            throw new SqlIllegalArgumentException("A number is required; received %s", right);
+            throw new SqlIllegalArgumentException("A number is required; received {}", right);
         }
 
         return operation.apply((Number) left, (Number) right);
@@ -112,6 +110,6 @@ public class BinaryArithmeticProcessor extends BinaryProcessor {
 
     @Override
     public String toString() {
-        return format(Locale.ROOT, "(%s %s %s)", left(), operation, right());
+        return String.format(Locale.ROOT, "(%s %s %s)", left(), operation, right());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/arithmetic/UnaryArithmeticProcessor.java

@@ -65,7 +65,7 @@ public class UnaryArithmeticProcessor implements Processor {
         if (input instanceof Number) {
             return operation.apply((Number) input);
         }
-        throw new SqlIllegalArgumentException("A number is required; received %s", input);
+        throw new SqlIllegalArgumentException("A number is required; received {}", input);
     }
 
 

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java

@@ -78,7 +78,7 @@ public class DateTimeProcessor implements Processor {
         }
 
         if (!(l instanceof ReadableDateTime)) {
-            throw new SqlIllegalArgumentException("A date/time is required; received %s", l);
+            throw new SqlIllegalArgumentException("A date/time is required; received {}", l);
         }
 
         ReadableDateTime dt = (ReadableDateTime) l;

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/processor/definition/ProcessorDefinitions.java

@@ -27,6 +27,6 @@ public abstract class ProcessorDefinitions {
         if (ex instanceof NamedExpression) {
             return new AttributeInput(ex.location(), ex, ((NamedExpression) ex).toAttribute());
         }
-        throw new SqlIllegalArgumentException("Cannot extract processor from %s", ex);
+        throw new SqlIllegalArgumentException("Cannot extract processor from {}", ex);
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/processor/runtime/HitExtractorProcessor.java

@@ -45,7 +45,7 @@ public class HitExtractorProcessor implements Processor {
     @Override
     public Object process(Object input) {
         if (!(input instanceof SearchHit)) {
-            throw new SqlIllegalArgumentException("Expected a SearchHit but received %s", input);
+            throw new SqlIllegalArgumentException("Expected a SearchHit but received {}", input);
         }
         return extractor.get((SearchHit) input);
     }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/script/Params.java

@@ -120,7 +120,7 @@ public class Params {
                     flatten.add(p);
                 }
                 else {
-                    throw new SqlIllegalArgumentException("Unsupported field %s", p);
+                    throw new SqlIllegalArgumentException("Unsupported field {}", p);
                 }
             }
         }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextUtils.java

@@ -31,12 +31,12 @@ abstract class FullTextUtils {
         for (String entry : list) {
             String[] split = splitInTwo(entry, "=");
             if (split == null) {
-                throw new ParsingException(location, "Cannot parse entry %s in options %s", entry, options);
+                throw new ParsingException(location, "Cannot parse entry {} in options {}", entry, options);
             }
 
             String previous = op.put(split[0], split[1]);
             if (previous != null) {
-                throw new ParsingException(location, "Duplicate option %s detected in options %s", entry, options);
+                throw new ParsingException(location, "Duplicate option {} detected in options {}", entry, options);
             }
 
         }
@@ -66,7 +66,7 @@ abstract class FullTextUtils {
                     try {
                         fields.put(split[0], Float.parseFloat(split[1]));
                     } catch (NumberFormatException nfe) {
-                        throw new ParsingException(location, "Cannot parse boosting for %s", fieldName);
+                        throw new ParsingException(location, "Cannot parse boosting for {}", fieldName);
                     }
                 }
             }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/AbstractBuilder.java

@@ -31,7 +31,7 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
     @Override
     public Object visit(ParseTree tree) {
         Object result = super.visit(tree);
-        Check.notNull(result, "Don't know how to handle context [%s] with value [%s]", tree.getClass(), tree.getText());
+        Check.notNull(result, "Don't know how to handle context [{}] with value [{}]", tree.getClass(), tree.getText());
         return result;
     }
 
@@ -42,7 +42,7 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
             return (T) result;
         }
 
-        throw new ParsingException(source(ctx), "Invalid query '%s'[%s] given; expected %s but found %s",
+        throw new ParsingException(source(ctx), "Invalid query '{}'[{}] given; expected {} but found {}",
                         ctx.getText(), ctx.getClass().getSimpleName(),
                         type.getSimpleName(), (result != null ? result.getClass().getSimpleName() : "null"));
     }
@@ -106,6 +106,6 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
 
     @Override
     public Object visitTerminal(TerminalNode node) {
-        throw new ParsingException(source(node), "Does not know how to handle %s", node.getText());
+        throw new ParsingException(source(node), "Does not know how to handle {}", node.getText());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java

@@ -82,8 +82,6 @@ import java.math.BigDecimal;
 import java.util.List;
 import java.util.Locale;
 
-import static java.lang.String.format;
-
 abstract class ExpressionBuilder extends IdentifierBuilder {
     /**
      * Time zone that we're executing in. Set on functions that deal
@@ -161,7 +159,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
             case SqlBaseParser.GTE:
                 return new GreaterThanOrEqual(loc, left, right);
             default:
-                throw new ParsingException(loc, "Unknown operator %s", op.getSymbol().getText());
+                throw new ParsingException(loc, "Unknown operator {}", op.getSymbol().getText());
         }
     }
 
@@ -199,7 +197,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
                 e = new IsNotNull(loc, exp);
                 return pCtx.NOT() != null ? e : new Not(loc, e);
             default:
-                throw new ParsingException(loc, "Unknown predicate %s", pCtx.kind.getText());
+                throw new ParsingException(loc, "Unknown predicate {}", pCtx.kind.getText());
         }
 
         return pCtx.NOT() != null ? new Not(loc, e) : e;
@@ -215,7 +213,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
         int pos = pattern.indexOf('*');
         if (pos >= 0) {
             throw new ParsingException(source(ctx.value),
-                    "Invalid char [*] found in pattern [%s] at position %d; use [%%] or [_] instead",
+                    "Invalid char [*] found in pattern [{}] at position {}; use [%] or [_] instead",
                     pattern, pos);
         }
 
@@ -225,12 +223,12 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
         if (Strings.hasText(escapeString)) {
             // shouldn't happen but adding validation in case the string parsing gets wonky
             if (escapeString.length() > 1) {
-                throw new ParsingException(source(ctx.escape), "A character not a string required for escaping; found [%s]", escapeString);
+                throw new ParsingException(source(ctx.escape), "A character not a string required for escaping; found [{}]", escapeString);
             } else if (escapeString.length() == 1) {
                 escape = escapeString.charAt(0);
                 // these chars already have a meaning
                 if (escape == '*' || escape == '%' || escape == '_') {
-                    throw new ParsingException(source(ctx.escape), "Char [%c] cannot be used for escaping", escape);
+                    throw new ParsingException(source(ctx.escape), "Char [{}] cannot be used for escaping", escape);
                 }
                 // lastly validate that escape chars (if present) are followed by special chars
                 for (int i = 0; i < pattern.length(); i++) {
@@ -238,13 +236,13 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
                     if (current == escape) {
                         if (i + 1 == pattern.length()) {
                             throw new ParsingException(source(ctx.value),
-                                    "Pattern [%s] is invalid as escape char [%c] at position %d does not escape anything", pattern, escape,
+                                    "Pattern [{}] is invalid as escape char [{}] at position {} does not escape anything", pattern, escape,
                                     i);
                         }
                         char next = pattern.charAt(i + 1);
                         if (next != '%' && next != '_') {
                             throw new ParsingException(source(ctx.value),
-                                    "Pattern [%s] is invalid as escape char [%c] at position %d can only escape wildcard chars; found [%c]",
+                                    "Pattern [{}] is invalid as escape char [{}] at position {} can only escape wildcard chars; found [{}]",
                                     pattern, escape, i, next);
                         }
                     }
@@ -270,7 +268,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
             case SqlBaseParser.MINUS:
                 return new Neg(source(ctx.operator), value);
             default:
-                throw new ParsingException(loc, "Unknown arithemtic %s", ctx.operator.getText());
+                throw new ParsingException(loc, "Unknown arithemtic {}", ctx.operator.getText());
         }
     }
 
@@ -293,7 +291,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
             case SqlBaseParser.MINUS:
                 return new Sub(loc, left, right);
             default:
-                throw new ParsingException(loc, "Unknown arithemtic %s", ctx.operator.getText());
+                throw new ParsingException(loc, "Unknown arithemtic {}", ctx.operator.getText());
         }
     }
 
@@ -361,7 +359,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
             case "string":
                 return DataType.KEYWORD;
             default:
-                throw new ParsingException(source(ctx), "Does not recognize type %s", type);
+                throw new ParsingException(source(ctx), "Does not recognize type {}", type);
         }
     }
 
@@ -384,7 +382,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
         try {
             extract = Extract.valueOf(fieldString.toUpperCase(Locale.ROOT));
         } catch (IllegalArgumentException ex) {
-            throw new ParsingException(source, format(Locale.ROOT, "Invalid EXTRACT field %s", fieldString));
+            throw new ParsingException(source, "Invalid EXTRACT field {}", fieldString);
         }
         return extract.toFunction(source, expression(ctx.valueExpression()), timeZone);
     }
@@ -422,7 +420,7 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
         if (type == SqlBaseParser.OR) {
             return new Or(loc, left, right);
         }
-        throw new ParsingException(loc, format(Locale.ROOT, "Don't know how to parse %s", ctx));
+        throw new ParsingException(loc, "Don't know how to parse {}", ctx);
     }
 
 

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/IdentifierBuilder.java

@@ -28,10 +28,10 @@ abstract class IdentifierBuilder extends AbstractBuilder {
         for (int i = 0; i < index.length(); i++) {
             char c = index.charAt(i);
             if (Character.isUpperCase(c)) {
-                throw new ParsingException(source, "Invalid index name (needs to be lowercase) %s", index);
+                throw new ParsingException(source, "Invalid index name (needs to be lowercase) {}", index);
             }
             if (c == '\\' || c == '/' || c == '<' || c == '>' || c == '|' || c == ',' || c == ' ') {
-                throw new ParsingException(source, "Invalid index name (illegal character %c) %s", c, index);
+                throw new ParsingException(source, "Invalid index name (illegal character {}) {}", c, index);
             }
         }
     }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java

@@ -67,7 +67,7 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
         Map<String, SubQueryAlias> cteRelations = new LinkedHashMap<>(namedQueries.size());
         for (SubQueryAlias namedQuery : namedQueries) {
             if (cteRelations.put(namedQuery.alias(), namedQuery) != null) {
-                throw new ParsingException(namedQuery.location(), "Duplicate alias %s", namedQuery.alias());
+                throw new ParsingException(namedQuery.location(), "Duplicate alias {}", namedQuery.alias());
             }
         }
 

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ParsingException.java

@@ -6,21 +6,19 @@
 package org.elasticsearch.xpack.sql.parser;
 
 import org.antlr.v4.runtime.RecognitionException;
+import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.xpack.sql.ClientSqlException;
 import org.elasticsearch.xpack.sql.tree.Location;
 
 import java.util.Locale;
 
-import static java.lang.String.format;
-
 public class ParsingException extends ClientSqlException {
     private final int line;
     private final int charPositionInLine;
 
     public ParsingException(String message, Exception cause, int line, int charPositionInLine) {
         super(message, cause);
-
         this.line = line;
         this.charPositionInLine = charPositionInLine;
     }
@@ -30,7 +28,9 @@ public class ParsingException extends ClientSqlException {
     }
 
     public ParsingException(Location nodeLocation, String message, Object... args) {
-        this(format(Locale.ROOT, message, args), null, nodeLocation.getLineNumber(), nodeLocation.getColumnNumber());
+        super(message, args);
+        this.line = nodeLocation.getLineNumber();
+        this.charPositionInLine = nodeLocation.getColumnNumber();
     }
 
     public int getLineNumber() {
@@ -52,6 +52,6 @@ public class ParsingException extends ClientSqlException {
 
     @Override
     public String getMessage() {
-        return format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), getErrorMessage());
+        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), getErrorMessage());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/physical/Unexecutable.java

@@ -13,12 +13,10 @@ import org.elasticsearch.xpack.sql.session.SqlSession;
 
 import java.util.Locale;
 
-import static java.lang.String.format;
-
 // this is mainly a marker interface to validate a plan before being executed
 public interface Unexecutable extends Executable {
 
     default void execute(SqlSession session, ActionListener<SchemaRowSet> listener) {
-        throw new PlanningException(format(Locale.ROOT, "Current plan %s is not executable", this));
+        throw new PlanningException("Current plan {} is not executable", this);
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/FoldingException.java

@@ -12,8 +12,6 @@ import org.elasticsearch.xpack.sql.tree.Node;
 
 import java.util.Locale;
 
-import static java.lang.String.format;
-
 public class FoldingException extends ClientSqlException {
 
     private final int line;
@@ -56,6 +54,6 @@ public class FoldingException extends ClientSqlException {
 
     @Override
     public String getMessage() {
-        return format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
+        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/PlanningException.java

@@ -12,11 +12,8 @@ import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.util.StringUtils;
 
 import java.util.Collection;
-import java.util.Locale;
 import java.util.stream.Collectors;
 
-import static java.lang.String.format;
-
 public class PlanningException extends ClientSqlException {
     public PlanningException(String message, Object... args) {
         super(message, args);

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java

@@ -339,7 +339,7 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
                             });
 
                             if (!proc.resolved()) {
-                                throw new FoldingException(child, "Cannot find grouping for '%s'", Expressions.name(child));
+                                throw new FoldingException(child, "Cannot find grouping for '{}'", Expressions.name(child));
                             }
 
                             // add the computed column
@@ -486,7 +486,7 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
                                     qContainer = qContainer.sort(new AttributeSort(at, direction));
                                 } else if (!sfa.orderBy().foldable()) {
                                     // ignore constant
-                                    throw new PlanningException("does not know how to order by expression %s", sfa.orderBy());
+                                    throw new PlanningException("does not know how to order by expression {}", sfa.orderBy());
                                 }
                             } else {
                                 // nope, use scripted sorting

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java

@@ -231,7 +231,7 @@ abstract class QueryTranslator {
             if (exp instanceof NamedExpression) {
                 return groupMap.get(((NamedExpression) exp).id());
             }
-            throw new SqlIllegalArgumentException("Don't know how to find group for expression %s", exp);
+            throw new SqlIllegalArgumentException("Don't know how to find group for expression {}", exp);
         }
 
         @Override
@@ -284,7 +284,7 @@ abstract class QueryTranslator {
                         agg = new GroupByScriptAgg(aggId, AggPath.bucketValue(propertyPath), nameOf(exp), sf.asScript());
                     }
                     else {
-                        throw new SqlIllegalArgumentException("Cannot GROUP BY function %s", exp);
+                        throw new SqlIllegalArgumentException("Cannot GROUP BY function {}", exp);
                     }
                 }
                 else {
@@ -294,7 +294,7 @@ abstract class QueryTranslator {
                 aggMap.put(ne.id(), agg);
             }
             else {
-                throw new SqlIllegalArgumentException("Don't know how to group on %s", exp.nodeString());
+                throw new SqlIllegalArgumentException("Don't know how to group on {}", exp.nodeString());
             }
         }
         return new GroupingContext(aggMap, propertyPath);
@@ -396,14 +396,14 @@ abstract class QueryTranslator {
         if (e instanceof Literal) {
             return String.valueOf(e.fold());
         }
-        throw new SqlIllegalArgumentException("Cannot determine name for %s", e);
+        throw new SqlIllegalArgumentException("Cannot determine name for {}", e);
     }
 
     static String idOf(Expression e) {
         if (e instanceof NamedExpression) {
             return ((NamedExpression) e).id().toString();
         }
-        throw new SqlIllegalArgumentException("Cannot determine id for %s", e);
+        throw new SqlIllegalArgumentException("Cannot determine id for {}", e);
     }
 
     static String dateFormat(Expression e) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/Aggs.java

@@ -115,7 +115,7 @@ public class Aggs {
             }
         }
 
-        throw new SqlIllegalArgumentException("Does not know how to handle type %s", agg);
+        throw new SqlIllegalArgumentException("Does not know how to handle type {}", agg);
     }
 
     public Aggs addAgg(PipelineAgg pipelineAgg) {
@@ -147,7 +147,7 @@ public class Aggs {
                 return with(groups);
             }
         }
-        throw new SqlIllegalArgumentException("Could not find group named %s", groupId);
+        throw new SqlIllegalArgumentException("Could not find group named {}", groupId);
     }
 
     public Aggs addAgg(String groupId, PipelineAgg child) {
@@ -165,7 +165,7 @@ public class Aggs {
                 return with(groups);
             }
         }
-        throw new SqlIllegalArgumentException("Could not find group named %s", groupId);
+        throw new SqlIllegalArgumentException("Could not find group named {}", groupId);
     }
 
     public GroupingAgg findGroupForAgg(String groupOrAggId) {
@@ -199,7 +199,7 @@ public class Aggs {
                 return with(groups);
             }
         }
-        throw new SqlIllegalArgumentException("Could not find group named %s", group.id());
+        throw new SqlIllegalArgumentException("Could not find group named {}", group.id());
     }
 
     public Aggs with(List<GroupingAgg> groups) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java

@@ -284,7 +284,7 @@ public class QueryContainer {
             return new Tuple<>(this, new ComputedRef(new ScoreProcessorDefinition(attr.location(), attr)));
         }
 
-        throw new SqlIllegalArgumentException("Unknown output attribute %s", attr);
+        throw new SqlIllegalArgumentException("Unknown output attribute {}", attr);
     }
 
     public QueryContainer addColumn(FieldExtraction ref) {

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java

@@ -357,9 +357,9 @@ public abstract class DataTypeConversion {
                 try {
                     return converter.apply(value.toString());
                 } catch (NumberFormatException e) {
-                    throw new SqlIllegalArgumentException("cannot cast [%s] to [%s]", value, to, e);
+                    throw new SqlIllegalArgumentException(e, "cannot cast [{}] to [{}]", value, to);
                 } catch (IllegalArgumentException e) {
-                    throw new SqlIllegalArgumentException("cannot cast [%s] to [%s]:%s", value, to, e.getMessage(), e);
+                    throw new SqlIllegalArgumentException(e, "cannot cast [{}] to [{}]:{}", value, to, e.getMessage());
                 }
             };
         }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java

@@ -49,6 +49,6 @@ public abstract class DataTypes {
         if (value instanceof String) {
             return DataType.KEYWORD;
         }
-        throw new SqlIllegalArgumentException("No idea what's the DataType for %s", value.getClass());
+        throw new SqlIllegalArgumentException("No idea what's the DataType for {}", value.getClass());
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/ReflectionUtils.java

@@ -23,7 +23,7 @@ public class ReflectionUtils {
         if (t instanceof ParameterizedType) {
             Type[] typeArguments = ((ParameterizedType) t).getActualTypeArguments();
             if (typeArguments.length != 1) {
-                throw new SqlIllegalArgumentException("Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments), t);
+                throw new SqlIllegalArgumentException("Unexpected number of type arguments {} for {}", Arrays.toString(typeArguments), t);
             }
 
             return detectType(typeArguments[0]);
@@ -36,7 +36,7 @@ public class ReflectionUtils {
             Type[] upperBounds = wt.getUpperBounds();
 
             if (upperBounds.length != 1) {
-                throw new SqlIllegalArgumentException("Unexpected number of upper bounds %s for %s", Arrays.toString(upperBounds), t);
+                throw new SqlIllegalArgumentException("Unexpected number of upper bounds {} for {}", Arrays.toString(upperBounds), t);
             }
 
             return detectType(upperBounds[0]);
@@ -45,7 +45,7 @@ public class ReflectionUtils {
             return detectType(((GenericArrayType) t).getGenericComponentType());
         }
 
-        throw new SqlIllegalArgumentException("Unrecognized type %s", t);
+        throw new SqlIllegalArgumentException("Unrecognized type {}", t);
     }
     
     @SuppressWarnings("unchecked")
@@ -55,7 +55,7 @@ public class ReflectionUtils {
             if (type instanceof ParameterizedType) {
                 Type[] typeArguments = ((ParameterizedType) type).getActualTypeArguments();
                 if (typeArguments.length != 2 && typeArguments.length != 1) {
-                    throw new SqlIllegalArgumentException("Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments),
+                    throw new SqlIllegalArgumentException("Unexpected number of type arguments {} for {}", Arrays.toString(typeArguments),
                             c);
                 }
 
@@ -63,7 +63,7 @@ public class ReflectionUtils {
             }
             clazz = clazz.getSuperclass();
         }
-        throw new SqlIllegalArgumentException("Unexpected class structure for class %s", c);
+        throw new SqlIllegalArgumentException("Unexpected class structure for class {}", c);
     }
     
     // remove packaging from the name - strategy used for naming rules by default  

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java

@@ -13,7 +13,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.execution.ExecutionException;
+import org.elasticsearch.xpack.sql.SqlException;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -173,7 +173,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase<Fiel
         SearchHit hit = new SearchHit(1);
         DocumentField field = new DocumentField(fieldName, asList("a", "b"));
         hit.fields(singletonMap(fieldName, field));
-        ExecutionException ex = expectThrows(ExecutionException.class, () -> fe.get(hit));
+        SqlException ex = expectThrows(SqlException.class, () -> fe.get(hit));
         assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported"));
     }
 
@@ -188,7 +188,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase<Fiel
         source.endObject();
         BytesReference sourceRef = source.bytes();
         hit.sourceRef(sourceRef);
-        ExecutionException ex = expectThrows(ExecutionException.class, () -> fe.get(hit));
+        SqlException ex = expectThrows(SqlException.class, () -> fe.get(hit));
         assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported"));
     }
 
@@ -218,7 +218,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase<Fiel
         FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d", false);
         Object value = randomNonNullValue();
         Map<String, Object> map = singletonMap("a", singletonMap("b", singletonMap("c", value)));
-        ExecutionException ex = expectThrows(ExecutionException.class, () -> fe.extractFromSource(map));
+        SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
         assertThat(ex.getMessage(), is("Cannot extract value [a.b.c.d] from source"));
     }
 
@@ -226,7 +226,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase<Fiel
         FieldHitExtractor fe = new FieldHitExtractor("a", false);
         Object value = randomValue();
         Map<String, Object> map = singletonMap("a", asList(value, value));
-        ExecutionException ex = expectThrows(ExecutionException.class, () -> fe.extractFromSource(map));
+        SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
         assertThat(ex.getMessage(), is("Arrays (returned by [a]) are not supported"));
     }