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

SQL: Remove time threshold for rule executor (elastic/x-pack-elasticsearch#2750) 

Refactor RuleExecutor to eliminate time-based thresholds

Original commit: elastic/x-pack-elasticsearch@ba131f8058
This commit is contained in:
Costin Leau 2017-10-27 18:43:13 +03:00 committed by GitHub
parent 3d0f57d976
commit 6b1d0d1c8e
5 changed files with 57 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -650,6 +650,11 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
return plan.transformExpressionsUp(e -> {
if (e instanceof UnresolvedFunction) {
UnresolvedFunction uf = (UnresolvedFunction) e;
if (uf.analyzed()) {
return uf;
}
String name = uf.name();
if (hasStar(uf.arguments())) {
@ -690,12 +695,8 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
}
List<String> matches = StringUtils.findSimilar(normalizedName, names);
if (!matches.isEmpty()) {
return new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), uf.children(), UnresolvedFunction.errorMessage(normalizedName, matches));
}
else {
return uf;
}
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..
Function f = functionRegistry.resolveFunction(uf, SqlSession.CURRENT_SETTINGS.get());

15
sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java
View File

@ -20,18 +20,22 @@ public class UnresolvedFunction extends Function implements Unresolvable {
private final String name;
private final boolean distinct;
private final String unresolvedMsg;
// flag to indicate analysis has been applied and there's no point in doing it again
// this is an optimization to prevent searching for a better unresolved message over and over again
private final boolean analyzed;
public UnresolvedFunction(Location location, String name, boolean distinct, List<Expression> children) {
this(location, name, distinct, children, null);
this(location, name, distinct, children, false, null);
}
/**
* Constructor used for specifying a more descriptive message (typically 'did you mean') instead of the default one.
*/
public UnresolvedFunction(Location location, String name, boolean distinct, List<Expression> children, String unresolvedMessage) {
public UnresolvedFunction(Location location, String name, boolean distinct, List<Expression> children, boolean analyzed, String unresolvedMessage) {
super(location, children);
this.name = name;
this.distinct = distinct;
this.analyzed = analyzed;
this.unresolvedMsg = unresolvedMessage == null ? errorMessage(name, null) : unresolvedMessage;
}
@ -54,6 +58,10 @@ public class UnresolvedFunction extends Function implements Unresolvable {
return distinct;
}
public boolean analyzed() {
return analyzed;
}
@Override
public DataType dataType() {
throw new UnresolvedException("dataType", this);
@ -82,7 +90,8 @@ public class UnresolvedFunction extends Function implements Unresolvable {
public static String errorMessage(String name, List<String> potentialMatches) {
String msg = "Unknown function [" + name + "]";
if (!CollectionUtils.isEmpty(potentialMatches)) {
msg += ", did you mean " + (potentialMatches.size() == 1 ? "[" + potentialMatches.get(0) + "]": "any of " + potentialMatches.toString()) + "?";
msg += ", did you mean "
+ (potentialMatches.size() == 1 ? "[" + potentialMatches.get(0) + "]" : "any of " + potentialMatches.toString()) + "?";
}
return msg;
}

15
sql/server/src/main/java/org/elasticsearch/xpack/sql/rule/RuleExecutionException.java Normal file
View File

@ -0,0 +1,15 @@
/*
* 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.rule;
import org.elasticsearch.xpack.sql.ServerSqlException;
public class RuleExecutionException extends ServerSqlException {
public RuleExecutionException(String message, Object... args) {
super(message, args);
}
}

40
sql/server/src/main/java/org/elasticsearch/xpack/sql/rule/RuleExecutor.java
View File

@ -5,38 +5,42 @@
*/
package org.elasticsearch.xpack.sql.rule;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.xpack.sql.tree.Node;
import org.elasticsearch.xpack.sql.tree.NodeUtils;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
public abstract class RuleExecutor<TreeType extends Node<TreeType>> {
private final Logger log = Loggers.getLogger(getClass());
public static class Limiter {
public static final Limiter DEFAULT = new Limiter(100, 5, TimeUnit.SECONDS);
public static final Limiter ONCE = new Limiter(1, 5, TimeUnit.SECONDS);
public static final Limiter DEFAULT = new Limiter(100);
public static final Limiter ONCE = new Limiter(1) {
@Override
boolean reached(int runs) {
return runs >= 1;
}
};
private final int runs;
private final long duration;
public Limiter(int runs, long duration, TimeUnit timeUnit) {
this.runs = runs;
this.duration = timeUnit.toNanos(duration);
public Limiter(int maximumRuns) {
this.runs = maximumRuns;
}
private boolean reached(int runs, long duration) {
// NOCOMMIT we should probably be throwing exceptions rather than stopping
// NOCOMMIT including time here seems like it'd cause very unpredictable behavior
return runs >= this.runs || duration >= this.duration;
boolean reached(int runs) {
if (runs >= this.runs) {
throw new RuleExecutionException("Rule execution limit %d reached", runs);
}
return false;
}
}
@ -160,12 +164,12 @@ public abstract class RuleExecutor<TreeType extends Node<TreeType>> {
}
else {
if (log.isTraceEnabled()) {
log.trace("Rule {} applied w/o changes\n", rule);
log.trace("Rule {} applied w/o changes", rule);
}
}
}
batchDuration = System.currentTimeMillis() - batchStart;
} while (hasChanged && !batch.limit.reached(batchRuns, batchDuration));
} while (hasChanged && !batch.limit.reached(batchRuns));
totalDuration += batchDuration;

2
sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
View File

@ -71,7 +71,7 @@ public class VerifierErrorMessagesTests extends ESTestCase {
assertEquals("1:8: Unknown function [ZAZ]", verify("SELECT ZAZ(bool) FROM test"));
}
public void testMispelledFunction() {
public void testMisspelledFunction() {
assertEquals("1:8: Unknown function [COONT], did you mean [COUNT]?", verify("SELECT COONT(bool) FROM test"));
}