Skip to content

Commit a58d98d

Browse files
committed
improve whitelisting plans
1 parent 96d6c49 commit a58d98d

File tree

1 file changed

+18
-34
lines changed
  • x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/approximate

1 file changed

+18
-34
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/approximate/Approximate.java

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,21 @@
3131
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals;
3232
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
3333
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
34+
import org.elasticsearch.xpack.esql.plan.logical.ChangePoint;
3435
import org.elasticsearch.xpack.esql.plan.logical.Dissect;
3536
import org.elasticsearch.xpack.esql.plan.logical.Drop;
37+
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
3638
import org.elasticsearch.xpack.esql.plan.logical.Eval;
37-
import org.elasticsearch.xpack.esql.plan.logical.Filter;
38-
import org.elasticsearch.xpack.esql.plan.logical.Fork;
3939
import org.elasticsearch.xpack.esql.plan.logical.Grok;
40-
import org.elasticsearch.xpack.esql.plan.logical.InlineStats;
40+
import org.elasticsearch.xpack.esql.plan.logical.Insist;
4141
import org.elasticsearch.xpack.esql.plan.logical.Keep;
4242
import org.elasticsearch.xpack.esql.plan.logical.LeafPlan;
4343
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
4444
import org.elasticsearch.xpack.esql.plan.logical.OrderBy;
4545
import org.elasticsearch.xpack.esql.plan.logical.Project;
4646
import org.elasticsearch.xpack.esql.plan.logical.Rename;
4747
import org.elasticsearch.xpack.esql.plan.logical.Sample;
48-
import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin;
48+
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
4949
import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject;
5050
import org.elasticsearch.xpack.esql.session.Result;
5151

@@ -90,34 +90,25 @@ public interface LogicalPlanRunner {
9090
void run(LogicalPlan plan, ActionListener<Result> listener);
9191
}
9292

93+
9394
/**
94-
* These commands preserve all rows, so can be swapped with {@code SAMPLE}.
95+
* These commands preserve all rows, making it easy to predict the number of output rows.
9596
*/
9697
private static final Set<Class<? extends LogicalPlan>> ROW_PRESERVING_COMMANDS = Set.of(
98+
ChangePoint.class,
9799
Dissect.class,
98100
Drop.class,
101+
Enrich.class,
99102
EsqlProject.class,
100103
Eval.class,
101-
Filter.class,
102-
Fork.class,
103104
Grok.class,
105+
Insist.class,
104106
Keep.class,
105107
OrderBy.class,
106108
Project.class,
107-
Rename.class,
108-
Sample.class
109+
Rename.class
109110
);
110111

111-
/**
112-
* These commands keep or filter rows, so can be swapped with {@code SAMPLE}.
113-
*/
114-
private static final Set<Class<? extends LogicalPlan>> ROW_FILTERING_COMMANDS = Set.of(Filter.class, Sample.class);
115-
116-
/**
117-
* Commands that cannot be used anywhere in an approximated query.
118-
*/
119-
private static final Set<Class<? extends LogicalPlan>> INCOMPATIBLE_COMMANDS = Set.of(InlineStats.class, LookupJoin.class);
120-
121112
// TODO: find a good default value, or alternative ways of setting it
122113
private static final int SAMPLE_ROW_COUNT = 100000;
123114

@@ -154,32 +145,25 @@ private boolean verifyPlan() {
154145
List.of(Failure.fail(logicalPlan.collectLeaves().getFirst(), "query without [STATS] cannot be approximated"))
155146
);
156147
}
148+
// For now, only support unary plans.
149+
// TODO: support binary plans (e.g. join) and n-ary plans (e.g. fork).
157150
logicalPlan.forEachUp(plan -> {
158-
if (INCOMPATIBLE_COMMANDS.contains(plan.getClass())) {
151+
if (plan instanceof LeafPlan == false && plan instanceof UnaryPlan == false) {
159152
throw new VerificationException(
160-
List.of(Failure.fail(plan, "query with [" + plan.sourceText() + "] cannot be approximated"))
153+
List.of(Failure.fail(plan, "query with [" + plan.nodeName() + "] cannot be approximated"))
161154
);
162155
}
163156
});
164157

165158
Holder<Boolean> encounteredStats = new Holder<>(false);
166159
Holder<Boolean> hasFilters = new Holder<>(false);
167160
logicalPlan.transformUp(plan -> {
168-
if (plan instanceof LeafPlan) {
169-
encounteredStats.set(false);
170-
} else if (encounteredStats.get() == false) {
161+
if (encounteredStats.get() == false) {
171162
if (plan instanceof Aggregate) {
172163
encounteredStats.set(true);
173-
} else if (ROW_PRESERVING_COMMANDS.contains(plan.getClass()) == false
174-
&& ROW_FILTERING_COMMANDS.contains(plan.getClass()) == false) {
175-
hasFilters.set(true); // TODO: fix
176-
177-
// throw new VerificationException(
178-
// List.of(Failure.fail(plan, "query with [" + plan.sourceText() + "] before [STATS] cannot be approximated"))
179-
// );
180-
} else if (ROW_FILTERING_COMMANDS.contains(plan.getClass())) {
181-
hasFilters.set(true);
182-
}
164+
} else if (ROW_PRESERVING_COMMANDS.contains(plan.getClass()) == false) {
165+
hasFilters.set(true);
166+
}
183167
}
184168
return plan;
185169
});

0 commit comments

Comments
 (0)