SOLR-10157: improve error message in case of unknown aggregations

This commit is contained in:
Munendra S N 2020-03-18 10:46:57 +05:30
parent 6a59d443bc
commit 0b063fd2b7
3 changed files with 52 additions and 11 deletions

View File

@ -76,6 +76,8 @@ Other Changes
* SOLR-14296: Update netty to 4.1.46 (Andras Solaman via Erick Erickson)
* SOLR-10157: Improve error messages when unknown aggregations are specified in the request (hossman, Munendra S N)
================== 8.5.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

View File

@ -424,31 +424,33 @@ public class FunctionQParser extends QParser {
/** @lucene.experimental */
public AggValueSource parseAgg(int flags) throws SyntaxError {
String id = sp.getId();
String origId = sp.getId();
AggValueSource vs = null;
boolean hasParen = false;
boolean hasParen;
if ("agg".equals(id)) {
if ("agg".equals(origId)) {
hasParen = sp.opt("(");
vs = parseAgg(flags | FLAG_IS_AGG);
} else {
// parse as an aggregation...
if (!id.startsWith("agg_")) {
id = "agg_" + id;
}
String id = origId.startsWith("agg_")? origId: "agg_" + origId;
hasParen = sp.opt("(");
ValueSourceParser argParser = req.getCore().getValueSourceParser(id);
if (argParser == null) {
throw new SyntaxError("Unknown aggregation " + id + " in (" + sp + ")");
argParser = req.getCore().getValueSourceParser(origId);
if (argParser == null) {
throw new SyntaxError("Unknown aggregation '" + origId + "' in input (" + sp + ")");
} else {
throw new SyntaxError("Expected multi-doc aggregation from '" + origId +
"' but got per-doc function in input (" + sp + ")");
}
}
ValueSource vv = argParser.parse(this);
if (!(vv instanceof AggValueSource)) {
if (argParser == null) { // why this??
throw new SyntaxError("Expected aggregation from " + id + " but got (" + vv + ") in (" + sp + ")");
}
throw new SyntaxError("Expected multi-doc aggregation from '" + origId +
"' but got (" + vv + ") in (" + sp + ")");
}
vs = (AggValueSource) vv;
}

View File

@ -45,6 +45,8 @@ import org.junit.Test;
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
import com.tdunning.math.stats.AVLTreeDigest;
import static org.hamcrest.core.StringContains.containsString;
// Related tests:
// TestCloudJSONFacetJoinDomain for random field faceting tests with domain modifications
// TestJsonFacetRefinement for refinement tests
@ -3818,6 +3820,41 @@ public class TestJsonFacets extends SolrTestCaseHS {
);
}
@Test
public void testAggErrors() {
ignoreException("aggregation");
SolrException e = expectThrows(SolrException.class, () -> {
h.query(req("q", "*:*", "json.facet", "{bleh:'div(2,4)'}"));
});
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertThat(e.getMessage(),
containsString("Expected multi-doc aggregation from 'div' but got per-doc function in input ('div(2,4)"));
e = expectThrows(SolrException.class, () -> {
h.query(req("q", "*:*", "json.facet", "{b:'agg(div(2,4))'}"));
});
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertThat(e.getMessage(),
containsString("Expected multi-doc aggregation from 'div' but got per-doc function in input ('agg(div(2,4))"));
e = expectThrows(SolrException.class, () -> {
h.query(req("q", "*:*", "json.facet", "{b:'agg(bleh(2,4))'}"));
});
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertThat(e.getMessage(),
containsString("Unknown aggregation 'bleh' in input ('agg(bleh(2,4))"));
e = expectThrows(SolrException.class, () -> {
h.query(req("q", "*:*", "json.facet", "{b:'bleh(2,4)'}"));
});
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertThat(e.getMessage(),
containsString("Unknown aggregation 'bleh' in input ('bleh(2,4)"));
resetExceptionIgnores();
}
public void XtestPercentiles() {
AVLTreeDigest catA = new AVLTreeDigest(100);