honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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@74c4c6e4ad
This commit is contained in:
Nik Everett 2017-12-30 19:09:55 -05:00 committed by GitHub
parent 16f996da92
commit c52b3350ef
3 changed files with 27 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
dev-tools/checkstyle_suppressions.xml
View File

@ -8,8 +8,6 @@
<!-- These files are generated by ANTLR so its silly to hold them to our rules. --> <!-- These files are generated by ANTLR so its silly to hold them to our rules. -->
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]parser[/\\]SqlBase(Base(Listener|Visitor)|Lexer|Listener|Parser|Visitor).java" checks="." /> <suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]parser[/\\]SqlBase(Base(Listener|Visitor)|Lexer|Listener|Parser|Visitor).java" checks="." />
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]analysis[/\\]analyzer[/\\]Analyzer.java" checks="LineLength" />
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]analysis[/\\]analyzer[/\\]Verifier.java" checks="LineLength" />
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]Attribute.java" checks="LineLength" /> <suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]Attribute.java" checks="LineLength" />
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]ExpressionIdGenerator.java" checks="LineLength" /> <suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]ExpressionIdGenerator.java" checks="LineLength" />
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]FieldAttribute.java" checks="LineLength" /> <suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]expression[/\\]FieldAttribute.java" checks="LineLength" />

18
sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java
View File

@ -450,7 +450,8 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
AttributeSet conflicting = left.outputSet().intersect(right.outputSet()); AttributeSet conflicting = left.outputSet().intersect(right.outputSet());
if (log.isTraceEnabled()) { 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"); throw new UnsupportedOperationException("don't know how to resolve conficting IDs yet");
@ -517,12 +518,14 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
if (ordinal > 0 && ordinal <= max) { if (ordinal > 0 && ordinal <= max) {
NamedExpression reference = aggregates.get(ordinal - 1); NamedExpression reference = aggregates.get(ordinal - 1);
if (containsAggregate(reference)) { 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); newGroupings.add(reference);
} }
else { 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 { else {
@ -762,7 +765,8 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
// TODO: might be removed // TODO: might be removed
// dedicated count optimization // dedicated count optimization
if (name.toUpperCase(Locale.ROOT).equals("COUNT")) { 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<LogicalPlan> {
} }
List<String> matches = StringUtils.findSimilar(normalizedName, names); List<String> 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); return new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), uf.children(), true, message);
} }
// TODO: look into Generator for significant terms, etc.. // TODO: look into Generator for significant terms, etc..
@ -930,7 +935,8 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
missing = findMissingAggregate(agg, condition); missing = findMissingAggregate(agg, condition);
if (!missing.isEmpty()) { 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); Filter newFilter = new Filter(f.location(), newAgg, condition);
// preserve old output // preserve old output
return new Project(f.location(), newFilter, f.output()); return new Project(f.location(), newFilter, f.output());

22
sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java
View File

@ -215,17 +215,21 @@ abstract class Verifier {
* Check validity of Aggregate/GroupBy. * Check validity of Aggregate/GroupBy.
* This rule is needed for two reasons: * This rule is needed for two reasons:
* 1. a user might specify an invalid aggregate (SELECT foo GROUP BY bar) * 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 * 2. the order/having might contain a non-grouped attribute. This is typically
* (because the expression gets resolved little by little without being pushed down, without the Analyzer modifying anything. * 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<Failure> localFailures, Map<String, Function> resolvedFunctions, Set<LogicalPlan> groupingFailures) { private static boolean checkGroupBy(LogicalPlan p, Set<Failure> localFailures,
Map<String, Function> resolvedFunctions, Set<LogicalPlan> groupingFailures) {
return checkGroupByAgg(p, localFailures, groupingFailures, resolvedFunctions) return checkGroupByAgg(p, localFailures, groupingFailures, resolvedFunctions)
&& checkGroupByOrder(p, localFailures, groupingFailures, resolvedFunctions) && checkGroupByOrder(p, localFailures, groupingFailures, resolvedFunctions)
&& checkGroupByHaving(p, localFailures, groupingFailures, resolvedFunctions); && checkGroupByHaving(p, localFailures, groupingFailures, resolvedFunctions);
} }
// check whether an orderBy failed // check whether an orderBy failed
private static boolean checkGroupByOrder(LogicalPlan p, Set<Failure> localFailures, Set<LogicalPlan> groupingFailures, Map<String, Function> functions) { private static boolean checkGroupByOrder(LogicalPlan p, Set<Failure> localFailures,
Set<LogicalPlan> groupingFailures, Map<String, Function> functions) {
if (p instanceof OrderBy) { if (p instanceof OrderBy) {
OrderBy o = (OrderBy) p; OrderBy o = (OrderBy) p;
if (o.child() instanceof Aggregate) { if (o.child() instanceof Aggregate) {
@ -250,7 +254,8 @@ abstract class Verifier {
} }
private static boolean checkGroupByHaving(LogicalPlan p, Set<Failure> localFailures, Set<LogicalPlan> groupingFailures, Map<String, Function> functions) { private static boolean checkGroupByHaving(LogicalPlan p, Set<Failure> localFailures,
Set<LogicalPlan> groupingFailures, Map<String, Function> functions) {
if (p instanceof Filter) { if (p instanceof Filter) {
Filter f = (Filter) p; Filter f = (Filter) p;
if (f.child() instanceof Aggregate) { 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 // check whether plain columns specified in an agg are mentioned in the group-by
private static boolean checkGroupByAgg(LogicalPlan p, Set<Failure> localFailures, Set<LogicalPlan> groupingFailures, Map<String, Function> functions) { private static boolean checkGroupByAgg(LogicalPlan p, Set<Failure> localFailures,
Set<LogicalPlan> groupingFailures, Map<String, Function> functions) {
if (p instanceof Aggregate) { if (p instanceof Aggregate) {
Aggregate a = (Aggregate) p; Aggregate a = (Aggregate) p;
@ -318,8 +324,8 @@ abstract class Verifier {
return true; return true;
} }
private static boolean checkGroupMatch(Expression e, Node<?> source, List<Expression> groupings, Map<Expression, Node<?>> missing, Map<String, Function> functions) { private static boolean checkGroupMatch(Expression e, Node<?> source, List<Expression> groupings,
Map<Expression, Node<?>> missing, Map<String, Function> functions) {
// resolve FunctionAttribute to backing functions // resolve FunctionAttribute to backing functions
if (e instanceof FunctionAttribute) { if (e instanceof FunctionAttribute) {
FunctionAttribute fa = (FunctionAttribute) e; FunctionAttribute fa = (FunctionAttribute) e;