Skip to content

Commit 134a0d4

Browse files
committed
Fixed PhysicalOptimizerTests not using same config for everything
1 parent b9d66b4 commit 134a0d4

File tree

5 files changed

+46
-14
lines changed

5 files changed

+46
-14
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,10 @@ public boolean profile() {
224224
return profile;
225225
}
226226

227+
public Set<String> activeEsqlFeatures() {
228+
return activeEsqlFeatures;
229+
}
230+
227231
public boolean clusterHasFeature(NodeFeature feature) {
228232
return activeEsqlFeatures.contains(feature.id());
229233
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public PhysicalPlanOptimizerTests(String name, Configuration config) {
217217
@Before
218218
public void init() {
219219
parser = new EsqlParser();
220-
logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(EsqlTestUtils.TEST_CFG));
220+
logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(config));
221221
physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(config));
222222
EsqlFunctionRegistry functionRegistry = new EsqlFunctionRegistry();
223223
mapper = new Mapper();
@@ -6588,7 +6588,7 @@ private PhysicalPlan physicalPlan(String query, TestDataSource dataSource) {
65886588
// System.out.println("Logical\n" + logical);
65896589
var physical = mapper.map(logical);
65906590
// System.out.println(physical);
6591-
assertSerialization(physical);
6591+
assertSerialization(physical, config);
65926592
return physical;
65936593
}
65946594

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/AbstractNodeSerializationTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public abstract class AbstractNodeSerializationTests<T extends Node<? super T>>
3535
* We use a single random config for all serialization because it's pretty
3636
* heavy to build, especially in {@link #testConcurrentSerialization()}.
3737
*/
38-
private Configuration config;
38+
private static Configuration config;
3939

4040
public static Source randomSource() {
4141
int lineNumber = between(0, EXAMPLE_QUERY.length - 1);
@@ -77,7 +77,7 @@ protected boolean alwaysEmptySource() {
7777
return false;
7878
}
7979

80-
public final Configuration configuration() {
80+
public static final Configuration configuration() {
8181
return config;
8282
}
8383

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/AggregateSerializationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public static List<? extends NamedExpression> randomAggregates() {
5252
new Literal(randomSource(), randomFrom("ASC", "DESC"), DataType.KEYWORD)
5353
);
5454
case 4 -> new Values(randomSource(), FieldAttributeTests.createFieldAttribute(1, true));
55-
case 5 -> new Sum(randomSource(), FieldAttributeTests.createFieldAttribute(1, true), EsqlTestUtils.TEST_CFG);
55+
case 5 -> new Sum(randomSource(), FieldAttributeTests.createFieldAttribute(1, true), configuration());
5656
default -> throw new IllegalArgumentException();
5757
};
5858
result.add(new Alias(randomSource(), randomAlphaOfLength(5), agg));

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequestTests.java

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1212
import org.elasticsearch.common.io.stream.Writeable;
1313
import org.elasticsearch.common.settings.Settings;
14+
import org.elasticsearch.features.NodeFeature;
1415
import org.elasticsearch.index.Index;
1516
import org.elasticsearch.index.IndexMode;
1617
import org.elasticsearch.index.query.TermQueryBuilder;
@@ -33,15 +34,16 @@
3334
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
3435
import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan;
3536
import org.elasticsearch.xpack.esql.planner.mapper.Mapper;
37+
import org.elasticsearch.xpack.esql.session.Configuration;
3638

3739
import java.io.IOException;
3840
import java.util.ArrayList;
3941
import java.util.List;
4042
import java.util.Map;
43+
import java.util.stream.Collectors;
4144

4245
import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomConfiguration;
4346
import static org.elasticsearch.xpack.esql.ConfigurationTestUtils.randomTables;
44-
import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_CFG;
4547
import static org.elasticsearch.xpack.esql.EsqlTestUtils.TEST_VERIFIER;
4648
import static org.elasticsearch.xpack.esql.EsqlTestUtils.emptyPolicyResolution;
4749
import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping;
@@ -79,14 +81,15 @@ protected DataNodeRequest createTestInstance() {
7981
| stats x = avg(salary)
8082
""");
8183
List<ShardId> shardIds = randomList(1, 10, () -> new ShardId("index-" + between(1, 10), "n/a", between(1, 10)));
82-
PhysicalPlan physicalPlan = mapAndMaybeOptimize(parse(query));
84+
Configuration configuration = randomLimitedConfiguration(query);
85+
PhysicalPlan physicalPlan = mapAndMaybeOptimize(parse(query, configuration), configuration);
8386
Map<Index, AliasFilter> aliasFilters = Map.of(
8487
new Index("concrete-index", "n/a"),
8588
AliasFilter.of(new TermQueryBuilder("id", "1"), "alias-1")
8689
);
8790
DataNodeRequest request = new DataNodeRequest(
8891
sessionId,
89-
randomConfiguration(query, randomTables()),
92+
configuration,
9093
randomAlphaOfLength(10),
9194
shardIds,
9295
aliasFilters,
@@ -164,7 +167,7 @@ protected DataNodeRequest mutateInstance(DataNodeRequest in) throws IOException
164167
in.clusterAlias(),
165168
in.shardIds(),
166169
in.aliasFilters(),
167-
mapAndMaybeOptimize(parse(newQuery)),
170+
mapAndMaybeOptimize(parse(newQuery, in.configuration()), in.configuration()),
168171
in.indices(),
169172
in.indicesOptions()
170173
);
@@ -260,20 +263,20 @@ protected DataNodeRequest mutateInstance(DataNodeRequest in) throws IOException
260263
};
261264
}
262265

263-
static LogicalPlan parse(String query) {
266+
static LogicalPlan parse(String query, Configuration configuration) {
264267
Map<String, EsField> mapping = loadMapping("mapping-basic.json");
265268
EsIndex test = new EsIndex("test", mapping, Map.of("test", IndexMode.STANDARD));
266269
IndexResolution getIndexResult = IndexResolution.valid(test);
267-
var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(TEST_CFG));
270+
var logicalOptimizer = new LogicalPlanOptimizer(new LogicalOptimizerContext(configuration));
268271
var analyzer = new Analyzer(
269-
new AnalyzerContext(EsqlTestUtils.TEST_CFG, new EsqlFunctionRegistry(), getIndexResult, emptyPolicyResolution()),
272+
new AnalyzerContext(configuration, new EsqlFunctionRegistry(), getIndexResult, emptyPolicyResolution()),
270273
TEST_VERIFIER
271274
);
272275
return logicalOptimizer.optimize(analyzer.analyze(new EsqlParser().createStatement(query)));
273276
}
274277

275-
static PhysicalPlan mapAndMaybeOptimize(LogicalPlan logicalPlan) {
276-
var physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(TEST_CFG));
278+
static PhysicalPlan mapAndMaybeOptimize(LogicalPlan logicalPlan, Configuration configuration) {
279+
var physicalPlanOptimizer = new PhysicalPlanOptimizer(new PhysicalOptimizerContext(configuration));
277280
var mapper = new Mapper();
278281
var physical = mapper.map(logicalPlan);
279282
if (randomBoolean()) {
@@ -286,4 +289,29 @@ static PhysicalPlan mapAndMaybeOptimize(LogicalPlan logicalPlan) {
286289
protected List<String> filteredWarnings() {
287290
return withDefaultLimitWarning(super.filteredWarnings());
288291
}
292+
293+
/**
294+
* Builds a configuration with a fixed limit of 10000 rows and 1000 bytes.
295+
* <p>
296+
* Without this, warnings would be randomized, and hard to filter from the warnings.
297+
* </p>
298+
*/
299+
private Configuration randomLimitedConfiguration(String query) {
300+
Configuration configuration = randomConfiguration(query, randomTables());
301+
302+
return new Configuration(
303+
configuration.zoneId(),
304+
configuration.locale(),
305+
configuration.username(),
306+
configuration.clusterName(),
307+
configuration.pragmas(),
308+
10000,
309+
1000,
310+
configuration.query(),
311+
configuration.profile(),
312+
configuration.tables(),
313+
configuration.getQueryStartTimeNanos(),
314+
configuration.activeEsqlFeatures()
315+
);
316+
}
289317
}

0 commit comments

Comments
 (0)