Skip to content

Commit

Permalink
For empty mappings use a LocalRelation (elastic#105081)
Browse files Browse the repository at this point in the history
Fixes elastic#104809 by
converting a plan to a local relation when there is no mapping for the
index pattern.
  • Loading branch information
astefan committed Feb 6, 2024
1 parent 6cf9258 commit bfa21b5
Show file tree
Hide file tree
Showing 11 changed files with 152 additions and 19 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/105081.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 105081
summary: For empty mappings use a `LocalRelation`
area: ES|QL
type: bug
issues:
- 104809
Original file line number Diff line number Diff line change
Expand Up @@ -1139,12 +1139,13 @@ FROM employees
| STATS x = CONCAT(TO_STRING(ROUND(AVG(salary % 3))), TO_STRING(MAX(emp_no))),
y = ROUND((MIN(emp_no / 3) + PI() - MEDIAN(salary))/E())
BY z = languages % 2
| SORT z
;

x:s | y:d | z:i
1.010029 | -16452.0 | null
1.010100 | -15260.0 | 0
1.010097 | -16701.0 | 1
1.010029 | -16452.0 | null
;

nestedAggsOverGroupingWithAlias#[skip:-8.12.99,reason:supported in 8.13]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.Build;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder;
import org.elasticsearch.action.bulk.BulkRequestBuilder;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.index.IndexRequestBuilder;
Expand Down Expand Up @@ -1413,6 +1414,89 @@ public void testCountTextField() {
}
}

public void testQueryOnEmptyMappingIndex() {
createIndex("empty-test", Settings.EMPTY);
createIndex("empty-test2", Settings.EMPTY);
IndicesAliasesRequestBuilder indicesAliasesRequestBuilder = indicesAdmin().prepareAliases()
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("empty-test").alias("alias-test"))
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("empty-test2").alias("alias-test"));
indicesAdmin().aliases(indicesAliasesRequestBuilder.request()).actionGet();

String[] indexPatterns = new String[] { "empty-test", "empty-test,empty-test2", "empty-test*", "alias-test", "*-test*" };
String from = "FROM " + randomFrom(indexPatterns) + " ";

assertEmptyIndexQueries(from);

try (EsqlQueryResponse resp = run(from + "[METADATA _source] | EVAL x = 123")) {
assertFalse(resp.values().hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("_source", "_source"), new ColumnInfo("x", "integer"))));
}

try (EsqlQueryResponse resp = run(from)) {
assertFalse(resp.values().hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("<no-fields>", "null"))));
}
}

public void testQueryOnEmptyDataIndex() {
createIndex("empty_data-test", Settings.EMPTY);
assertAcked(client().admin().indices().prepareCreate("empty_data-test2").setMapping("name", "type=text"));
IndicesAliasesRequestBuilder indicesAliasesRequestBuilder = indicesAdmin().prepareAliases()
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("empty_data-test").alias("alias-empty_data-test"))
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("empty_data-test2").alias("alias-empty_data-test"));
indicesAdmin().aliases(indicesAliasesRequestBuilder.request()).actionGet();

String[] indexPatterns = new String[] {
"empty_data-test2",
"empty_data-test,empty_data-test2",
"alias-empty_data-test",
"*data-test" };
String from = "FROM " + randomFrom(indexPatterns) + " ";

assertEmptyIndexQueries(from);

try (EsqlQueryResponse resp = run(from + "[METADATA _source] | EVAL x = 123")) {
assertFalse(resp.values().hasNext());
assertThat(
resp.columns(),
equalTo(List.of(new ColumnInfo("name", "text"), new ColumnInfo("_source", "_source"), new ColumnInfo("x", "integer")))
);
}

try (EsqlQueryResponse resp = run(from)) {
assertFalse(resp.values().hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("name", "text"))));
}
}

private void assertEmptyIndexQueries(String from) {
try (EsqlQueryResponse resp = run(from + "[METADATA _source] | KEEP _source | LIMIT 1")) {
assertFalse(resp.values().hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("_source", "_source"))));
}

try (EsqlQueryResponse resp = run(from + "| EVAL y = 1 | KEEP y | LIMIT 1 | EVAL x = 1")) {
assertFalse(resp.values().hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("y", "integer"), new ColumnInfo("x", "integer"))));
}

try (EsqlQueryResponse resp = run(from + "| STATS c = count()")) {
assertTrue(resp.values().hasNext());
Iterator<Object> row = resp.values().next();
assertThat(row.next(), equalTo((long) 0));
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("c", "long"))));
}

try (EsqlQueryResponse resp = run(from + "| STATS c = count() | EVAL x = 123")) {
assertTrue(resp.values().hasNext());
Iterator<Object> row = resp.values().next();
assertThat(row.next(), equalTo((long) 0));
assertThat(row.next(), equalTo(123));
assertFalse(row.hasNext());
assertThat(resp.columns(), equalTo(List.of(new ColumnInfo("c", "long"), new ColumnInfo("x", "integer"))));
}
}

private void createNestedMappingIndex(String indexName) throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,10 @@
import static org.elasticsearch.xpack.ql.type.DataTypes.NESTED;

public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerContext> {
static final List<Attribute> NO_FIELDS = List.of(
new ReferenceAttribute(Source.EMPTY, "<no-fields>", DataTypes.NULL, null, Nullability.TRUE, null, false)
// marker list of attributes for plans that do not have any concrete fields to return, but have other computed columns to return
// ie from test | stats c = count(*)
public static final List<Attribute> NO_FIELDS = List.of(
new ReferenceAttribute(Source.EMPTY, "<no-fields>", DataTypes.NULL, null, Nullability.TRUE, null, true)
);
private static final Iterable<RuleExecutor.Batch<LogicalPlan>> rules;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ protected static List<Batch<LogicalPlan>> rules() {
// lastly replace surrogate functions
new SubstituteSurrogates(),
new ReplaceRegexMatch(),
new ReplaceAliasingEvalWithProject()
new ReplaceAliasingEvalWithProject(),
new SkipQueryOnEmptyMappings()
// new NormalizeAggregate(), - waits on https://github.com/elastic/elasticsearch/issues/100634
);

Expand Down Expand Up @@ -704,6 +705,14 @@ protected LogicalPlan rule(UnaryPlan plan) {
}
}

static class SkipQueryOnEmptyMappings extends OptimizerRules.OptimizerRule<EsRelation> {

@Override
protected LogicalPlan rule(EsRelation plan) {
return plan.index().concreteIndices().isEmpty() ? new LocalRelation(plan.source(), plan.output(), LocalSupplier.EMPTY) : plan;
}
}

@SuppressWarnings("removal")
static class PropagateEmptyRelation extends OptimizerRules.OptimizerRule<UnaryPlan> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

import static java.util.Collections.emptyMap;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.L;
Expand Down Expand Up @@ -69,7 +70,7 @@ public static void init() {
parser = new EsqlParser();

mapping = loadMapping("mapping-basic.json");
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);
logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG));

Expand Down Expand Up @@ -321,7 +322,7 @@ public void testSparseDocument() throws Exception {

SearchStats searchStats = statsForExistingField("field000", "field001", "field002", "field003", "field004");

EsIndex index = new EsIndex("large", large);
EsIndex index = new EsIndex("large", large, Set.of("large"));
IndexResolution getIndexResult = IndexResolution.valid(index);
var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static java.util.Arrays.asList;
Expand Down Expand Up @@ -143,7 +144,7 @@ public void init() {

private Analyzer makeAnalyzer(String mappingFileName, EnrichResolution enrichResolution) {
var mapping = loadMapping(mappingFileName);
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);

return new Analyzer(new AnalyzerContext(config, functionRegistry, getIndexResult, enrichResolution), new Verifier(new Metrics()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.xpack.esql.analysis.Analyzer;
import org.elasticsearch.xpack.esql.analysis.AnalyzerContext;
import org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils;
import org.elasticsearch.xpack.esql.analysis.EnrichResolution;
import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.GreaterThan;
import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.GreaterThanOrEqual;
Expand Down Expand Up @@ -85,9 +86,11 @@

import java.util.List;
import java.util.Map;
import java.util.Set;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static java.util.Collections.singletonList;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.L;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_VERIFIER;
Expand All @@ -96,6 +99,7 @@
import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.localSource;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
import static org.elasticsearch.xpack.esql.analysis.Analyzer.NO_FIELDS;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypes.GEO_POINT;
import static org.elasticsearch.xpack.ql.TestUtils.relation;
import static org.elasticsearch.xpack.ql.tree.Source.EMPTY;
Expand Down Expand Up @@ -124,21 +128,17 @@ public class LogicalPlanOptimizerTests extends ESTestCase {
private static Map<String, EsField> mapping;
private static Map<String, EsField> mappingAirports;
private static Analyzer analyzerAirports;
private static EnrichResolution enrichResolution;

@BeforeClass
public static void init() {
parser = new EsqlParser();
logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG));
var enrichResolution = AnalyzerTestUtils.loadEnrichPolicyResolution(
"languages_idx",
"id",
"languages_idx",
"mapping-languages.json"
);
enrichResolution = AnalyzerTestUtils.loadEnrichPolicyResolution("languages_idx", "id", "languages_idx", "mapping-languages.json");

// Most tests used data from the test index, so we load it here, and use it in the plan() function.
mapping = loadMapping("mapping-basic.json");
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);
analyzer = new Analyzer(
new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, enrichResolution),
Expand All @@ -147,7 +147,7 @@ public static void init() {

// Some tests use data from the airports index, so we load it here, and use it in the plan_airports() function.
mappingAirports = loadMapping("mapping-airports.json");
EsIndex airports = new EsIndex("airports", mappingAirports);
EsIndex airports = new EsIndex("airports", mappingAirports, Set.of("airports"));
IndexResolution getIndexResultAirports = IndexResolution.valid(airports);
analyzerAirports = new Analyzer(
new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution),
Expand Down Expand Up @@ -3182,6 +3182,33 @@ public void testStatsWithCanonicalAggregate() throws Exception {
assertThat(Expressions.attribute(fields.get(1)), is(Expressions.attribute(sum_argument)));
}

public void testEmptyMappingIndex() {
EsIndex empty = new EsIndex("empty_test", emptyMap(), emptySet());
IndexResolution getIndexResultAirports = IndexResolution.valid(empty);
var analyzer = new Analyzer(
new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResultAirports, enrichResolution),
TEST_VERIFIER
);

var plan = logicalOptimizer.optimize(analyzer.analyze(parser.createStatement("from empty_test")));
as(plan, LocalRelation.class);
assertThat(plan.output(), equalTo(NO_FIELDS));

plan = logicalOptimizer.optimize(analyzer.analyze(parser.createStatement("from empty_test [metadata _id] | eval x = 1")));
as(plan, LocalRelation.class);
assertThat(Expressions.names(plan.output()), contains("_id", "x"));

plan = logicalOptimizer.optimize(analyzer.analyze(parser.createStatement("from empty_test [metadata _id, _version] | limit 5")));
as(plan, LocalRelation.class);
assertThat(Expressions.names(plan.output()), contains("_id", "_version"));

plan = logicalOptimizer.optimize(
analyzer.analyze(parser.createStatement("from empty_test | eval x = \"abc\" | enrich languages_idx on x"))
);
LocalRelation local = as(plan, LocalRelation.class);
assertThat(Expressions.names(local.output()), contains(NO_FIELDS.get(0).name(), "x", "language_code", "language_name"));
}

private LogicalPlan optimizedPlan(String query) {
return plan(query);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void init() {
mapper = new Mapper(functionRegistry);
// Most tests used data from the test index, so we load it here, and use it in the plan() function.
mapping = loadMapping("mapping-basic.json");
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);
var enrichResolution = setupEnrichResolution();
analyzer = new Analyzer(new AnalyzerContext(config, functionRegistry, getIndexResult, enrichResolution), TEST_VERIFIER);
Expand All @@ -194,7 +194,7 @@ public void init() {

// Some tests use data from the airports index, so we load it here, and use it in the plan_airports() function.
mappingAirports = loadMapping("mapping-airports.json");
EsIndex airports = new EsIndex("airports", mappingAirports);
EsIndex airports = new EsIndex("airports", mappingAirports, Set.of("airports"));
IndexResolution getIndexResultAirports = IndexResolution.valid(airports);
analyzerAirports = new Analyzer(
new AnalyzerContext(config, functionRegistry, getIndexResultAirports, enrichResolution),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.io.UncheckedIOException;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static java.util.Arrays.asList;
import static org.elasticsearch.index.query.QueryBuilders.rangeQuery;
Expand Down Expand Up @@ -72,7 +73,7 @@ public static void init() {
parser = new EsqlParser();

mapping = loadMapping("mapping-basic.json");
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);
logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG));
physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(EsqlTestUtils.TEST_CFG));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_CFG;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_VERIFIER;
Expand Down Expand Up @@ -208,7 +209,7 @@ protected DataNodeRequest mutateInstance(DataNodeRequest in) throws IOException

static LogicalPlan parse(String query) {
Map<String, EsField> mapping = loadMapping("mapping-basic.json");
EsIndex test = new EsIndex("test", mapping);
EsIndex test = new EsIndex("test", mapping, Set.of("test"));
IndexResolution getIndexResult = IndexResolution.valid(test);
var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(TEST_CFG));
var analyzer = new Analyzer(
Expand Down

0 comments on commit bfa21b5

Please sign in to comment.