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

SQL: Enhance message for PERCENTILE[_RANK] with field as 2nd arg (#36933) 

Enhance error message for the case that the 2nd argument of PERCENTILE
and PERCENTILE_RANK is not a foldable, as it doesn't make sense to have
a dynamic value coming from a field.

Fixes: #36903
This commit is contained in:
Marios Trivyzas 2019-01-03 13:55:09 +02:00 committed by GitHub
parent 046f86f274
commit 33137907cf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 40 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/reference/sql/functions/aggs.asciidoc
View File

@ -192,7 +192,7 @@ PERCENTILE(field_name<1>, numeric_exp<2>)
*Input*:
<1> a numeric field
<2> a numeric expression
<2> a numeric expression (must be a constant and not based on a field)
*Output*: `double` numeric value
@ -218,7 +218,7 @@ PERCENTILE_RANK(field_name<1>, numeric_exp<2>)
*Input*:
<1> a numeric field
<2> a numeric expression
<2> a numeric expression (must be a constant and not based on a field)
*Output*: `double` numeric value

13
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.expression.function.aggregate;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Expressions;
import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
@ -41,15 +42,19 @@ public class Percentile extends NumericAggregate implements EnclosedAgg {
@Override
protected TypeResolution resolveType() {
TypeResolution resolution = super.resolveType();
if (TypeResolution.TYPE_RESOLVED.equals(resolution)) {
resolution = Expressions.typeMustBeNumeric(percent(), functionName(), ParamOrdinal.DEFAULT);
if (!percent.foldable()) {
throw new SqlIllegalArgumentException("2nd argument of PERCENTILE must be constant, received [{}]",
Expressions.name(percent));
}
TypeResolution resolution = super.resolveType();
if (resolution.unresolved()) {
return resolution;
}
return Expressions.typeMustBeNumeric(percent, functionName(), ParamOrdinal.DEFAULT);
}
public Expression percent() {
return percent;
}

6
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.expression.function.aggregate;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Expressions;
import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
@ -41,6 +42,11 @@ public class PercentileRank extends AggregateFunction implements EnclosedAgg {
@Override
protected TypeResolution resolveType() {
if (!value.foldable()) {
throw new SqlIllegalArgumentException("2nd argument of PERCENTILE_RANK must be constant, received [{}]",
Expressions.name(value));
}
TypeResolution resolution = super.resolveType();
if (resolution.unresolved()) {
return resolution;

24
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.analysis.analyzer;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.TestUtils;
import org.elasticsearch.xpack.sql.analysis.AnalysisException;
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
@ -26,12 +27,13 @@ import org.elasticsearch.xpack.sql.type.TypesTests;
import java.util.Map;
public class VerifierErrorMessagesTests extends ESTestCase {
private SqlParser parser = new SqlParser();
private IndexResolution indexResolution = IndexResolution.valid(new EsIndex("test",
TypesTests.loadMapping("mapping-multi-field-with-nested.json")));
private String error(String sql) {
Map<String, EsField> mapping = TypesTests.loadMapping("mapping-multi-field-with-nested.json");
EsIndex test = new EsIndex("test", mapping);
return error(IndexResolution.valid(test), sql);
return error(indexResolution, sql);
}
private String error(IndexResolution getIndexResult, String sql) {
@ -504,4 +506,20 @@ public class VerifierErrorMessagesTests extends ESTestCase {
assertEquals("1:47: Cannot use an aggregate [MAX] for grouping",
error("SELECT MAX(date) FROM test GROUP BY HISTOGRAM(MAX(int), 1)"));
}
public void testErrorMessageForPercentileWithSecondArgBasedOnAField() {
Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), indexResolution, new Verifier(new Metrics()));
SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> analyzer.analyze(parser.createStatement(
"SELECT PERCENTILE(int, ABS(int)) FROM test"), true));
assertEquals("2nd argument of PERCENTILE must be constant, received [ABS(int)]",
e.getMessage());
}
public void testErrorMessageForPercentileRankWithSecondArgBasedOnAField() {
Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), indexResolution, new Verifier(new Metrics()));
SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> analyzer.analyze(parser.createStatement(
"SELECT PERCENTILE_RANK(int, ABS(int)) FROM test"), true));
assertEquals("2nd argument of PERCENTILE_RANK must be constant, received [ABS(int)]",
e.getMessage());
}
}