Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
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
5 changes: 5 additions & 0 deletions docs/changelog/105067.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 105067
summary: "ESQL: Use faster field caps"
area: ES|QL
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.util.Map;
import java.util.Objects;

final class FieldCapabilitiesIndexResponse implements Writeable {
public final class FieldCapabilitiesIndexResponse implements Writeable {
private static final TransportVersion MAPPING_HASH_VERSION = TransportVersions.V_8_2_0;

private final String indexName;
Expand All @@ -34,7 +34,7 @@ final class FieldCapabilitiesIndexResponse implements Writeable {
private final boolean canMatch;
private final transient TransportVersion originVersion;

FieldCapabilitiesIndexResponse(
public FieldCapabilitiesIndexResponse(
String indexName,
@Nullable String indexMappingHash,
Map<String, IndexFieldCapabilities> responseMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public FieldCapabilitiesRequest() {}
* <p>
* Note that when using the high-level REST client, results are always merged (this flag is always considered 'true').
*/
boolean isMergeResults() {
public boolean isMergeResults() {
return mergeResults;
}

Expand All @@ -85,7 +85,7 @@ boolean isMergeResults() {
* <p>
* Note that when using the high-level REST client, results are always merged (this flag is always considered 'true').
*/
void setMergeResults(boolean mergeResults) {
public void setMergeResults(boolean mergeResults) {
this.mergeResults = mergeResults;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public FieldCapabilitiesResponse(String[] indices, Map<String, Map<String, Field
this(indices, responseMap, Collections.emptyList(), Collections.emptyList());
}

FieldCapabilitiesResponse(List<FieldCapabilitiesIndexResponse> indexResponses, List<FieldCapabilitiesFailure> failures) {
public FieldCapabilitiesResponse(List<FieldCapabilitiesIndexResponse> indexResponses, List<FieldCapabilitiesFailure> failures) {
this(Strings.EMPTY_ARRAY, Collections.emptyMap(), indexResponses, failures);
}

Expand Down Expand Up @@ -117,7 +117,7 @@ public List<FieldCapabilitiesFailure> getFailures() {
/**
* Returns the actual per-index field caps responses
*/
List<FieldCapabilitiesIndexResponse> getIndexResponses() {
public List<FieldCapabilitiesIndexResponse> getIndexResponses() {
return indexResponses;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.esql.qa.mixed;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;

import org.elasticsearch.test.TestClustersThreadFilter;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.xpack.esql.qa.rest.FieldExtractorTestCase;
import org.junit.ClassRule;

@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class FieldExtractorIT extends FieldExtractorTestCase {
@ClassRule
public static ElasticsearchCluster cluster = Clusters.mixedVersionCluster();

@Override
protected String getTestRestCluster() {
return cluster.getHttpAddresses();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.esql.qa.multi_node;

import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;

public class Clusters {
public static ElasticsearchCluster testCluster() {
return ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.nodes(2)
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,13 @@
package org.elasticsearch.xpack.esql.qa.multi_node;

import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
import org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase;
import org.elasticsearch.xpack.ql.CsvSpecReader.CsvTestCase;
import org.junit.ClassRule;

public class EsqlSpecIT extends EsqlSpecTestCase {
@ClassRule
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.nodes(2)
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.build();
public static ElasticsearchCluster cluster = Clusters.testCluster();

@Override
protected String getTestRestCluster() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.esql.qa.multi_node;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;

import org.elasticsearch.test.TestClustersThreadFilter;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.xpack.esql.qa.rest.FieldExtractorTestCase;
import org.junit.ClassRule;

@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class FieldExtractorIT extends FieldExtractorTestCase {
@ClassRule
public static ElasticsearchCluster cluster = Clusters.testCluster();

@Override
protected String getTestRestCluster() {
return cluster.getHttpAddresses();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.esql.qa.single_node;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;

import org.elasticsearch.test.TestClustersThreadFilter;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.xpack.esql.qa.rest.FieldExtractorTestCase;
import org.junit.ClassRule;

@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class FieldExtractorIT extends FieldExtractorTestCase {
@ClassRule
public static ElasticsearchCluster cluster = Clusters.testCluster();

@Override
protected String getTestRestCluster() {
return cluster.getHttpAddresses();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ public void testIncompatibleMappingsErrors() throws IOException {
assertException("from test_alias | where _size is not null | limit 1", "Unknown column [_size]");
assertException(
"from test_alias | where message.hash is not null | limit 1",
"Cannot use field [message.hash] due to ambiguities",
"incompatible types: [integer] in [index2], [murmur3] in [index1]"
"Cannot use field [message.hash] with unsupported type [murmur3]"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how this works, but I don't think it's a killer for the approach, but we should decide. This example makes it look like using two field types will give you the ambiguities error. But in other examples the QL index resolution stuff will give you the unsupported error. We could do either one on the ESQL side, but I picked doing the unsupported error because it's the first such error message I found. I'd be happy to switch it, but that'll break other tests which I'll have to coax back to grene.

Copy link
Contributor

Choose a reason for hiding this comment

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

These tests here are new (added with https://github.com/elastic/elasticsearch/pull/105544/files) because we didn't cover this in tests.

In any case, my personal opinion is that your approach is the better approach: tell the user early that there is field that's unsupported, rather than telling "these two fields have mixed incompatible types" and the user then trying to make them compatible (reindexing or whatever) and later to find out that the effort was in vain because the type they chose to make compatible is the wrong type.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm glad you like it!

);
assertException(
"from index1 | where message.hash is not null | limit 1",
Expand Down
Loading