From 2347a2a7fcef5348a1ee4ee2d9d62802206b9a47 Mon Sep 17 00:00:00 2001 From: Chloe Date: Thu, 4 Feb 2021 16:12:52 -0600 Subject: [PATCH] Refine PPL head command syntax (#1022) * removed head operator * removed keywords * update * update --- .../sql/analysis/Analyzer.java | 15 +- .../sql/ast/dsl/AstDSL.java | 14 +- .../sql/ast/tree/Head.java | 2 +- .../sql/executor/Explain.java | 10 -- .../sql/planner/DefaultImplementor.java | 11 -- .../sql/planner/logical/LogicalHead.java | 54 ------- .../sql/planner/logical/LogicalPlanDSL.java | 5 - .../logical/LogicalPlanNodeVisitor.java | 4 - .../sql/planner/physical/HeadOperator.java | 114 ------------- .../sql/planner/physical/PhysicalPlanDSL.java | 5 - .../physical/PhysicalPlanNodeVisitor.java | 4 - .../sql/analysis/AnalyzerTest.java | 11 +- .../sql/executor/ExplainTest.java | 23 --- .../sql/planner/DefaultImplementorTest.java | 80 ++++----- .../logical/LogicalPlanNodeVisitorTest.java | 26 +-- .../planner/physical/HeadOperatorTest.java | 152 ------------------ .../physical/PhysicalPlanNodeVisitorTest.java | 41 ++--- docs/experiment/ppl/cmd/head.rst | 41 +---- .../ElasticsearchExecutionProtector.java | 15 -- .../ElasticsearchExecutionProtectorTest.java | 33 ++-- .../sql/ppl/HeadCommandIT.java | 27 ---- ppl/src/main/antlr/OpenDistroPPLLexer.g4 | 2 - ppl/src/main/antlr/OpenDistroPPLParser.g4 | 2 - .../sql/ppl/parser/AstBuilder.java | 5 +- .../sql/ppl/utils/ArgumentFactory.java | 25 --- .../sql/ppl/utils/PPLQueryDataAnonymizer.java | 14 +- .../sql/ppl/parser/AstBuilderTest.java | 42 +---- .../ppl/utils/PPLQueryDataAnonymizerTest.java | 8 +- 28 files changed, 79 insertions(+), 706 deletions(-) delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalHead.java delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperator.java delete mode 100644 core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperatorTest.java diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java index b8e418a43a..8b1c54f349 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/Analyzer.java @@ -29,7 +29,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Map; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedArgument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Aggregation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Dedupe; @@ -59,7 +58,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalDedupe; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalEval; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalHead; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalLimit; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; @@ -358,20 +356,11 @@ public LogicalPlan visitDedupe(Dedupe node, AnalysisContext context) { } /** - * Build {@link LogicalHead}. + * Logical head is identical to {@link LogicalLimit}. */ public LogicalPlan visitHead(Head node, AnalysisContext context) { LogicalPlan child = node.getChild().get(0).accept(this, context); - List options = node.getOptions(); - Boolean keeplast = (Boolean) getOptionAsLiteral(options, 0).getValue(); - Expression whileExpr = expressionAnalyzer.analyze(options.get(1).getValue(), context); - Integer number = (Integer) getOptionAsLiteral(options, 2).getValue(); - - return new LogicalHead(child, keeplast, whileExpr, number); - } - - private static Literal getOptionAsLiteral(List options, int optionIdx) { - return (Literal) options.get(optionIdx).getValue(); + return new LogicalLimit(child, node.getSize(), 0); } @Override diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java index 502b2ffc4d..a1515a7479 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java @@ -365,18 +365,8 @@ public static Dedupe dedupe(UnresolvedPlan input, List options, Field. return new Dedupe(input, options, Arrays.asList(fields)); } - public static Head head(UnresolvedPlan input, List options) { - return new Head(input, options); - } - - /** - * Default Head Command Args. - */ - public static List defaultHeadArgs() { - return unresolvedArgList( - unresolvedArg("keeplast", booleanLiteral(true)), - unresolvedArg("whileExpr", booleanLiteral(true)), - unresolvedArg("number", intLiteral(10))); + public static Head head(UnresolvedPlan input, Integer size) { + return new Head(input, size); } public static List defaultTopArgs() { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Head.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Head.java index 89a5b36e67..83a22d3808 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Head.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Head.java @@ -38,7 +38,7 @@ public class Head extends UnresolvedPlan { private UnresolvedPlan child; - private final List options; + private final Integer size; @Override public Head attach(UnresolvedPlan child) { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/Explain.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/Explain.java index 5e026967c3..c49d337cf0 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/Explain.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/Explain.java @@ -24,7 +24,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.DedupeOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.EvalOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.HeadOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.LimitOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanNodeVisitor; @@ -141,15 +140,6 @@ public ExplainResponseNode visitRareTopN(RareTopNOperator node, Object context) ))); } - @Override - public ExplainResponseNode visitHead(HeadOperator node, Object context) { - return explain(node, context, explainNode -> explainNode.setDescription(ImmutableMap.of( - "keepLast", node.getKeepLast(), - "whileExpr", node.getWhileExpr().toString(), - "number", node.getNumber() - ))); - } - @Override public ExplainResponseNode visitValues(ValuesOperator node, Object context) { return explain(node, context, explainNode -> explainNode.setDescription(ImmutableMap.of( diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementor.java index aac9979ef7..f1db614813 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementor.java @@ -20,7 +20,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalDedupe; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalEval; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalHead; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalLimit; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanNodeVisitor; @@ -36,7 +35,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.DedupeOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.EvalOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.HeadOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.LimitOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; @@ -80,15 +78,6 @@ public PhysicalPlan visitDedupe(LogicalDedupe node, C context) { node.getConsecutive()); } - @Override - public PhysicalPlan visitHead(LogicalHead node, C context) { - return new HeadOperator( - visitChild(node, context), - node.getKeeplast(), - node.getWhileExpr(), - node.getNumber()); - } - @Override public PhysicalPlan visitProject(LogicalProject node, C context) { return new ProjectOperator(visitChild(node, context), node.getProjectList()); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalHead.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalHead.java deleted file mode 100644 index 51c614bbb5..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalHead.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package com.amazon.opendistroforelasticsearch.sql.planner.logical; - -import com.amazon.opendistroforelasticsearch.sql.expression.Expression; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import lombok.ToString; - - -@Getter -@ToString -@EqualsAndHashCode(callSuper = true) -public class LogicalHead extends LogicalPlan { - - private final Boolean keeplast; - private final Expression whileExpr; - private final Integer number; - - /** - * Constructor of LogicalHead. - */ - public LogicalHead( - LogicalPlan child, Boolean keeplast, - Expression whileExpr, Integer number) { - super(Collections.singletonList(child)); - this.keeplast = keeplast; - this.whileExpr = whileExpr; - this.number = number; - } - - - @Override - public R accept(LogicalPlanNodeVisitor visitor, C context) { - return visitor.visitHead(this, context); - } -} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanDSL.java index f3be1955b8..36abe5838f 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanDSL.java @@ -90,11 +90,6 @@ public static LogicalPlan dedupe( return new LogicalDedupe( input, Arrays.asList(fields), allowedDuplication, keepEmpty, consecutive); } - - public static LogicalPlan head( - LogicalPlan input, boolean keeplast, Expression whileExpr, int number) { - return new LogicalHead(input, keeplast, whileExpr, number); - } public static LogicalPlan rareTopN(LogicalPlan input, CommandType commandType, List groupByList, Expression... fields) { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitor.java index 6269c996c7..ba8e48fbf8 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitor.java @@ -43,10 +43,6 @@ public R visitDedupe(LogicalDedupe plan, C context) { return visitNode(plan, context); } - public R visitHead(LogicalHead plan, C context) { - return visitNode(plan, context); - } - public R visitRename(LogicalRename plan, C context) { return visitNode(plan, context); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperator.java deleted file mode 100644 index 48e35eacab..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperator.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package com.amazon.opendistroforelasticsearch.sql.planner.physical; - -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.expression.Expression; -import com.amazon.opendistroforelasticsearch.sql.expression.LiteralExpression; -import com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate.BinaryPredicateOperator; -import java.util.Collections; -import java.util.List; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.NonNull; - -/** - * The Head operator returns the first {@link HeadOperator#number} number of results until the - * {@link HeadOperator#whileExpr} evaluates to true. If {@link HeadOperator#keepLast} is true then - * first result which evalutes {@link HeadOperator#whileExpr} to false is also returned. - * The NULL and MISSING are handled by the logic defined in {@link BinaryPredicateOperator}. - */ -@Getter -@EqualsAndHashCode -public class HeadOperator extends PhysicalPlan { - - @Getter - private final PhysicalPlan input; - @Getter - private final Boolean keepLast; - @Getter - private final Expression whileExpr; - @Getter - private final Integer number; - - private static final Integer DEFAULT_LIMIT = 10; - private static final Boolean IGNORE_LAST = false; - - @EqualsAndHashCode.Exclude - private int recordCount = 0; - @EqualsAndHashCode.Exclude - private boolean foundFirstFalse = false; - @EqualsAndHashCode.Exclude - private ExprValue next; - - @NonNull - public HeadOperator(PhysicalPlan input) { - this(input, IGNORE_LAST, new LiteralExpression(ExprBooleanValue.of(true)), DEFAULT_LIMIT); - } - - /** - * HeadOperator Constructor. - * - * @param input Input {@link PhysicalPlan} - * @param keepLast Controls whether the last result in the result set is retained. The last - * result returned is the result that caused the whileExpr to evaluate to false - * or NULL. - * @param whileExpr The search returns results until this expression evaluates to false - * @param number Number of specified results - */ - @NonNull - public HeadOperator(PhysicalPlan input, Boolean keepLast, Expression whileExpr, Integer number) { - this.input = input; - this.keepLast = keepLast; - this.whileExpr = whileExpr; - this.number = number; - } - - @Override - public R accept(PhysicalPlanNodeVisitor visitor, C context) { - return visitor.visitHead(this, context); - } - - @Override - public List getChild() { - return Collections.singletonList(input); - } - - @Override - public boolean hasNext() { - if (!input.hasNext() || foundFirstFalse || (recordCount >= number)) { - return false; - } - ExprValue inputVal = input.next(); - ExprValue exprValue = whileExpr.valueOf(inputVal.bindingTuples()); - if (exprValue.isNull() || exprValue.isMissing() || !(exprValue.booleanValue())) { - // First false is when we decide whether to keep the last value - foundFirstFalse = true; - if (!keepLast) { - return false; - } - } - this.next = inputVal; - recordCount++; - return true; - } - - @Override - public ExprValue next() { - return this.next; - } -} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanDSL.java index f0a0a5be8b..e7067a60e6 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanDSL.java @@ -88,11 +88,6 @@ public WindowOperator window(PhysicalPlan input, return new WindowOperator(input, windowFunction, windowDefinition); } - public static HeadOperator head(PhysicalPlan input, boolean keepLast, Expression whileExpr, - int number) { - return new HeadOperator(input, keepLast, whileExpr, number); - } - public static RareTopNOperator rareTopN(PhysicalPlan input, CommandType commandType, List groups, Expression... expressions) { return new RareTopNOperator(input, commandType, Arrays.asList(expressions), groups); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitor.java index 00f701bc08..ee74c90623 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitor.java @@ -72,10 +72,6 @@ public R visitValues(ValuesOperator node, C context) { public R visitSort(SortOperator node, C context) { return visitNode(node, context); } - - public R visitHead(HeadOperator node, C context) { - return visitNode(node, context); - } public R visitRareTopN(RareTopNOperator node, C context) { return visitNode(node, context); diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/AnalyzerTest.java index 4c97c83bdd..db07fab0f9 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/AnalyzerTest.java @@ -80,15 +80,8 @@ public void filter_relation() { @Test public void head_relation() { assertAnalyzeEqual( - LogicalPlanDSL.head( - LogicalPlanDSL.relation("schema"), - false, dsl.equal(DSL.ref("integer_value", INTEGER), DSL.literal(integerValue(1))), 10), - AstDSL.head( - AstDSL.relation("schema"), - unresolvedArgList( - unresolvedArg("keeplast", booleanLiteral(false)), - unresolvedArg("whileExpr", compare("=", field("integer_value"), intLiteral(1))), - unresolvedArg("number", intLiteral(10))))); + LogicalPlanDSL.limit(LogicalPlanDSL.relation("schema"),10, 0), + AstDSL.head(AstDSL.relation("schema"), 10)); } @Test diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/executor/ExplainTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/executor/ExplainTest.java index 3314feae18..b5eb55801c 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/executor/ExplainTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/executor/ExplainTest.java @@ -28,7 +28,6 @@ import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.dedupe; import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.eval; import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.filter; -import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.head; import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.limit; import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.project; import static com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlanDSL.rareTopN; @@ -140,28 +139,6 @@ void can_explain_rare_top_n() { explain.apply(plan)); } - @Test - void can_explain_head() { - Boolean keepLast = false; - Expression whileExpr = dsl.and( - dsl.equal(ref("balance", INTEGER), literal(10000)), - dsl.greater(ref("age", INTEGER), literal(30))); - Integer number = 5; - - PhysicalPlan plan = head(tableScan, keepLast, whileExpr, number); - - assertEquals( - new ExplainResponse( - new ExplainResponseNode( - "HeadOperator", - ImmutableMap.of( - "keepLast", false, - "whileExpr", "and(=(balance, 10000), >(age, 30))", - "number", 5), - singletonList(tableScan.explainNode()))), - explain.apply(plan)); - } - @Test void can_explain_window() { List partitionByList = ImmutableList.of(DSL.ref("state", STRING)); diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementorTest.java index 04ee791325..2dd82b40c2 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/DefaultImplementorTest.java @@ -24,7 +24,6 @@ import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.aggregation; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.eval; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.filter; -import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.head; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.limit; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.project; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.rareTopN; @@ -99,9 +98,6 @@ public void visitShouldReturnDefaultPhysicalOperator() { ImmutablePair.of(ref("name1", STRING), ref("name", STRING)); Pair sortField = ImmutablePair.of(Sort.SortOption.DEFAULT_ASC, ref("name1", STRING)); - Boolean keeplast = true; - Expression whileExpr = literal(ExprBooleanValue.of(true)); - Integer number = 5; Integer limit = 1; Integer offset = 1; @@ -109,26 +105,22 @@ public void visitShouldReturnDefaultPhysicalOperator() { project( limit( LogicalPlanDSL.dedupe( - head( - rareTopN( - sort( - eval( - remove( - rename( - aggregation( - filter(values(emptyList()), filterExpr), - aggregators, - groupByExprs), - mappings), - exclude), - newEvalField), - sortField), - CommandType.TOP, - topByExprs, - rareTopNField), - keeplast, - whileExpr, - number), + rareTopN( + sort( + eval( + remove( + rename( + aggregation( + filter(values(emptyList()), filterExpr), + aggregators, + groupByExprs), + mappings), + exclude), + newEvalField), + sortField), + CommandType.TOP, + topByExprs, + rareTopNField), dedupeField), limit, offset), @@ -140,28 +132,24 @@ public void visitShouldReturnDefaultPhysicalOperator() { PhysicalPlanDSL.project( PhysicalPlanDSL.limit( PhysicalPlanDSL.dedupe( - PhysicalPlanDSL.head( - PhysicalPlanDSL.rareTopN( - PhysicalPlanDSL.sort( - PhysicalPlanDSL.eval( - PhysicalPlanDSL.remove( - PhysicalPlanDSL.rename( - PhysicalPlanDSL.agg( - PhysicalPlanDSL.filter( - PhysicalPlanDSL.values(emptyList()), - filterExpr), - aggregators, - groupByExprs), - mappings), - exclude), - newEvalField), - sortField), - CommandType.TOP, - topByExprs, - rareTopNField), - keeplast, - whileExpr, - number), + PhysicalPlanDSL.rareTopN( + PhysicalPlanDSL.sort( + PhysicalPlanDSL.eval( + PhysicalPlanDSL.remove( + PhysicalPlanDSL.rename( + PhysicalPlanDSL.agg( + PhysicalPlanDSL.filter( + PhysicalPlanDSL.values(emptyList()), + filterExpr), + aggregators, + groupByExprs), + mappings), + exclude), + newEvalField), + sortField), + CommandType.TOP, + topByExprs, + rareTopNField), dedupeField), limit, offset), diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitorTest.java index 9b9863a5cc..a5844e984a 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalPlanNodeVisitorTest.java @@ -53,19 +53,17 @@ public void logicalPlanShouldTraversable() { LogicalPlan logicalPlan = LogicalPlanDSL.rename( LogicalPlanDSL.aggregation( - LogicalPlanDSL.head( - LogicalPlanDSL.rareTopN( - LogicalPlanDSL.filter(LogicalPlanDSL.relation("schema"), expression), - CommandType.TOP, - ImmutableList.of(expression), - expression), - false, expression, 10), + LogicalPlanDSL.rareTopN( + LogicalPlanDSL.filter(LogicalPlanDSL.relation("schema"), expression), + CommandType.TOP, + ImmutableList.of(expression), + expression), ImmutableList.of(DSL.named("avg", aggregator)), ImmutableList.of(DSL.named("group", expression))), ImmutableMap.of(ref, ref)); Integer result = logicalPlan.accept(new NodesCount(), null); - assertEquals(6, result); + assertEquals(5, result); } @Test @@ -78,10 +76,6 @@ public void testAbstractPlanNodeVisitorShouldReturnNull() { assertNull(filter.accept(new LogicalPlanNodeVisitor() { }, null)); - LogicalPlan head = LogicalPlanDSL.head(relation, false, expression, 10); - assertNull(head.accept(new LogicalPlanNodeVisitor() { - }, null)); - LogicalPlan aggregation = LogicalPlanDSL.aggregation( filter, ImmutableList.of(DSL.named("avg", aggregator)), ImmutableList.of(DSL.named( @@ -139,14 +133,6 @@ public Integer visitFilter(LogicalFilter plan, Object context) { .collect(Collectors.summingInt(Integer::intValue)); } - @Override - public Integer visitHead(LogicalHead plan, Object context) { - return 1 - + plan.getChild().stream() - .map(child -> child.accept(this, context)) - .collect(Collectors.summingInt(Integer::intValue)); - } - @Override public Integer visitAggregation(LogicalAggregation plan, Object context) { return 1 diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperatorTest.java deleted file mode 100644 index a510529cea..0000000000 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/HeadOperatorTest.java +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package com.amazon.opendistroforelasticsearch.sql.planner.physical; - -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; -import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.when; - -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; -import com.amazon.opendistroforelasticsearch.sql.expression.DSL; -import com.google.common.collect.ImmutableMap; -import java.util.LinkedHashMap; -import java.util.List; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -@ExtendWith(MockitoExtension.class) -class HeadOperatorTest extends PhysicalPlanTestBase { - - @Mock - private PhysicalPlan inputPlan; - - private final int defaultResultCount = 10; - - @Test - public void headTest() { - HeadOperator plan = new HeadOperator(new CountTestScan()); - List result = execute(plan); - assertEquals(defaultResultCount, result.size()); - assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("id", 1, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 2, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 3, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 4, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 5, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 6, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 7, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 8, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 9, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 10, "testString", "asdf")) - )); - } - - @Test - public void headTest_Number() { - HeadOperator plan = new HeadOperator(new CountTestScan(), - false, DSL.literal(true), 2); - List result = execute(plan); - assertEquals(2, result.size()); - assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("id", 1, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 2, "testString", "asdf")) - )); - } - - @Test - public void headTest_InputEnd() { - HeadOperator plan = new HeadOperator(new CountTestScan(), - false, DSL.literal(true), 12); - List result = execute(plan); - assertEquals(11, result.size()); - assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("id", 1, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 2, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 3, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 4, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 5, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 6, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 7, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 8, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 9, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 10, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 11, "testString", "asdf")) - )); - } - - @Test - public void headTest_keepLastTrue() { - HeadOperator plan = new HeadOperator(new CountTestScan(), - true, dsl.less(DSL.ref("id", INTEGER), DSL.literal(5)), defaultResultCount); - List result = execute(plan); - assertEquals(5, result.size()); - assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("id", 1, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 2, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 3, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 4, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 5, "testString", "asdf")) - )); - } - - @Test - public void headTest_keepLastFalse() { - HeadOperator plan = new HeadOperator(new CountTestScan(), - false, dsl.less(DSL.ref("id", INTEGER), DSL.literal(5)), defaultResultCount); - List result = execute(plan); - assertEquals(4, result.size()); - assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("id", 1, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 2, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 3, "testString", "asdf")), - ExprValueUtils.tupleValue(ImmutableMap.of("id", 4, "testString", "asdf")) - )); - } - - @Test - public void nullValueShouldBeenIgnored() { - LinkedHashMap value = new LinkedHashMap<>(); - value.put("id", LITERAL_NULL); - when(inputPlan.hasNext()).thenReturn(true, false); - when(inputPlan.next()).thenReturn(new ExprTupleValue(value)); - - HeadOperator plan = new HeadOperator(inputPlan, - false, dsl.less(DSL.ref("id", INTEGER), DSL.literal(5)), defaultResultCount); - List result = execute(plan); - assertEquals(0, result.size()); - } - - @Test - public void headTest_missingValueShouldBeenIgnored() { - LinkedHashMap value = new LinkedHashMap<>(); - value.put("id", LITERAL_MISSING); - when(inputPlan.hasNext()).thenReturn(true, false); - when(inputPlan.next()).thenReturn(new ExprTupleValue(value)); - - HeadOperator plan = new HeadOperator(inputPlan, - false, dsl.less(DSL.ref("id", INTEGER), DSL.literal(5)), defaultResultCount); - List result = execute(plan); - assertEquals(0, result.size()); - } -} \ No newline at end of file diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitorTest.java index f97c551afe..e2d9dbcfde 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/PhysicalPlanNodeVisitorTest.java @@ -53,20 +53,16 @@ public void print_physical_plan() { PhysicalPlanDSL.project( PhysicalPlanDSL.rename( PhysicalPlanDSL.agg( - PhysicalPlanDSL.head( - PhysicalPlanDSL.rareTopN( - PhysicalPlanDSL.filter( - PhysicalPlanDSL.limit( - new TestScan(), - 1, 1 - ), - dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10))), - CommandType.TOP, - ImmutableList.of(), - DSL.ref("response", INTEGER)), - false, - DSL.literal(false), - 10), + PhysicalPlanDSL.rareTopN( + PhysicalPlanDSL.filter( + PhysicalPlanDSL.limit( + new TestScan(), + 1, 1 + ), + dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10))), + CommandType.TOP, + ImmutableList.of(), + DSL.ref("response", INTEGER)), ImmutableList .of(DSL.named("avg(response)", dsl.avg(DSL.ref("response", INTEGER)))), ImmutableList.of()), @@ -80,10 +76,9 @@ public void print_physical_plan() { + "\tProject->\n" + "\t\tRename->\n" + "\t\t\tAggregation->\n" - + "\t\t\t\tHead->\n" - + "\t\t\t\t\tRareTopN->\n" - + "\t\t\t\t\t\tFilter->\n" - + "\t\t\t\t\t\t\tLimit->", + + "\t\t\t\tRareTopN->\n" + + "\t\t\t\t\tFilter->\n" + + "\t\t\t\t\t\tLimit->", printer.print(plan)); } @@ -95,11 +90,6 @@ public void test_PhysicalPlanVisitor_should_return_null() { assertNull(filter.accept(new PhysicalPlanNodeVisitor() { }, null)); - PhysicalPlan head = PhysicalPlanDSL.head( - new TestScan(), false, dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10)), 10); - assertNull(head.accept(new PhysicalPlanNodeVisitor() { - }, null)); - PhysicalPlan aggregation = PhysicalPlanDSL.agg( filter, ImmutableList.of(DSL.named("avg(response)", @@ -164,11 +154,6 @@ public String visitFilter(FilterOperator node, Integer tabs) { return name(node, "Filter->", tabs); } - @Override - public String visitHead(HeadOperator node, Integer tabs) { - return name(node, "Head->", tabs); - } - @Override public String visitAggregation(AggregationOperator node, Integer tabs) { return name(node, "Aggregation->", tabs); diff --git a/docs/experiment/ppl/cmd/head.rst b/docs/experiment/ppl/cmd/head.rst index d88dcc1800..00bc7fb372 100644 --- a/docs/experiment/ppl/cmd/head.rst +++ b/docs/experiment/ppl/cmd/head.rst @@ -16,10 +16,8 @@ Description Syntax ============ -head [keeplast = (true | false)] [while "("")"] [N] +head [N] -* keeplast: optional. use in conjunction with the while argument to determine whether the last result in the result set is retained. The last result returned is the result that caused the to evaluate to false or NULL. Set keeplast=true to retain the last result in the result set. Set keeplast=false to discard the last result. **Default:** true -* while: optional. expression that evaluates to either true or false. statistical functions can not be used in the expression. **Default:** false * N: optional. number of results to return. **Default:** 10 Example 1: Get first 10 results @@ -63,40 +61,3 @@ PPL query:: | Nanette | 28 | +---------------+-----------+ -Example 3: Get first N results with while condition -=========================================================== - -The example show first N results from accounts index with while condition. - -PPL query:: - - od> source=accounts | fields firstname, age | sort age | head while(age < 21) 7; - fetched rows / total rows = 4/4 - +---------------+-----------+ - | firstname | age | - |---------------+-----------| - | Claudia | 20 | - | Copeland | 20 | - | Cornelia | 20 | - | Schultz | 20 | - | Simpson | 21 | - +---------------+-----------+ - -Example 4: Get first N results with while condition and last result which failed condition -========================================================================================== - -The example show first N results with while condition and last result which failed condition. - -PPL query:: - - od> source=accounts | fields firstname, age | sort age | head keeplast=false while(age < 21) 7; - fetched rows / total rows = 4/4 - +---------------+-----------+ - | firstname | age | - |---------------+-----------| - | Claudia | 20 | - | Copeland | 20 | - | Cornelia | 20 | - | Schultz | 20 | - +---------------+-----------+ - diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/protector/ElasticsearchExecutionProtector.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/protector/ElasticsearchExecutionProtector.java index b017839616..6f992b02e8 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/protector/ElasticsearchExecutionProtector.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/protector/ElasticsearchExecutionProtector.java @@ -22,7 +22,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.DedupeOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.EvalOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.HeadOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.LimitOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; @@ -101,20 +100,6 @@ public PhysicalPlan visitDedupe(DedupeOperator node, Object context) { node.getAllowedDuplication(), node.getKeepEmpty(), node.getConsecutive()); } - @Override - public PhysicalPlan visitHead(HeadOperator node, Object context) { - return new HeadOperator( - visitInput(node.getInput(), context), - node.getKeepLast(), - node.getWhileExpr(), - node.getNumber() - ); - } - - /** - * Decorate input node with {@link ResourceMonitorPlan} to avoid aggregate - * window function pre-loads too many data into memory in worst case. - */ @Override public PhysicalPlan visitWindow(WindowOperator node, Object context) { return new WindowOperator( diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/ElasticsearchExecutionProtectorTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/ElasticsearchExecutionProtectorTest.java index 0a38cc5740..ea2d55b6c1 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/ElasticsearchExecutionProtectorTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/executor/ElasticsearchExecutionProtectorTest.java @@ -101,9 +101,6 @@ public void testProtectIndexScan() { ReferenceExpression topField = ref("name", STRING); List topExprs = Arrays.asList(ref("age", INTEGER)); Expression filterExpr = literal(ExprBooleanValue.of(true)); - Expression whileExpr = literal(ExprBooleanValue.of(true)); - Boolean keepLast = false; - Integer headNumber = 5; List groupByExprs = Arrays.asList(named("age", ref("age", INTEGER))); List aggregators = Arrays.asList(named("avg(age)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), @@ -130,16 +127,12 @@ public void testProtectIndexScan() { PhysicalPlanDSL.remove( PhysicalPlanDSL.rename( PhysicalPlanDSL.agg( - PhysicalPlanDSL.head( - filter( - resourceMonitor( - new ElasticsearchIndexScan( - client, settings, indexName, - exprValueFactory)), - filterExpr), - keepLast, - whileExpr, - headNumber), + filter( + resourceMonitor( + new ElasticsearchIndexScan( + client, settings, indexName, + exprValueFactory)), + filterExpr), aggregators, groupByExprs), mappings), @@ -163,15 +156,11 @@ public void testProtectIndexScan() { PhysicalPlanDSL.remove( PhysicalPlanDSL.rename( PhysicalPlanDSL.agg( - PhysicalPlanDSL.head( - filter( - new ElasticsearchIndexScan( - client, settings, indexName, - exprValueFactory), - filterExpr), - keepLast, - whileExpr, - headNumber), + filter( + new ElasticsearchIndexScan( + client, settings, indexName, + exprValueFactory), + filterExpr), aggregators, groupByExprs), mappings), diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/HeadCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/HeadCommandIT.java index fb9497d102..b6b1a67e02 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/HeadCommandIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/HeadCommandIT.java @@ -68,31 +68,4 @@ public void testHeadWithNumber() throws IOException { rows("Hattie", 36), rows("Nanette", 28)); } - - @Test - public void testHeadWithWhile() throws IOException { - JSONObject result = - executeQuery(String - .format("source=%s | fields firstname, age | sort age | head while(age < 21) 7", - TEST_INDEX_ACCOUNT)); - verifyDataRows(result, - rows("Claudia", 20), - rows("Copeland", 20), - rows("Cornelia", 20), - rows("Schultz", 20), - rows("Simpson", 21)); - } - - @Test - public void testHeadWithKeeplast() throws IOException { - JSONObject result = - executeQuery(String.format( - "source=%s | fields firstname, age | sort age | head keeplast=false while(age < 21) 7", - TEST_INDEX_ACCOUNT)); - verifyDataRows(result, - rows("Claudia", 20), - rows("Copeland", 20), - rows("Cornelia", 20), - rows("Schultz", 20)); - } } diff --git a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 index 9866085710..59fb313b9e 100644 --- a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 @@ -51,13 +51,11 @@ NUM: 'NUM'; // ARGUMENT KEYWORDS KEEPEMPTY: 'KEEPEMPTY'; -KEEPLAST: 'KEEPLAST'; CONSECUTIVE: 'CONSECUTIVE'; DEDUP_SPLITVALUES: 'DEDUP_SPLITVALUES'; PARTITIONS: 'PARTITIONS'; ALLNUM: 'ALLNUM'; DELIM: 'DELIM'; -WHILE: 'WHILE'; // COMPARISON FUNCTION KEYWORDS CASE: 'CASE'; diff --git a/ppl/src/main/antlr/OpenDistroPPLParser.g4 b/ppl/src/main/antlr/OpenDistroPPLParser.g4 index a4fe7f0938..077b265457 100644 --- a/ppl/src/main/antlr/OpenDistroPPLParser.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLParser.g4 @@ -77,8 +77,6 @@ evalCommand headCommand : HEAD - (KEEPLAST EQUAL keeplast=booleanLiteral)? - (WHILE LT_PRTHS whileExpr=logicalExpression RT_PRTHS)? (number=integerLiteral)? ; diff --git a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilder.java index 6e56c80509..32a8531d4d 100644 --- a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilder.java @@ -188,9 +188,8 @@ public UnresolvedPlan visitDedupCommand(DedupCommandContext ctx) { */ @Override public UnresolvedPlan visitHeadCommand(HeadCommandContext ctx) { - UnresolvedExpression unresolvedExpr = - ctx.whileExpr != null ? visitExpression(ctx.logicalExpression()) : null; - return new Head(ArgumentFactory.getArgumentList(ctx, unresolvedExpr)); + Integer size = ctx.number != null ? Integer.parseInt(ctx.number.getText()) : 10; + return new Head(size); } /** diff --git a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/ArgumentFactory.java b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/ArgumentFactory.java index b4dbf841db..8b5d9e5dd4 100644 --- a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/ArgumentFactory.java +++ b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/ArgumentFactory.java @@ -18,10 +18,8 @@ import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.BooleanLiteralContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.DedupCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.FieldsCommandContext; -import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.HeadCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.IntegerLiteralContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.RareCommandContext; -import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SortCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SortFieldContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.StatsCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.TopCommandContext; @@ -29,8 +27,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.DataType; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedArgument; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils; import java.util.Arrays; import java.util.Collections; @@ -101,27 +97,6 @@ public static List getArgumentList(DedupCommandContext ctx) { ); } - /** - * Get list of {@link Argument}. - * - * @param ctx HeadCommandContext instance - * @return the list of arguments fetched from the head command - */ - public static List getArgumentList(HeadCommandContext ctx, - UnresolvedExpression unresolvedExpr) { - return Arrays.asList( - ctx.keeplast != null - ? new UnresolvedArgument("keeplast", getArgumentValue(ctx.keeplast)) - : new UnresolvedArgument("keeplast", new Literal(true, DataType.BOOLEAN)), - ctx.whileExpr != null && unresolvedExpr != null - ? new UnresolvedArgument("whileExpr", unresolvedExpr) - : new UnresolvedArgument("whileExpr", new Literal(true, DataType.BOOLEAN)), - ctx.number != null - ? new UnresolvedArgument("number", getArgumentValue(ctx.number)) - : new UnresolvedArgument("number", new Literal(10, DataType.INTEGER)) - ); - } - /** * Get list of {@link Argument}. * diff --git a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizer.java b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizer.java index 4595e017c9..16f1dccd39 100644 --- a/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizer.java +++ b/ppl/src/main/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizer.java @@ -31,7 +31,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Map; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Not; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Or; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedArgument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Xor; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Aggregation; @@ -49,7 +48,6 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalAggregation; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalDedupe; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalEval; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalHead; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRareTopN; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove; @@ -208,19 +206,11 @@ public String visitDedupe(Dedupe node, String context) { consecutive); } - /** - * Build {@link LogicalHead}. - */ @Override public String visitHead(Head node, String context) { String child = node.getChild().get(0).accept(this, context); - List options = node.getOptions(); - Boolean keeplast = (Boolean) ((Literal) options.get(0).getValue()).getValue(); - String whileExpr = visitExpression(options.get(1).getValue()); - Integer number = (Integer) ((Literal) options.get(2).getValue()).getValue(); - - return StringUtils.format("%s | head keeplast=%b while(%s) %d", child, keeplast, whileExpr, - number); + Integer size = node.getSize(); + return StringUtils.format("%s | head %d", child, size); } private String visitFieldList(List fieldList) { diff --git a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilderTest.java index 82bd27aa0e..5c2bc96ed6 100644 --- a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/parser/AstBuilderTest.java @@ -24,7 +24,6 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.dedupe; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.defaultDedupArgs; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.defaultFieldsArgs; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.defaultHeadArgs; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.defaultSortFieldArgs; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.defaultStatsArgs; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.eval; @@ -44,8 +43,6 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rename; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.sort; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.unresolvedArg; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.unresolvedArgList; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; @@ -306,48 +303,13 @@ public void testDedupCommandWithSortby() { @Test public void testHeadCommand() { assertEqual("source=t | head", - head( - relation("t"), - defaultHeadArgs() - )); + head(relation("t"), 10)); } @Test public void testHeadCommandWithNumber() { assertEqual("source=t | head 3", - head( - relation("t"), - unresolvedArgList( - unresolvedArg("keeplast", booleanLiteral(true)), - unresolvedArg("whileExpr", booleanLiteral(true)), - unresolvedArg("number", intLiteral(3))) - )); - } - - @Test - public void testHeadCommandWithWhileExpr() { - - assertEqual("source=t | head while(a < 5) 5", - head( - relation("t"), - unresolvedArgList( - unresolvedArg("keeplast", booleanLiteral(true)), - unresolvedArg("whileExpr", compare("<", field("a"), intLiteral(5))), - unresolvedArg("number", intLiteral(5))) - )); - } - - @Test - public void testHeadCommandWithKeepLast() { - - assertEqual("source=t | head keeplast=false while(a < 5) 5", - head( - relation("t"), - unresolvedArgList( - unresolvedArg("keeplast", booleanLiteral(false)), - unresolvedArg("whileExpr", compare("<", field("a"), intLiteral(5))), - unresolvedArg("number", intLiteral(5))) - )); + head(relation("t"), 3)); } @Test diff --git a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java index 2221245866..62ce424238 100644 --- a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java +++ b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java @@ -86,16 +86,10 @@ public void testDedupCommand() { @Test public void testHeadCommandWithNumber() { - assertEquals("source=t | head keeplast=true while(***) 3", + assertEquals("source=t | head 3", anonymize("source=t | head 3")); } - @Test - public void testHeadCommandWithWhileExpr() { - assertEquals("source=t | head keeplast=true while(a < ***) 5", - anonymize("source=t | head while(a < 5) 5")); - } - //todo, sort order is ignored, it doesn't impact the log analysis. @Test public void testSortCommandWithOptions() {