Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -202,7 +202,9 @@ private static class ResolveTable extends ParameterizedAnalyzerRule<UnresolvedRe
protected LogicalPlan rule(UnresolvedRelation plan, AnalyzerContext context) {
return resolveIndex(
plan,
plan.indexMode().equals(IndexMode.LOOKUP) ? context.lookupResolution().get(plan.table().index()) : context.indexResolution()
plan.indexMode().equals(IndexMode.LOOKUP)
? context.lookupResolution().get(plan.indexPattern().indexPattern())
: context.indexResolution()
);
}

Expand All @@ -213,20 +215,20 @@ private LogicalPlan resolveIndex(UnresolvedRelation plan, IndexResolution indexR
? plan
: new UnresolvedRelation(
plan.source(),
plan.table(),
plan.indexPattern(),
plan.frozen(),
plan.metadataFields(),
plan.indexMode(),
indexResolutionMessage,
plan.commandName()
);
}
TableIdentifier table = plan.table();
if (indexResolution.matches(table.index()) == false) {
IndexPattern table = plan.indexPattern();
if (indexResolution.matches(table.indexPattern()) == false) {
// TODO: fix this (and tests), or drop check (seems SQL-inherited, where's also defective)
new UnresolvedRelation(
plan.source(),
plan.table(),
plan.indexPattern(),
plan.frozen(),
plan.metadataFields(),
plan.indexMode(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ protected PreAnalysis doPreAnalyze(LogicalPlan plan) {

plan.forEachUp(UnresolvedRelation.class, p -> {
List<TableInfo> list = p.indexMode() == IndexMode.LOOKUP ? lookupIndices : indices;
list.add(new TableInfo(p.table()));
list.add(new TableInfo(p.indexPattern()));
});
plan.forEachUp(Enrich.class, unresolvedEnriches::add);

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 indexPattern() {
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 indexPattern;
private final boolean frozen;
private final List<Attribute> metadataFields;
/*
Expand All @@ -40,19 +40,19 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable {

public UnresolvedRelation(
Source source,
TableIdentifier table,
IndexPattern indexPattern,
boolean frozen,
List<Attribute> metadataFields,
IndexMode indexMode,
String unresolvedMessage,
String commandName
) {
super(source);
this.table = table;
this.indexPattern = indexPattern;
this.frozen = frozen;
this.metadataFields = metadataFields;
this.indexMode = indexMode;
this.unresolvedMsg = unresolvedMessage == null ? "Unknown index [" + table.index() + "]" : unresolvedMessage;
this.unresolvedMsg = unresolvedMessage == null ? "Unknown index [" + indexPattern.indexPattern() + "]" : unresolvedMessage;
this.commandName = commandName;
}

Expand All @@ -68,11 +68,11 @@ public String getWriteableName() {

@Override
protected NodeInfo<UnresolvedRelation> info() {
return NodeInfo.create(this, UnresolvedRelation::new, table, frozen, metadataFields, indexMode, unresolvedMsg, commandName);
return NodeInfo.create(this, UnresolvedRelation::new, indexPattern, frozen, metadataFields, indexMode, unresolvedMsg, commandName);
}

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

public boolean frozen() {
Expand Down Expand Up @@ -124,7 +124,7 @@ public String unresolvedMessage() {

@Override
public int hashCode() {
return Objects.hash(source(), table, metadataFields, indexMode, unresolvedMsg);
return Objects.hash(source(), indexPattern, metadataFields, indexMode, unresolvedMsg);
}

@Override
Expand All @@ -138,7 +138,7 @@ public boolean equals(Object obj) {
}

UnresolvedRelation other = (UnresolvedRelation) obj;
return Objects.equals(table, other.table)
return Objects.equals(indexPattern, other.indexPattern)
&& Objects.equals(frozen, other.frozen)
&& Objects.equals(metadataFields, other.metadataFields)
&& indexMode == other.indexMode
Expand All @@ -147,11 +147,11 @@ public boolean equals(Object obj) {

@Override
public List<Object> nodeProperties() {
return singletonList(table);
return singletonList(indexPattern);
}

@Override
public String toString() {
return UNRESOLVED_PREFIX + table.index();
return UNRESOLVED_PREFIX + indexPattern.indexPattern();
}
}
Loading