From c52b3350efdb116834078f003e4156639afaf322 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Sat, 30 Dec 2017 19:09:55 -0500 Subject: [PATCH] SQL: Break long lines in the analyzer package (elastic/x-pack-elasticsearch#3455) Break lines longer than 140 characters in the analyzer package into multiple lines so they are easier to read and to appease checkstyle. Original commit: elastic/x-pack-elasticsearch@74c4c6e4ad8968f3c8c4ede8557297ff54dcd9fa --- dev-tools/checkstyle_suppressions.xml | 2 -- .../xpack/sql/analysis/analyzer/Analyzer.java | 18 +++++++++----- .../xpack/sql/analysis/analyzer/Verifier.java | 24 ++++++++++++------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/dev-tools/checkstyle_suppressions.xml b/dev-tools/checkstyle_suppressions.xml index fefe17cd316..4238f7bdfdb 100644 --- a/dev-tools/checkstyle_suppressions.xml +++ b/dev-tools/checkstyle_suppressions.xml @@ -8,8 +8,6 @@ - - diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java index dbe7194d302..0d793338c3d 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java @@ -450,7 +450,8 @@ public class Analyzer extends RuleExecutor { AttributeSet conflicting = left.outputSet().intersect(right.outputSet()); if (log.isTraceEnabled()) { - log.trace("Trying to resolve conflicts {} between left {} and right {}", conflicting, left.nodeString(), right.nodeString()); + log.trace("Trying to resolve conflicts " + conflicting + " between left " + left.nodeString() + + " and right " + right.nodeString()); } throw new UnsupportedOperationException("don't know how to resolve conficting IDs yet"); @@ -517,12 +518,14 @@ public class Analyzer extends RuleExecutor { if (ordinal > 0 && ordinal <= max) { NamedExpression reference = aggregates.get(ordinal - 1); if (containsAggregate(reference)) { - throw new AnalysisException(exp, "Group ordinal %d refers to an aggregate function %s which is not compatible/allowed with GROUP BY", ordinal, reference.nodeName()); + throw new AnalysisException(exp, "Group ordinal " + ordinal + " refers to an aggregate function " + + reference.nodeName() + " which is not compatible/allowed with GROUP BY"); } newGroupings.add(reference); } else { - throw new AnalysisException(exp, "Invalid ordinal %d specified in Aggregate (valid range is [1, %d])", ordinal, max); + throw new AnalysisException(exp, "Invalid ordinal " + ordinal + + " specified in Aggregate (valid range is [1, " + max + "])"); } } else { @@ -762,7 +765,8 @@ public class Analyzer extends RuleExecutor { // TODO: might be removed // dedicated count optimization if (name.toUpperCase(Locale.ROOT).equals("COUNT")) { - uf = new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), singletonList(Literal.of(uf.arguments().get(0).location(), Integer.valueOf(1)))); + uf = new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), + singletonList(Literal.of(uf.arguments().get(0).location(), Integer.valueOf(1)))); } } @@ -793,7 +797,8 @@ public class Analyzer extends RuleExecutor { } List matches = StringUtils.findSimilar(normalizedName, names); - String message = matches.isEmpty() ? uf.unresolvedMessage() : UnresolvedFunction.errorMessage(normalizedName, matches); + String message = matches.isEmpty() ? + uf.unresolvedMessage() : UnresolvedFunction.errorMessage(normalizedName, matches); return new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), uf.children(), true, message); } // TODO: look into Generator for significant terms, etc.. @@ -930,7 +935,8 @@ public class Analyzer extends RuleExecutor { missing = findMissingAggregate(agg, condition); if (!missing.isEmpty()) { - Aggregate newAgg = new Aggregate(agg.location(), agg.child(), agg.groupings(), combine(agg.aggregates(), missing)); + Aggregate newAgg = new Aggregate(agg.location(), agg.child(), agg.groupings(), + combine(agg.aggregates(), missing)); Filter newFilter = new Filter(f.location(), newAgg, condition); // preserve old output return new Project(f.location(), newFilter, f.output()); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java index a330bfeef30..e883c3f533b 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java @@ -215,17 +215,21 @@ abstract class Verifier { * Check validity of Aggregate/GroupBy. * This rule is needed for two reasons: * 1. a user might specify an invalid aggregate (SELECT foo GROUP BY bar) - * 2. the order/having might contain a non-grouped attribute. This is typically caught by the Analyzer however if wrapped in a function (ABS()) it gets resolved - * (because the expression gets resolved little by little without being pushed down, without the Analyzer modifying anything. + * 2. the order/having might contain a non-grouped attribute. This is typically + * caught by the Analyzer however if wrapped in a function (ABS()) it gets resolved + * (because the expression gets resolved little by little without being pushed down, + * without the Analyzer modifying anything. */ - private static boolean checkGroupBy(LogicalPlan p, Set localFailures, Map resolvedFunctions, Set groupingFailures) { + private static boolean checkGroupBy(LogicalPlan p, Set localFailures, + Map resolvedFunctions, Set groupingFailures) { return checkGroupByAgg(p, localFailures, groupingFailures, resolvedFunctions) && checkGroupByOrder(p, localFailures, groupingFailures, resolvedFunctions) && checkGroupByHaving(p, localFailures, groupingFailures, resolvedFunctions); } // check whether an orderBy failed - private static boolean checkGroupByOrder(LogicalPlan p, Set localFailures, Set groupingFailures, Map functions) { + private static boolean checkGroupByOrder(LogicalPlan p, Set localFailures, + Set groupingFailures, Map functions) { if (p instanceof OrderBy) { OrderBy o = (OrderBy) p; if (o.child() instanceof Aggregate) { @@ -250,7 +254,8 @@ abstract class Verifier { } - private static boolean checkGroupByHaving(LogicalPlan p, Set localFailures, Set groupingFailures, Map functions) { + private static boolean checkGroupByHaving(LogicalPlan p, Set localFailures, + Set groupingFailures, Map functions) { if (p instanceof Filter) { Filter f = (Filter) p; if (f.child() instanceof Aggregate) { @@ -275,7 +280,8 @@ abstract class Verifier { // check whether plain columns specified in an agg are mentioned in the group-by - private static boolean checkGroupByAgg(LogicalPlan p, Set localFailures, Set groupingFailures, Map functions) { + private static boolean checkGroupByAgg(LogicalPlan p, Set localFailures, + Set groupingFailures, Map functions) { if (p instanceof Aggregate) { Aggregate a = (Aggregate) p; @@ -318,8 +324,8 @@ abstract class Verifier { return true; } - private static boolean checkGroupMatch(Expression e, Node source, List groupings, Map> missing, Map functions) { - + private static boolean checkGroupMatch(Expression e, Node source, List groupings, + Map> missing, Map functions) { // resolve FunctionAttribute to backing functions if (e instanceof FunctionAttribute) { FunctionAttribute fa = (FunctionAttribute) e; @@ -384,4 +390,4 @@ abstract class Verifier { .forEach(exp -> localFailures.add(fail(exp, "[SCORE()] cannot be an argument to a function"))), Function.class)); } -} \ No newline at end of file +}