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.

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

For empty mappings use a LocalRelation #105081

Merged
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(*)
Comment on lines +86 to +87
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not part of the skip batch? It'd be worth a comment if there's a reason.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this can be determine upfront, which facilitates the rest of the rules and folding of LocalRelation through-out the plan.
Skip is done at the end for cases where a normal relationship is changed due to filtering.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does make the other rules do less work, yes, but since there's a ONCE limiter, it might then have made more sense as a first rule. Anyways, minor.

// 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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With all the similar changes to add a concrete index, wondering if EsIndex(String name, Map<String, EsField> mapping) c'tor, which is only used in tests, shouldn't be updated instead to add a set with the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have mixed feelings about this. I know it's not a big deal, but if something is used only in tests, the method/constructor access modifier should reflect this. But this class is used in many different packages and the only possible access modifier is public. I would rather have the usage be explicit and the user of the constructor be aware of the implications of providing an empty Set or specific values in that Set, rather than use the constructor without being aware of its content.

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