Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK #37803

Merged
merged 2 commits into from
Jan 24, 2019
Merged

SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK #37803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a6acb8
SQL: Improve handling of invalide args for PERCENTILE/PERCENTILE_RANK
matriv Jan 24, 2019
078dd77
Address comment
matriv Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...l/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
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;
Expand All @@ -17,6 +16,7 @@
import java.util.List;

import static java.util.Collections.singletonList;
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;

public class Percentile extends NumericAggregate implements EnclosedAgg {

Expand All @@ -43,8 +43,8 @@ public Percentile replaceChildren(List<Expression> newChildren) {
@Override
protected TypeResolution resolveType() {
if (!percent.foldable()) {
throw new SqlIllegalArgumentException("2nd argument of PERCENTILE must be constant, received [{}]",
Expressions.name(percent));
return new TypeResolution(format(null, "2nd argument of PERCENTILE must be a constant, received [{}]",
Expressions.name(percent)));
}

TypeResolution resolution = super.resolveType();
Expand Down
6 changes: 3 additions & 3 deletions ...c/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
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;
Expand All @@ -17,6 +16,7 @@
import java.util.List;

import static java.util.Collections.singletonList;
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;

public class PercentileRank extends AggregateFunction implements EnclosedAgg {

Expand All @@ -43,8 +43,8 @@ public Expression replaceChildren(List<Expression> newChildren) {
@Override
protected TypeResolution resolveType() {
if (!value.foldable()) {
throw new SqlIllegalArgumentException("2nd argument of PERCENTILE_RANK must be constant, received [{}]",
Expressions.name(value));
return new TypeResolution(format(null, "2nd argument of PERCENTILE_RANK must be a constant, received [{}]",
Expressions.name(value)));
}

TypeResolution resolution = super.resolveType();
Expand Down
15 changes: 4 additions & 11 deletions ...c/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
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;
Expand Down Expand Up @@ -533,19 +532,13 @@ public void testAggsInHistogram() {
}

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());
assertEquals("1:8: 2nd argument of PERCENTILE must be a constant, received [ABS(int)]",
error("SELECT PERCENTILE(int, ABS(int)) FROM test"));
}

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());
assertEquals("1:8: 2nd argument of PERCENTILE_RANK must be a constant, received [ABS(int)]",
error("SELECT PERCENTILE_RANK(int, ABS(int)) FROM test"));
}
}