QL: Extract common Failure class (#52281)

Shared across SQL and EQL

(cherry picked from commit 1aeda20d3ec3d6c885de03c6043dd1e8eab9f230)
This commit is contained in:
Costin Leau 2020-02-13 14:14:42 +02:00 committed by Costin Leau
parent b0ad37126c
commit 5373a77fb9
20 changed files with 52 additions and 219 deletions

View File

@ -6,7 +6,7 @@
package org.elasticsearch.xpack.eql.analysis;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.expression.NamedExpression;
import org.elasticsearch.xpack.ql.expression.UnresolvedAttribute;

View File

@ -7,7 +7,7 @@ package org.elasticsearch.xpack.eql.analysis;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.eql.EqlClientException;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.ql.common.Failure;
import java.util.Collection;

View File

@ -6,8 +6,8 @@
package org.elasticsearch.xpack.eql.analysis;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.ql.capabilities.Unresolvable;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.expression.UnresolvedAttribute;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
@ -23,7 +23,7 @@ import java.util.Map;
import java.util.Set;
import static java.util.stream.Collectors.toMap;
import static org.elasticsearch.xpack.eql.common.Failure.fail;
import static org.elasticsearch.xpack.ql.common.Failure.fail;
/**
* The verifier has the role of checking the analyzed tree for failures and build a list of failures following this check.

View File

@ -6,8 +6,8 @@
package org.elasticsearch.xpack.eql.planner;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.eql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
import java.util.List;

View File

@ -7,7 +7,7 @@ package org.elasticsearch.xpack.eql.planner;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.eql.EqlClientException;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.ql.common.Failure;
import java.util.Collection;

View File

@ -5,15 +5,15 @@
*/
package org.elasticsearch.xpack.eql.planner;
import org.elasticsearch.xpack.eql.common.Failure;
import org.elasticsearch.xpack.eql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.eql.plan.physical.Unexecutable;
import org.elasticsearch.xpack.eql.plan.physical.UnplannedExec;
import org.elasticsearch.xpack.ql.common.Failure;
import java.util.ArrayList;
import java.util.List;
import static org.elasticsearch.xpack.eql.common.Failure.fail;
import static org.elasticsearch.xpack.ql.common.Failure.fail;
abstract class Verifier {

View File

@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.eql.common;
package org.elasticsearch.xpack.ql.common;
import org.elasticsearch.xpack.ql.tree.Location;
import org.elasticsearch.xpack.ql.tree.Node;
@ -36,7 +36,7 @@ public class Failure {
@Override
public int hashCode() {
return Objects.hash(message, node);
return Objects.hash(node);
}
@Override
@ -50,7 +50,7 @@ public class Failure {
}
Failure other = (Failure) obj;
return Objects.equals(message, other.message) && Objects.equals(node, other.node);
return Objects.equals(node, other.node);
}
@Override
@ -67,7 +67,7 @@ public class Failure {
Location l = f.node().source().source();
return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message();
}).collect(Collectors.joining(StringUtils.NEW_LINE,
format("Found {} problem{}\n", failures.size(), failures.size() > 1 ? "s" : StringUtils.EMPTY),
format("Found {} problem{}\n", failures.size(), failures.size() > 1 ? "s" : StringUtils.EMPTY),
StringUtils.EMPTY));
}
}

View File

@ -106,6 +106,6 @@ public abstract class ErrorsTestCase extends CliIntegrationTestCase implements o
}
public static void assertFoundOneProblem(String commandResult) {
assertEquals(START + "Bad request [[3;33;22mFound 1 problem(s)", commandResult);
assertEquals(START + "Bad request [[3;33;22mFound 1 problem", commandResult);
}
}

View File

@ -20,7 +20,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
public void testSelectInvalidSql() throws Exception {
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FRO").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:8: Cannot determine columns for [*]", e.getMessage());
assertEquals("Found 1 problem\nline 1:8: Cannot determine columns for [*]", e.getMessage());
}
}
@ -28,7 +28,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
public void testSelectFromMissingIndex() throws SQLException {
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM test").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:15: Unknown index [test]", e.getMessage());
assertEquals("Found 1 problem\nline 1:15: Unknown index [test]", e.getMessage());
}
}
@ -42,8 +42,8 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM test").executeQuery());
// see https://github.com/elastic/elasticsearch/issues/34719
//assertEquals("Found 1 problem(s)\nline 1:15: [test] doesn't have any types so it is incompatible with sql", e.getMessage());
assertEquals("Found 1 problem(s)\nline 1:15: Unknown index [test]", e.getMessage());
//assertEquals("Found 1 problem\nline 1:15: [test] doesn't have any types so it is incompatible with sql", e.getMessage());
assertEquals("Found 1 problem\nline 1:15: Unknown index [test]", e.getMessage());
}
}
@ -52,7 +52,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
index("test", body -> body.field("test", "test"));
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT missing FROM test").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:8: Unknown column [missing]", e.getMessage());
assertEquals("Found 1 problem\nline 1:8: Unknown column [missing]", e.getMessage());
}
}
@ -61,7 +61,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
index("test", body -> body.field("foo", 1));
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT missing(foo) FROM test").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:8: Unknown function [missing]", e.getMessage());
assertEquals("Found 1 problem\nline 1:8: Unknown function [missing]", e.getMessage());
}
}
@ -71,7 +71,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () ->
c.prepareStatement("SELECT foo, SCORE(), COUNT(*) FROM test GROUP BY foo").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:13: Cannot use non-grouped column [SCORE()], expected [foo]", e.getMessage());
assertEquals("Found 1 problem\nline 1:13: Cannot use non-grouped column [SCORE()], expected [foo]", e.getMessage());
}
}
@ -82,7 +82,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
SQLException e = expectThrows(SQLException.class, () ->
c.prepareStatement("SELECT foo, COUNT(*) FROM test GROUP BY foo ORDER BY SCORE()").executeQuery());
assertEquals(
"Found 1 problem(s)\nline 1:54: Cannot order by non-grouped column [SCORE()], expected [foo] or an aggregate function",
"Found 1 problem\nline 1:54: Cannot order by non-grouped column [SCORE()], expected [foo] or an aggregate function",
e.getMessage());
}
}
@ -93,7 +93,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () ->
c.prepareStatement("SELECT COUNT(*) FROM test GROUP BY SCORE()").executeQuery());
assertEquals("Found 1 problem(s)\nline 1:36: Cannot use [SCORE()] for grouping", e.getMessage());
assertEquals("Found 1 problem\nline 1:36: Cannot use [SCORE()] for grouping", e.getMessage());
}
}
@ -113,7 +113,7 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
try (Connection c = esJdbc()) {
SQLException e = expectThrows(SQLException.class, () ->
c.prepareStatement("SELECT SIN(SCORE()) FROM test").executeQuery());
assertThat(e.getMessage(), startsWith("Found 1 problem(s)\nline 1:12: [SCORE()] cannot be an argument to a function"));
assertThat(e.getMessage(), startsWith("Found 1 problem\nline 1:12: [SCORE()] cannot be an argument to a function"));
}
}

View File

@ -1,59 +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.analysis;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.ql.tree.Location;
import org.elasticsearch.xpack.ql.tree.Node;
import org.elasticsearch.xpack.sql.SqlClientException;
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
public class AnalysisException extends SqlClientException {
private final int line;
private final int column;
public AnalysisException(Node<?> source, String message, Object... args) {
super(message, args);
Location loc = Location.EMPTY;
if (source != null && source.source() != null) {
loc = source.source().source();
}
this.line = loc.getLineNumber();
this.column = loc.getColumnNumber();
}
public AnalysisException(Node<?> source, String message, Throwable cause) {
super(message, cause);
Location loc = Location.EMPTY;
if (source != null && source.source() != null) {
loc = source.source().source();
}
this.line = loc.getLineNumber();
this.column = loc.getColumnNumber();
}
public int getLineNumber() {
return line;
}
public int getColumnNumber() {
return column;
}
@Override
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
@Override
public String getMessage() {
return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), super.getMessage());
}
}

View File

@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.analysis.analyzer;
import org.elasticsearch.common.logging.LoggerMessageFormat;
import org.elasticsearch.xpack.ql.capabilities.Resolvables;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Alias;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.expression.AttributeMap;
@ -45,7 +46,6 @@ import org.elasticsearch.xpack.ql.type.InvalidMappedField;
import org.elasticsearch.xpack.ql.type.UnsupportedEsField;
import org.elasticsearch.xpack.ql.util.CollectionUtils;
import org.elasticsearch.xpack.ql.util.Holder;
import org.elasticsearch.xpack.sql.analysis.analyzer.Verifier.Failure;
import org.elasticsearch.xpack.sql.expression.Foldables;
import org.elasticsearch.xpack.sql.expression.SubQueryExpression;
import org.elasticsearch.xpack.sql.expression.function.scalar.Cast;

View File

@ -5,31 +5,20 @@
*/
package org.elasticsearch.xpack.sql.analysis.analyzer;
import org.elasticsearch.xpack.ql.tree.Location;
import org.elasticsearch.xpack.ql.util.StringUtils;
import org.elasticsearch.xpack.sql.analysis.AnalysisException;
import org.elasticsearch.xpack.sql.analysis.analyzer.Verifier.Failure;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.sql.SqlClientException;
import java.util.Collection;
import java.util.stream.Collectors;
public class VerificationException extends AnalysisException {
private final Collection<Failure> failures;
public class VerificationException extends SqlClientException {
protected VerificationException(Collection<Failure> sources) {
super(null, StringUtils.EMPTY);
failures = sources;
super(Failure.failMessage(sources));
}
@Override
public String getMessage() {
return failures.stream()
.map(f -> {
Location l = f.node().source().source();
return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message();
})
.collect(Collectors.joining(StringUtils.NEW_LINE, "Found " + failures.size() + " problem(s)\n", StringUtils.EMPTY));
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
}

View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.analysis.analyzer;
import org.elasticsearch.xpack.ql.capabilities.Unresolvable;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.Alias;
import org.elasticsearch.xpack.ql.expression.Attribute;
import org.elasticsearch.xpack.ql.expression.AttributeMap;
@ -61,12 +62,11 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Consumer;
import static java.util.stream.Collectors.toMap;
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.ql.common.Failure.fail;
import static org.elasticsearch.xpack.ql.util.CollectionUtils.combine;
import static org.elasticsearch.xpack.sql.stats.FeatureMetric.COMMAND;
import static org.elasticsearch.xpack.sql.stats.FeatureMetric.GROUPBY;
@ -89,52 +89,6 @@ public final class Verifier {
this.metrics = metrics;
}
static class Failure {
private final Node<?> node;
private final String message;
Failure(Node<?> node, String message) {
this.node = node;
this.message = message;
}
Node<?> node() {
return node;
}
String message() {
return message;
}
@Override
public int hashCode() {
return Objects.hash(node);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
Verifier.Failure other = (Verifier.Failure) obj;
return Objects.equals(node, other.node);
}
@Override
public String toString() {
return message;
}
}
private static Failure fail(Node<?> source, String message, Object... args) {
return new Failure(source, format(message, args));
}
public Map<Node<?>, String> verifyFailures(LogicalPlan plan) {
Collection<Failure> failures = verify(plan);
return failures.stream().collect(toMap(Failure::node, Failure::message));

View File

@ -5,10 +5,10 @@
*/
package org.elasticsearch.xpack.sql.planner;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.ql.tree.Node;
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.sql.planner.Verifier.Failure;
import java.util.List;
import java.util.Map;
@ -49,7 +49,7 @@ public class Planner {
public Map<Node<?>, String> verifyMappingPlanFailures(PhysicalPlan plan) {
List<Failure> failures = Verifier.verifyMappingPlan(plan);
return failures.stream().collect(toMap(Failure::source, Failure::message));
return failures.stream().collect(toMap(Failure::node, Failure::message));
}
public PhysicalPlan verifyExecutingPlan(PhysicalPlan plan) {
@ -62,6 +62,6 @@ public class Planner {
public Map<Node<?>, String> verifyExecutingPlanFailures(PhysicalPlan plan) {
List<Failure> failures = Verifier.verifyExecutingPlan(plan);
return failures.stream().collect(toMap(Failure::source, Failure::message));
return failures.stream().collect(toMap(Failure::node, Failure::message));
}
}

View File

@ -6,12 +6,10 @@
package org.elasticsearch.xpack.sql.planner;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.ql.tree.Location;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.sql.SqlClientException;
import org.elasticsearch.xpack.sql.planner.Verifier.Failure;
import java.util.Collection;
import java.util.stream.Collectors;
public class PlanningException extends SqlClientException {
public PlanningException(String message, Object... args) {
@ -19,20 +17,11 @@ public class PlanningException extends SqlClientException {
}
public PlanningException(Collection<Failure> sources) {
super(extractMessage(sources));
super(Failure.failMessage(sources));
}
@Override
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
private static String extractMessage(Collection<Failure> failures) {
return failures.stream()
.map(f -> {
Location l = f.source().source().source();
return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message();
})
.collect(Collectors.joining("\n", "Found " + failures.size() + " problem(s)\n", ""));
}
}

View File

@ -5,8 +5,8 @@
*/
package org.elasticsearch.xpack.sql.planner;
import org.elasticsearch.xpack.ql.common.Failure;
import org.elasticsearch.xpack.ql.expression.function.aggregate.InnerAggregate;
import org.elasticsearch.xpack.ql.tree.Node;
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.sql.plan.physical.PivotExec;
import org.elasticsearch.xpack.sql.plan.physical.Unexecutable;
@ -14,53 +14,11 @@ import org.elasticsearch.xpack.sql.plan.physical.UnplannedExec;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.ql.common.Failure.fail;
abstract class Verifier {
static class Failure {
private final Node<?> source;
private final String message;
Failure(Node<?> source, String message) {
this.source = source;
this.message = message;
}
Node<?> source() {
return source;
}
String message() {
return message;
}
@Override
public int hashCode() {
return source.hashCode();
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
Verifier.Failure other = (Verifier.Failure) obj;
return Objects.equals(source, other.source);
}
}
private static Failure fail(Node<?> source, String message, Object... args) {
return new Failure(source, format(null, message, args));
}
static List<Failure> verifyMappingPlan(PhysicalPlan plan) {
List<Failure> failures = new ArrayList<>();

View File

@ -144,17 +144,17 @@ public class FieldAttributeTests extends ESTestCase {
}
public void testDottedFieldPath() {
assertThat(error("some"), is("Found 1 problem(s)\nline 1:8: Cannot use field [some] type [object] only its subfields"));
assertThat(error("some"), is("Found 1 problem\nline 1:8: Cannot use field [some] type [object] only its subfields"));
}
public void testDottedFieldPathDeeper() {
assertThat(error("some.dotted"),
is("Found 1 problem(s)\nline 1:8: Cannot use field [some.dotted] type [object] only its subfields"));
is("Found 1 problem\nline 1:8: Cannot use field [some.dotted] type [object] only its subfields"));
}
public void testDottedFieldPathTypo() {
assertThat(error("some.dotted.fild"),
is("Found 1 problem(s)\nline 1:8: Unknown column [some.dotted.fild], did you mean [some.dotted.field]?"));
is("Found 1 problem\nline 1:8: Unknown column [some.dotted.fild], did you mean [some.dotted.field]?"));
}
public void testStarExpansionExcludesObjectAndUnsupportedTypes() {
@ -177,13 +177,13 @@ public class FieldAttributeTests extends ESTestCase {
VerificationException ex = expectThrows(VerificationException.class, () -> plan("SELECT test.bar FROM test"));
assertEquals(
"Found 1 problem(s)\nline 1:8: Reference [test.bar] is ambiguous (to disambiguate use quotes or qualifiers); "
"Found 1 problem\nline 1:8: Reference [test.bar] is ambiguous (to disambiguate use quotes or qualifiers); "
+ "matches any of [\"test\".\"bar\", \"test\".\"test.bar\"]",
ex.getMessage());
ex = expectThrows(VerificationException.class, () -> plan("SELECT test.test FROM test"));
assertEquals(
"Found 1 problem(s)\nline 1:8: Reference [test.test] is ambiguous (to disambiguate use quotes or qualifiers); "
"Found 1 problem\nline 1:8: Reference [test.test] is ambiguous (to disambiguate use quotes or qualifiers); "
+ "matches any of [\"test\".\"test\", \"test\".\"test.test\"]",
ex.getMessage());

View File

@ -50,9 +50,11 @@ public class VerifierErrorMessagesTests extends ESTestCase {
private String error(IndexResolution getIndexResult, String sql) {
Analyzer analyzer = new Analyzer(SqlTestUtils.TEST_CFG, new SqlFunctionRegistry(), getIndexResult, new Verifier(new Metrics()));
VerificationException e = expectThrows(VerificationException.class, () -> analyzer.analyze(parser.createStatement(sql), true));
assertTrue(e.getMessage().startsWith("Found "));
String header = "Found 1 problem(s)\nline ";
return e.getMessage().substring(header.length());
String message = e.getMessage();
assertTrue(message.startsWith("Found "));
String pattern = "\nline ";
int index = message.indexOf(pattern);
return message.substring(index + pattern.length());
}
private LogicalPlan accept(String sql) {

View File

@ -87,7 +87,7 @@ public class QuerierTests extends ESTestCase {
@SuppressWarnings({"rawtypes", "unchecked"})
public void testAggSorting_FourFields() {
List<Comparator> comparators = Arrays.asList(
List<Comparator> comparators = Arrays.<Comparator> asList(
Comparator.naturalOrder(),
Comparator.naturalOrder(),
Comparator.reverseOrder(),

View File

@ -61,7 +61,7 @@ public class PostOptimizerVerifierTests extends ESTestCase {
private String error(IndexResolution getIndexResult, String sql) {
PlanningException e = expectThrows(PlanningException.class, () -> plan(sql));
assertTrue(e.getMessage().startsWith("Found "));
String header = "Found 1 problem(s)\nline ";
String header = "Found 1 problem\nline ";
return e.getMessage().substring(header.length());
}