Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
import org.elasticsearch.xpack.esql.index.EsIndex;
import org.elasticsearch.xpack.esql.index.IndexResolution;
import org.elasticsearch.xpack.esql.parser.ParsingException;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Drop;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
Expand Down Expand Up @@ -221,7 +221,7 @@ private LogicalPlan resolveIndex(UnresolvedRelation plan, IndexResolution indexR
plan.commandName()
);
}
TableIdentifier table = plan.table();
IndexPattern table = plan.table();
if (indexResolution.matches(table.index()) == false) {
// TODO: fix this (and tests), or drop check (seems SQL-inherited, where's also defective)
new UnresolvedRelation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@

package org.elasticsearch.xpack.esql.analysis;

import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;

public class TableInfo {
Copy link
Contributor Author

@alex-spies alex-spies Jan 24, 2025

Choose a reason for hiding this comment

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

In a future PR, I think this class just has to go. It is a wrapper around TableIdentifier, except that it throws away the equality/hash methods and uses instance identity for equality.


private final TableIdentifier id;
private final IndexPattern id;

public TableInfo(TableIdentifier id) {
public TableInfo(IndexPattern id) {
this.id = id;
}

public TableIdentifier id() {
public IndexPattern id() {
return id;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.elasticsearch.xpack.esql.expression.Order;
import org.elasticsearch.xpack.esql.expression.UnresolvedNamePattern;
import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Dissect;
import org.elasticsearch.xpack.esql.plan.logical.Drop;
Expand Down Expand Up @@ -255,7 +255,7 @@ public LogicalPlan visitRowCommand(EsqlBaseParser.RowCommandContext ctx) {
@Override
public LogicalPlan visitFromCommand(EsqlBaseParser.FromCommandContext ctx) {
Source source = source(ctx);
TableIdentifier table = new TableIdentifier(source, null, visitIndexPattern(ctx.indexPattern()));
IndexPattern table = new IndexPattern(source, visitIndexPattern(ctx.indexPattern()));
Map<String, Attribute> metadataMap = new LinkedHashMap<>();
if (ctx.metadata() != null) {
for (var c : ctx.metadata().UNQUOTED_SOURCE()) {
Expand Down Expand Up @@ -468,7 +468,7 @@ public LogicalPlan visitMetricsCommand(EsqlBaseParser.MetricsCommandContext ctx)
throw new IllegalArgumentException("METRICS command currently requires a snapshot build");
}
Source source = source(ctx);
TableIdentifier table = new TableIdentifier(source, null, visitIndexPattern(ctx.indexPattern()));
IndexPattern table = new IndexPattern(source, visitIndexPattern(ctx.indexPattern()));

if (ctx.aggregates == null && ctx.grouping == null) {
return new UnresolvedRelation(source, table, false, List.of(), IndexMode.STANDARD, null, "METRICS");
Expand Down Expand Up @@ -530,7 +530,7 @@ public PlanFactory visitJoinCommand(EsqlBaseParser.JoinCommandContext ctx) {

UnresolvedRelation right = new UnresolvedRelation(
source(target),
new TableIdentifier(source(target.index), null, rightPattern),
new IndexPattern(source(target.index), rightPattern),
false,
emptyList(),
IndexMode.LOOKUP,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* 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.plan;

import org.elasticsearch.xpack.esql.core.tree.Source;

import java.util.Objects;

/**
* Contains an index pattern together with its {@link Source}. Can also be a comma-separated list, like {@code idx-*,remote:other-idx*}.
*/
public class IndexPattern {

private final Source source;
private final String indexPattern;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be called indexPattern(s) as it might list coma separated patterns?

Not sure if it is needed often, but #120277 would benefit if it is possible to return list/array of separate patterns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can add a helper method as part of #120277! I'd like to not add logic in this PR, so it remains a straight refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, yeah, indexPatterns (plural) would be even more precise - but then we'd have to align e.g. EsRelation as well. And it's also reasonable to consider idx,logs-* a single pattern which matches idx and anything starting with logs-.


public IndexPattern(Source source, String indexPattern) {
this.source = source;
this.indexPattern = indexPattern;
}

public String index() {
return indexPattern;
}

@Override
public int hashCode() {
return Objects.hash(indexPattern);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}

if (obj == null || getClass() != obj.getClass()) {
return false;
}

IndexPattern other = (IndexPattern) obj;
return Objects.equals(indexPattern, other.indexPattern);
}

public Source source() {
return source;
}

@Override
public String toString() {
return indexPattern;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.elasticsearch.xpack.esql.core.expression.Attribute;
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
import org.elasticsearch.xpack.esql.core.tree.Source;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;

import java.util.Collections;
import java.util.List;
Expand All @@ -22,7 +22,7 @@

public class UnresolvedRelation extends LeafPlan implements Unresolvable {

private final TableIdentifier table;
private final IndexPattern table;
private final boolean frozen;
private final List<Attribute> metadataFields;
/*
Expand All @@ -40,7 +40,7 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable {

public UnresolvedRelation(
Source source,
TableIdentifier table,
IndexPattern table,
boolean frozen,
List<Attribute> metadataFields,
IndexMode indexMode,
Expand Down Expand Up @@ -71,7 +71,7 @@ protected NodeInfo<UnresolvedRelation> info() {
return NodeInfo.create(this, UnresolvedRelation::new, table, frozen, metadataFields, indexMode, unresolvedMsg, commandName);
}

public TableIdentifier table() {
public IndexPattern table() {
return table;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import org.elasticsearch.xpack.esql.optimizer.PhysicalPlanOptimizer;
import org.elasticsearch.xpack.esql.parser.EsqlParser;
import org.elasticsearch.xpack.esql.parser.QueryParams;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
import org.elasticsearch.xpack.esql.plan.logical.Keep;
Expand Down Expand Up @@ -373,7 +373,7 @@ public void analyzedPlan(
}

private void preAnalyzeLookupIndex(TableInfo tableInfo, PreAnalysisResult result, ActionListener<PreAnalysisResult> listener) {
TableIdentifier table = tableInfo.id();
IndexPattern table = tableInfo.id();
Set<String> fieldNames = result.wildcardJoinIndices().contains(table.index()) ? IndexResolver.ALL_FIELDS : result.fieldNames;
// call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types
indexResolver.resolveAsMergedMapping(
Expand All @@ -400,7 +400,7 @@ private void preAnalyzeIndices(
// known to be unavailable from the enrich policy API call
Map<String, Exception> unavailableClusters = result.enrichResolution.getUnavailableClusters();
TableInfo tableInfo = indices.get(0);
TableIdentifier table = tableInfo.id();
IndexPattern table = tableInfo.id();

Map<String, OriginalIndices> clusterIndices = indicesExpressionGrouper.groupIndices(IndicesOptions.DEFAULT, table.index());
for (Map.Entry<String, OriginalIndices> entry : clusterIndices.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import org.elasticsearch.xpack.esql.index.IndexResolution;
import org.elasticsearch.xpack.esql.parser.ParsingException;
import org.elasticsearch.xpack.esql.parser.QueryParams;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
Expand Down Expand Up @@ -98,7 +98,7 @@ public class AnalyzerTests extends ESTestCase {

private static final UnresolvedRelation UNRESOLVED_RELATION = new UnresolvedRelation(
EMPTY,
new TableIdentifier(EMPTY, null, "idx"),
new IndexPattern(EMPTY, "idx"),
false,
List.of(),
IndexMode.STANDARD,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation;

Expand Down Expand Up @@ -72,7 +72,7 @@ static UnresolvedFunction function(String name, List<Expression> args) {
}

static UnresolvedRelation relation(String name) {
return new UnresolvedRelation(EMPTY, new TableIdentifier(EMPTY, null, name), false, List.of(), IndexMode.STANDARD, null, "FROM");
return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, name), false, List.of(), IndexMode.STANDARD, null, "FROM");
}

static Literal integer(int i) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Dissect;
import org.elasticsearch.xpack.esql.plan.logical.Drop;
Expand Down Expand Up @@ -2283,20 +2283,12 @@ public void testInvalidAlias() {
}

private LogicalPlan unresolvedRelation(String index) {
return new UnresolvedRelation(EMPTY, new TableIdentifier(EMPTY, null, index), false, List.of(), IndexMode.STANDARD, null, "FROM");
return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, index), false, List.of(), IndexMode.STANDARD, null, "FROM");
}

private LogicalPlan unresolvedTSRelation(String index) {
List<Attribute> metadata = List.of(new MetadataAttribute(EMPTY, MetadataAttribute.TSID_FIELD, DataType.KEYWORD, false));
return new UnresolvedRelation(
EMPTY,
new TableIdentifier(EMPTY, null, index),
false,
metadata,
IndexMode.TIME_SERIES,
null,
"FROM TS"
);
return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, index), false, metadata, IndexMode.TIME_SERIES, null, "FROM TS");
}

public void testMetricWithGroupKeyAsAgg() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.elasticsearch.xpack.esql.core.type.EsField;
import org.elasticsearch.xpack.esql.index.EsIndex;
import org.elasticsearch.xpack.esql.index.IndexResolution;
import org.elasticsearch.xpack.esql.plan.TableIdentifier;
import org.elasticsearch.xpack.esql.plan.IndexPattern;
import org.elasticsearch.xpack.esql.type.EsFieldTests;

import java.util.ArrayList;
Expand Down Expand Up @@ -702,7 +702,7 @@ public void testCheckForCcsLicense() {
// local only search does not require an enterprise license
{
List<TableInfo> indices = new ArrayList<>();
indices.add(new TableInfo(new TableIdentifier(EMPTY, null, randomFrom("idx", "idx1,idx2*"))));
indices.add(new TableInfo(new IndexPattern(EMPTY, randomFrom("idx", "idx1,idx2*"))));

checkForCcsLicense(executionInfo, indices, indicesGrouper, enterpriseLicenseValid);
checkForCcsLicense(executionInfo, indices, indicesGrouper, platinumLicenseValid);
Expand All @@ -727,10 +727,10 @@ public void testCheckForCcsLicense() {
List<TableInfo> indices = new ArrayList<>();
final String indexExprWithRemotes = randomFrom("remote:idx", "idx1,remote:idx2*,remote:logs,c*:idx4");
if (randomBoolean()) {
indices.add(new TableInfo(new TableIdentifier(EMPTY, null, indexExprWithRemotes)));
indices.add(new TableInfo(new IndexPattern(EMPTY, indexExprWithRemotes)));
} else {
indices.add(new TableInfo(new TableIdentifier(EMPTY, null, randomFrom("idx", "idx1,idx2*"))));
indices.add(new TableInfo(new TableIdentifier(EMPTY, null, indexExprWithRemotes)));
indices.add(new TableInfo(new IndexPattern(EMPTY, randomFrom("idx", "idx1,idx2*"))));
indices.add(new TableInfo(new IndexPattern(EMPTY, indexExprWithRemotes)));
}

// licenses that work
Expand Down