SQL: Break long lines in planner package (elastic/x-pack-elasticsearch#3457)
Break lines longer than 140 characters in the planner package to make them a bit easier to read and make checkstyle happy. It'll make some merge conflicts but we should be able to deal with them easilly enough. Original commit: elastic/x-pack-elasticsearch@de8c116f33
This commit is contained in:
Nik Everett
GitHub
parent
bc8103e268
commit
1cc64f4ca1
|
|
@ -10,9 +10,6 @@
|
|||
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]optimizer[/\\]Optimizer.java" checks="LineLength" />
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]plan[/\\]logical[/\\]command[/\\]Explain.java" checks="LineLength" />
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]planner[/\\]PlanningException.java" checks="LineLength" />
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]planner[/\\]QueryFolder.java" checks="LineLength" />
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]planner[/\\]QueryTranslator.java" checks="LineLength" />
|
||||
<suppress files="sql[/\\]server[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]tree[/\\]NodeUtils.java" checks="LineLength" />
|
||||
|
||||
<suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]common[/\\]action[/\\]XPackDeleteByQueryAction.java" checks="LineLength" />
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.planner;
|
|||
import org.elasticsearch.rest.RestStatus;
|
||||
import org.elasticsearch.xpack.sql.ClientSqlException;
|
||||
import org.elasticsearch.xpack.sql.planner.Verifier.Failure;
|
||||
import org.elasticsearch.xpack.sql.tree.Location;
|
||||
import org.elasticsearch.xpack.sql.util.StringUtils;
|
||||
|
||||
import java.util.Collection;
|
||||
|
|
@ -32,7 +33,10 @@ public class PlanningException extends ClientSqlException {
|
|||
|
||||
private static String extractMessage(Collection<Failure> failures) {
|
||||
return failures.stream()
|
||||
.map(f -> format(Locale.ROOT, "line %s:%s: %s", f.source().location().getLineNumber(), f.source().location().getColumnNumber(), f.message()))
|
||||
.collect(Collectors.joining(StringUtils.NEW_LINE, "Found " + failures.size() + " problem(s)\n", StringUtils.EMPTY));
|
||||
.map(f -> {
|
||||
Location l = f.source().location();
|
||||
return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message();
|
||||
})
|
||||
.collect(Collectors.joining("\n", "Found " + failures.size() + " problem(s)\n", ""));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -134,7 +134,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
}
|
||||
}
|
||||
|
||||
QueryContainer clone = new QueryContainer(queryC.query(), queryC.aggs(), queryC.columns(), aliases, queryC.pseudoFunctions(), processors, queryC.sort(), queryC.limit());
|
||||
QueryContainer clone = new QueryContainer(queryC.query(), queryC.aggs(), queryC.columns(), aliases,
|
||||
queryC.pseudoFunctions(), processors, queryC.sort(), queryC.limit());
|
||||
return new EsQueryExec(exec.location(), exec.index(), project.output(), clone);
|
||||
}
|
||||
return project;
|
||||
|
|
@ -151,7 +152,10 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
|
||||
QueryTranslation qt = toQuery(plan.condition(), plan.isHaving());
|
||||
|
||||
Query query = (qContainer.query() != null || qt.query != null) ? and(plan.location(), qContainer.query(), qt.query) : null;
|
||||
Query query = null;
|
||||
if (qContainer.query() != null || qt.query != null) {
|
||||
query = and(plan.location(), qContainer.query(), qt.query);
|
||||
}
|
||||
Aggs aggs = addPipelineAggs(qContainer, qt, plan);
|
||||
|
||||
qContainer = new QueryContainer(query, aggs, qContainer.columns(), qContainer.aliases(),
|
||||
|
|
@ -193,7 +197,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
}
|
||||
|
||||
if (groupAgg == null) {
|
||||
throw new FoldingException(fexec, "Cannot find group for agg %s referrenced by agg filter %s(%s)", refId, filter.name(), filter);
|
||||
throw new FoldingException(fexec, "Cannot find group for agg " + refId
|
||||
+ " referrenced by agg filter " + filter.name() + "(" + filter + ")");
|
||||
}
|
||||
|
||||
String path = groupAgg.asParentPath();
|
||||
|
|
@ -288,7 +293,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
return p;
|
||||
}
|
||||
|
||||
// get the backing expression and check if it belongs to a agg group or whether it's an expression in the first place
|
||||
// get the backing expression and check if it belongs to a agg group or whether it's
|
||||
// an expression in the first place
|
||||
Expression exp = p.expression();
|
||||
GroupingAgg matchingGroup = null;
|
||||
if (groupingContext != null) {
|
||||
|
|
@ -299,16 +305,21 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
// a scalar function can be used only if has already been mentioned for grouping
|
||||
// (otherwise it is the opposite of grouping)
|
||||
if (exp instanceof ScalarFunction) {
|
||||
throw new FoldingException(exp, "Scalar function %s can be used only if included already in grouping", exp.toString());
|
||||
throw new FoldingException(exp, "Scalar function " +exp.toString()
|
||||
+ " can be used only if included already in grouping");
|
||||
}
|
||||
}
|
||||
|
||||
// found match for expression; if it's an attribute or scalar, end the processing chain with the reference to the backing agg
|
||||
// found match for expression; if it's an attribute or scalar, end the processing chain with
|
||||
// the reference to the backing agg
|
||||
if (matchingGroup != null) {
|
||||
if (exp instanceof Attribute || exp instanceof ScalarFunction) {
|
||||
Processor action = null;
|
||||
// special handling of dates since aggs return the typed Date object which needs extraction
|
||||
// instead of handling this in the scroller, the folder handles this as it already got access to the extraction action
|
||||
/*
|
||||
* special handling of dates since aggs return the typed Date object which needs
|
||||
* extraction instead of handling this in the scroller, the folder handles this
|
||||
* as it already got access to the extraction action
|
||||
*/
|
||||
if (exp instanceof DateTimeHistogramFunction) {
|
||||
action = ((UnaryProcessorDefinition) p).action();
|
||||
}
|
||||
|
|
@ -318,7 +329,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
// or found an aggregate expression (which has to work on an attribute used for grouping)
|
||||
// (can happen when dealing with a root group)
|
||||
if (Functions.isAggregate(exp)) {
|
||||
Tuple<QueryContainer, AggPathInput> withFunction = addAggFunction(matchingGroup, (AggregateFunction) exp, compoundAggMap, qC.get());
|
||||
Tuple<QueryContainer, AggPathInput> withFunction = addAggFunction(matchingGroup,
|
||||
(AggregateFunction) exp, compoundAggMap, qC.get());
|
||||
qC.set(withFunction.v1());
|
||||
return withFunction.v2();
|
||||
}
|
||||
|
|
@ -334,7 +346,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
queryC = qC.get().addColumn(new ComputedRef(proc));
|
||||
|
||||
// TODO: is this needed?
|
||||
// redirect the alias to the scalar group id (changing the id altogether doesn't work it is already used in the aggpath)
|
||||
// redirect the alias to the scalar group id (changing the id altogether doesn't work it is
|
||||
// already used in the aggpath)
|
||||
//aliases.put(as.toAttribute(), sf.toAttribute());
|
||||
}
|
||||
// apply the same logic above (for function inputs) to non-scalar functions with small variantions:
|
||||
|
|
@ -352,8 +365,10 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
}
|
||||
else {
|
||||
// the only thing left is agg function
|
||||
Check.isTrue(Functions.isAggregate(child), "Expected aggregate function inside alias; got %s", child.nodeString());
|
||||
Tuple<QueryContainer, AggPathInput> withAgg = addAggFunction(matchingGroup, (AggregateFunction) child, compoundAggMap, queryC);
|
||||
Check.isTrue(Functions.isAggregate(child),
|
||||
"Expected aggregate function inside alias; got %s", child.nodeString());
|
||||
Tuple<QueryContainer, AggPathInput> withAgg = addAggFunction(matchingGroup,
|
||||
(AggregateFunction) child, compoundAggMap, queryC);
|
||||
//FIXME: what about inner key
|
||||
queryC = withAgg.v1().addAggColumn(withAgg.v2().context());
|
||||
if (withAgg.v2().innerKey() != null) {
|
||||
|
|
@ -382,7 +397,8 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
|
|||
return a;
|
||||
}
|
||||
|
||||
private Tuple<QueryContainer, AggPathInput> addAggFunction(GroupingAgg parentAgg, AggregateFunction f, Map<CompoundNumericAggregate, String> compoundAggMap, QueryContainer queryC) {
|
||||
private Tuple<QueryContainer, AggPathInput> addAggFunction(GroupingAgg parentAgg, AggregateFunction f,
|
||||
Map<CompoundNumericAggregate, String> compoundAggMap, QueryContainer queryC) {
|
||||
String functionId = f.functionId();
|
||||
// handle count as a special case agg
|
||||
if (f instanceof Count) {
|
||||
|
|
|
|||
|
|
@ -273,7 +273,8 @@ abstract class QueryTranslator {
|
|||
// dates are handled differently because of date histograms
|
||||
if (exp instanceof DateTimeHistogramFunction) {
|
||||
DateTimeHistogramFunction dthf = (DateTimeHistogramFunction) exp;
|
||||
agg = new GroupByDateAgg(aggId, AggPath.bucketValue(propertyPath), nameOf(exp), dthf.interval(), dthf.timeZone());
|
||||
agg = new GroupByDateAgg(aggId, AggPath.bucketValue(propertyPath), nameOf(exp),
|
||||
dthf.interval(), dthf.timeZone());
|
||||
}
|
||||
// all other scalar functions become a script
|
||||
else if (exp instanceof ScalarFunction) {
|
||||
|
|
@ -418,7 +419,8 @@ abstract class QueryTranslator {
|
|||
if (arg instanceof Literal) {
|
||||
return String.valueOf(((Literal) arg).value());
|
||||
}
|
||||
throw new SqlIllegalArgumentException("Does not know how to convert argument %s for function %s", arg.nodeString(), af.nodeString());
|
||||
throw new SqlIllegalArgumentException("Does not know how to convert argument " + arg.nodeString()
|
||||
+ " for function " + af.nodeString());
|
||||
}
|
||||
|
||||
// TODO: need to optimize on ngram
|
||||
|
|
@ -531,7 +533,8 @@ abstract class QueryTranslator {
|
|||
protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) {
|
||||
Check.isTrue(bc.right().foldable(),
|
||||
"Line %d:%d - Comparisons against variables are not (currently) supported; offender %s in %s",
|
||||
bc.right().location().getLineNumber(), bc.right().location().getColumnNumber(), bc.right().nodeName(), bc.nodeName());
|
||||
bc.right().location().getLineNumber(), bc.right().location().getColumnNumber(),
|
||||
bc.right().nodeName(), bc.nodeName());
|
||||
|
||||
if (bc.left() instanceof NamedExpression) {
|
||||
NamedExpression ne = (NamedExpression) bc.left();
|
||||
|
|
@ -549,7 +552,8 @@ abstract class QueryTranslator {
|
|||
ScriptTemplate scriptTemplate = sfa.script();
|
||||
|
||||
String template = formatTemplate(format(Locale.ROOT, "%s %s {}", scriptTemplate.template(), bc.symbol()));
|
||||
// no need to bind the wrapped/target agg - it is already available through the nested script (needed to create the script itself)
|
||||
// no need to bind the wrapped/target agg - it is already available through the nested script
|
||||
// (needed to create the script itself)
|
||||
Params params = paramsBuilder().script(scriptTemplate.params()).variable(valueOf(bc.right())).build();
|
||||
ScriptTemplate script = new ScriptTemplate(template, params, DataTypes.BOOLEAN);
|
||||
if (onAggs) {
|
||||
|
|
@ -650,7 +654,8 @@ abstract class QueryTranslator {
|
|||
Attribute at = ne.toAttribute();
|
||||
|
||||
// scalar function can appear in both WHERE and HAVING so handle it first
|
||||
// in both cases the function script is used - script-query/query for the former, bucket-selector/aggFilter for the latter
|
||||
// in both cases the function script is used - script-query/query for the former, bucket-selector/aggFilter
|
||||
// for the latter
|
||||
|
||||
if (at instanceof ScalarFunctionAttribute) {
|
||||
ScalarFunctionAttribute sfa = (ScalarFunctionAttribute) at;
|
||||
|
|
@ -662,7 +667,8 @@ abstract class QueryTranslator {
|
|||
scriptTemplate.template(),
|
||||
r.includeUpper() ? "<=" : "<"));
|
||||
|
||||
// no need to bind the wrapped/target - it is already available through the nested script (needed to create the script itself)
|
||||
// no need to bind the wrapped/target - it is already available through the nested script (needed to
|
||||
// create the script itself)
|
||||
Params params = paramsBuilder().variable(lower)
|
||||
.script(scriptTemplate.params())
|
||||
.script(scriptTemplate.params())
|
||||
|
|
@ -701,7 +707,8 @@ abstract class QueryTranslator {
|
|||
.build();
|
||||
|
||||
}
|
||||
aggFilter = new AggFilter(((NamedExpression) r.value()).id().toString(), new ScriptTemplate(template, params, DataTypes.BOOLEAN));
|
||||
aggFilter = new AggFilter(((NamedExpression) r.value()).id().toString(),
|
||||
new ScriptTemplate(template, params, DataTypes.BOOLEAN));
|
||||
}
|
||||
//
|
||||
// WHERE
|
||||
|
|
|
|||
Loading…
Reference in New Issue