From 5fe5aa77df04373f483da127ebc7fd690cacf868 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Mon, 17 Aug 2020 14:28:26 -0700 Subject: [PATCH 01/15] ast - rare and top command --- .../sql/ast/AbstractNodeVisitor.java | 10 ++++ .../sql/ast/dsl/AstDSL.java | 10 ++++ .../sql/ast/tree/Rare.java | 48 +++++++++++++++++ .../sql/ast/tree/Top.java | 53 +++++++++++++++++++ ppl/src/main/antlr/OpenDistroPPLLexer.g4 | 2 + ppl/src/main/antlr/OpenDistroPPLParser.g4 | 15 +++++- .../sql/ppl/parser/AstBuilder.java | 49 +++++++++++++++++ .../sql/ppl/utils/ArgumentFactory.java | 15 ++++++ .../sql/ppl/antlr/PPLSyntaxParserTest.java | 18 +++++++ .../sql/ppl/parser/AstBuilderTest.java | 34 ++++++++++++ 10 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java index 02806fbcfd..8676186959 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java @@ -43,6 +43,8 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; /** * AST nodes visitor Defines the traverse path. @@ -177,6 +179,14 @@ public T visitDedupe(Dedupe node, C context) { return visitChildren(node, context); } + public T visitRare(Rare node, C context) { + return visitChildren(node, context); + } + + public T visitTop(Top node, C context) { + return visitChildren(node, context); + } + public T visitValues(Values node, C context) { return visitChildren(node, context); } 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 4b85c9b2bf..090acace48 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 @@ -45,6 +45,8 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import java.util.Arrays; import java.util.List; import lombok.experimental.UtilityClass; @@ -287,4 +289,12 @@ public static Sort sort(UnresolvedPlan input, List options, Field... s public static Dedupe dedupe(UnresolvedPlan input, List options, Field... fields) { return new Dedupe(input, options, Arrays.asList(fields)); } + + public static Rare rare(UnresolvedPlan input, List groupList, Field... fields) { + return new Rare(Arrays.asList(fields), groupList).attach(input); + } + + public static Top top(UnresolvedPlan input, List options, List groupList, Field... fields) { + return new Top(options, Arrays.asList(fields), groupList).attach(input); + } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java new file mode 100644 index 0000000000..348389640e --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java @@ -0,0 +1,48 @@ +package com.amazon.opendistroforelasticsearch.sql.ast.tree; + +import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; +import com.google.common.collect.ImmutableList; +import java.util.List; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; + +/** + * AST node represent Rare operation. + */ +@Getter +@Setter +@ToString +@EqualsAndHashCode(callSuper = false) +public class Rare extends UnresolvedPlan { + private UnresolvedPlan child; + private final List fields; + private final List groupExprList; + + /** + * Rare Constructor. + */ + public Rare(List fields,List groupExprList) { + this.fields = fields; + this.groupExprList = groupExprList; + } + + @Override + public Rare attach(UnresolvedPlan child) { + this.child = child; + return this; + } + + @Override + public List getChild() { + return ImmutableList.of(this.child); + } + + @Override + public T accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitRare(this, context); + } +} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java new file mode 100644 index 0000000000..fa244c2cf6 --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java @@ -0,0 +1,53 @@ +package com.amazon.opendistroforelasticsearch.sql.ast.tree; + + +import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; +import com.google.common.collect.ImmutableList; +import java.util.List; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; + +/** + * AST node represent Top operation. + */ +@Getter +@Setter +@ToString +@EqualsAndHashCode(callSuper = false) +public class Top extends UnresolvedPlan { + private UnresolvedPlan child; + private final List N; + private final List fields; + private final List groupExprList; + + /** + * Top Constructors. + */ + public Top(List N, List fields,List groupExprList) { + this.N = N; + this.fields = fields; + this.groupExprList = groupExprList; + } + + @Override + public Top attach(UnresolvedPlan child) { + this.child = child; + return this; + } + + @Override + public List getChild() { + return ImmutableList.of(this.child); + } + + @Override + public T accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitTop(this, context); + } +} + diff --git a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 index 16564a6e21..63d9ee5dac 100644 --- a/ppl/src/main/antlr/OpenDistroPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLLexer.g4 @@ -28,6 +28,8 @@ STATS: 'STATS'; DEDUP: 'DEDUP'; SORT: 'SORT'; EVAL: 'EVAL'; +TOP: 'TOP'; +RARE: 'RARE'; // COMMAND ASSIST KEYWORDS AS: 'AS'; diff --git a/ppl/src/main/antlr/OpenDistroPPLParser.g4 b/ppl/src/main/antlr/OpenDistroPPLParser.g4 index d6ee05fc93..9f405cf8e0 100644 --- a/ppl/src/main/antlr/OpenDistroPPLParser.g4 +++ b/ppl/src/main/antlr/OpenDistroPPLParser.g4 @@ -29,7 +29,7 @@ pplStatement /** commands */ commands : whereCommand | fieldsCommand | renameCommand | statsCommand | dedupCommand | sortCommand | evalCommand - ; + | topCommand | rareCommand; searchCommand : (SEARCH)? fromClause #searchFrom @@ -75,6 +75,19 @@ evalCommand : EVAL evalClause (COMMA evalClause)* ; +topCommand + : TOP + (number=integerLiteral)? + fieldList + (byClause)? + ; + +rareCommand + : RARE + fieldList + (byClause)? + ; + /** clauses */ fromClause : SOURCE EQUAL tableSource 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 9924f3ac12..09bdd3ec66 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 @@ -27,6 +27,8 @@ import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SortCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.StatsCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.WhereCommandContext; +import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.RareCommandContext; +import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.TopCommandContext; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let; @@ -40,6 +42,8 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParserBaseVisitor; @@ -197,6 +201,51 @@ public UnresolvedPlan visitEvalCommand(EvalCommandContext ctx) { ); } + /** + * Rare command. + */ + @Override + public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { + List groupList = ctx.byClause() == null ? Collections.emptyList() : + ctx.byClause() + .fieldList() + .fieldExpression() + .stream() + .map(this::visitExpression) + .collect(Collectors.toList()); + return new Rare( + ctx.fieldList() + .fieldExpression() + .stream() + .map(field -> (Field) visitExpression(field)) + .collect(Collectors.toList()), + groupList + ); + } + + /** + * Top command. + */ + @Override + public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { + List groupList = ctx.byClause() == null ? Collections.emptyList() : + ctx.byClause() + .fieldList() + .fieldExpression() + .stream() + .map(this::visitExpression) + .collect(Collectors.toList()); + return new Top( + ArgumentFactory.getArgumentList(ctx), + ctx.fieldList() + .fieldExpression() + .stream() + .map(field -> (Field) visitExpression(field)) + .collect(Collectors.toList()), + groupList + ); + } + /** * From clause. */ 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 6167639ed5..35822e7d6e 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 @@ -22,6 +22,7 @@ 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; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.DataType; @@ -143,4 +144,18 @@ private static Literal getArgumentValue(ParserRuleContext ctx) { : new Literal(StringUtils.unquoteText(ctx.getText()), DataType.STRING); } + /** + * Get list of {@link Argument}. + * + * @param ctx TopCommandContext instance + * @return the list of arguments fetched from the top command + */ + public static List getArgumentList(TopCommandContext ctx) { + return Collections.singletonList( + ctx.number != null + ? new Argument("N", getArgumentValue(ctx.number)) + : new Argument("N", new Literal(10, DataType.INTEGER)) + ); + } + } diff --git a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java index a847181847..e297ba1c03 100644 --- a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -51,4 +51,22 @@ public void testSearchCommandWithoutSourceShouldFail() { new PPLSyntaxParser().analyzeSyntax("search a=1"); } + + @Test + public void testRareCommandShouldPass(){ + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | rare a by b"); + assertNotEquals(null, tree); + } + + @Test + public void testTopCommandWithNShouldPass(){ + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top 1 a by b"); + assertNotEquals(null, tree); + } + + @Test + public void testTopCommandWithoutNShouldPass(){ + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a by b"); + assertNotEquals(null, tree); + } } \ No newline at end of file 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 d021dd5c68..107c1ba135 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 @@ -41,6 +41,8 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.sort; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.sortOptions; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rare; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.top; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; @@ -301,6 +303,38 @@ public void testIndexName() { )); } + @Test + public void testRareCommand(){ + assertEqual("source=t | rare a by b", + rare( + relation("t"), + exprList(field("b")), + field("a") + )); + } + + @Test + public void testTopCommandWithN(){ + assertEqual("source=t | top 1 a by b", + top( + relation("t"), + exprList(argument("N", intLiteral(1))), + exprList(field("b")), + field("a") + )); + } + + @Test + public void testTopCommandWithoutN(){ + assertEqual("source=t | top a by b", + top( + relation("t"), + exprList(argument("N", intLiteral(10))), + exprList(field("b")), + field("a") + )); + } + protected void assertEqual(String query, Node expectedPlan) { Node actualPlan = plan(query); assertEquals(expectedPlan, actualPlan); From 4c2000afbff13760abb719b9c129568334c1ab18 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Mon, 17 Aug 2020 17:25:48 -0700 Subject: [PATCH 02/15] fix build failure --- .../sql/ast/AbstractNodeVisitor.java | 4 +- .../sql/ast/dsl/AstDSL.java | 11 +-- .../sql/ast/tree/Rare.java | 57 ++++++++------- .../sql/ast/tree/Top.java | 61 ++++++++-------- .../sql/ppl/parser/AstBuilder.java | 6 +- .../sql/ppl/utils/ArgumentFactory.java | 22 +++--- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 7 +- .../sql/ppl/parser/AstBuilderTest.java | 71 ++++++++++++------- 8 files changed, 135 insertions(+), 104 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java index 8676186959..189e4e5088 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java @@ -39,12 +39,12 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; /** * AST nodes visitor Defines the traverse path. 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 090acace48..b530c348f8 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 @@ -40,13 +40,13 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import java.util.Arrays; import java.util.List; import lombok.experimental.UtilityClass; @@ -290,11 +290,14 @@ public static Dedupe dedupe(UnresolvedPlan input, List options, Field. return new Dedupe(input, options, Arrays.asList(fields)); } - public static Rare rare(UnresolvedPlan input, List groupList, Field... fields) { + public static Rare rare( + UnresolvedPlan input, List groupList, Field... fields) { return new Rare(Arrays.asList(fields), groupList).attach(input); } - public static Top top(UnresolvedPlan input, List options, List groupList, Field... fields) { + public static Top top( + UnresolvedPlan input, List options, + List groupList, Field... fields) { return new Top(options, Arrays.asList(fields), groupList).attach(input); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java index 348389640e..2d70b2aa45 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java @@ -4,7 +4,9 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.google.common.collect.ImmutableList; + import java.util.List; + import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; @@ -18,31 +20,32 @@ @ToString @EqualsAndHashCode(callSuper = false) public class Rare extends UnresolvedPlan { - private UnresolvedPlan child; - private final List fields; - private final List groupExprList; - - /** - * Rare Constructor. - */ - public Rare(List fields,List groupExprList) { - this.fields = fields; - this.groupExprList = groupExprList; - } - - @Override - public Rare attach(UnresolvedPlan child) { - this.child = child; - return this; - } - - @Override - public List getChild() { - return ImmutableList.of(this.child); - } - - @Override - public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitRare(this, context); - } + + private UnresolvedPlan child; + private final List fields; + private final List groupExprList; + + /** + * Rare Constructor. + */ + public Rare(List fields, List groupExprList) { + this.fields = fields; + this.groupExprList = groupExprList; + } + + @Override + public Rare attach(UnresolvedPlan child) { + this.child = child; + return this; + } + + @Override + public List getChild() { + return ImmutableList.of(this.child); + } + + @Override + public T accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitRare(this, context); + } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java index fa244c2cf6..42a09ffd50 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java @@ -1,6 +1,5 @@ package com.amazon.opendistroforelasticsearch.sql.ast.tree; - import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; @@ -20,34 +19,36 @@ @ToString @EqualsAndHashCode(callSuper = false) public class Top extends UnresolvedPlan { - private UnresolvedPlan child; - private final List N; - private final List fields; - private final List groupExprList; - - /** - * Top Constructors. - */ - public Top(List N, List fields,List groupExprList) { - this.N = N; - this.fields = fields; - this.groupExprList = groupExprList; - } - - @Override - public Top attach(UnresolvedPlan child) { - this.child = child; - return this; - } - - @Override - public List getChild() { - return ImmutableList.of(this.child); - } - - @Override - public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitTop(this, context); - } + + private UnresolvedPlan child; + private final List noOfResults; + private final List fields; + private final List groupExprList; + + /** + * Top Constructors. + */ + public Top( + List noOfResults, List fields, List groupExprList) { + this.noOfResults = noOfResults; + this.fields = fields; + this.groupExprList = groupExprList; + } + + @Override + public Top attach(UnresolvedPlan child) { + this.child = child; + return this; + } + + @Override + public List getChild() { + return ImmutableList.of(this.child); + } + + @Override + public T accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitTop(this, context); + } } 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 09bdd3ec66..65c7bcefc7 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 @@ -20,15 +20,15 @@ import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.FieldsCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.FromClauseContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.PplStatementContext; +import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.RareCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.RenameCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SearchFilterFromContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SearchFromContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SearchFromFilterContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.SortCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.StatsCommandContext; -import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.WhereCommandContext; -import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.RareCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.TopCommandContext; +import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.WhereCommandContext; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let; @@ -39,10 +39,10 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser; 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 35822e7d6e..26cf185ed7 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 @@ -136,14 +136,6 @@ public static List getArgumentList(SortFieldContext ctx) { ); } - private static Literal getArgumentValue(ParserRuleContext ctx) { - return ctx instanceof IntegerLiteralContext - ? new Literal(Integer.parseInt(ctx.getText()), DataType.INTEGER) - : ctx instanceof BooleanLiteralContext - ? new Literal(Boolean.valueOf(ctx.getText()), DataType.BOOLEAN) - : new Literal(StringUtils.unquoteText(ctx.getText()), DataType.STRING); - } - /** * Get list of {@link Argument}. * @@ -152,10 +144,18 @@ private static Literal getArgumentValue(ParserRuleContext ctx) { */ public static List getArgumentList(TopCommandContext ctx) { return Collections.singletonList( - ctx.number != null - ? new Argument("N", getArgumentValue(ctx.number)) - : new Argument("N", new Literal(10, DataType.INTEGER)) + ctx.number != null + ? new Argument("noOfResults", getArgumentValue(ctx.number)) + : new Argument("noOfResults", new Literal(10, DataType.INTEGER)) ); } + private static Literal getArgumentValue(ParserRuleContext ctx) { + return ctx instanceof IntegerLiteralContext + ? new Literal(Integer.parseInt(ctx.getText()), DataType.INTEGER) + : ctx instanceof BooleanLiteralContext + ? new Literal(Boolean.valueOf(ctx.getText()), DataType.BOOLEAN) + : new Literal(StringUtils.unquoteText(ctx.getText()), DataType.STRING); + } + } diff --git a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java index e297ba1c03..0eacd6d508 100644 --- a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -23,6 +23,7 @@ import org.junit.rules.ExpectedException; public class PPLSyntaxParserTest { + @Rule public ExpectedException exceptionRule = ExpectedException.none(); @@ -53,19 +54,19 @@ public void testSearchCommandWithoutSourceShouldFail() { } @Test - public void testRareCommandShouldPass(){ + public void testRareCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | rare a by b"); assertNotEquals(null, tree); } @Test - public void testTopCommandWithNShouldPass(){ + public void testTopCommandWithNShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top 1 a by b"); assertNotEquals(null, tree); } @Test - public void testTopCommandWithoutNShouldPass(){ + public void testTopCommandWithoutNShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a by b"); assertNotEquals(null, tree); } 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 107c1ba135..a8cf235493 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 @@ -36,12 +36,12 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.map; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.nullLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.projectWithArg; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rare; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.relation; 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.sortOptions; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rare; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.top; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; @@ -304,35 +304,58 @@ public void testIndexName() { } @Test - public void testRareCommand(){ - assertEqual("source=t | rare a by b", - rare( - relation("t"), - exprList(field("b")), - field("a") - )); + public void testRareCommand() { + assertEqual("source=t | rare a by b", + rare( + relation("t"), + exprList(field("b")), + field("a") + )); } @Test - public void testTopCommandWithN(){ - assertEqual("source=t | top 1 a by b", - top( - relation("t"), - exprList(argument("N", intLiteral(1))), - exprList(field("b")), - field("a") - )); + public void testRareCommandWithMultipleFields() { + assertEqual("source=t | rare `a`, `b` by `c`", + rare( + relation("t"), + exprList(field("c")), + field("a"), + field("b") + )); } @Test - public void testTopCommandWithoutN(){ - assertEqual("source=t | top a by b", - top( - relation("t"), - exprList(argument("N", intLiteral(10))), - exprList(field("b")), - field("a") - )); + public void testTopCommandWithN() { + assertEqual("source=t | top 1 a by b", + top( + relation("t"), + exprList(argument("noOfResults", intLiteral(1))), + exprList(field("b")), + field("a") + )); + } + + @Test + public void testTopCommandWithMultipleFields() { + assertEqual("source=t | top 1 `a`, `b` by `c`", + top( + relation("t"), + exprList(argument("noOfResults", intLiteral(1))), + exprList(field("c")), + field("a"), + field("b") + )); + } + + @Test + public void testTopCommandWithoutN() { + assertEqual("source=t | top a by b", + top( + relation("t"), + exprList(argument("noOfResults", intLiteral(10))), + exprList(field("b")), + field("a") + )); } protected void assertEqual(String query, Node expectedPlan) { From 2d98f316718ef08d03f4142c2dd81662a1886d66 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Mon, 17 Aug 2020 17:44:11 -0700 Subject: [PATCH 03/15] fix build failure - test coverage ratio for ppl --- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 20 ++++++++- .../sql/ppl/parser/AstBuilderTest.java | 43 ++++++++++++++----- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 0eacd6d508..1d7fe72cf5 100644 --- a/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -55,18 +55,36 @@ public void testSearchCommandWithoutSourceShouldFail() { @Test public void testRareCommandShouldPass() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | rare a"); + assertNotEquals(null, tree); + } + + @Test + public void testRareCommandWithGroupByShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | rare a by b"); assertNotEquals(null, tree); } + @Test + public void testTopCommandWithoutNShouldPass() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a"); + assertNotEquals(null, tree); + } + @Test public void testTopCommandWithNShouldPass() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top 1 a"); + assertNotEquals(null, tree); + } + + @Test + public void testTopCommandWithNAndGroupByShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top 1 a by b"); assertNotEquals(null, tree); } @Test - public void testTopCommandWithoutNShouldPass() { + public void testTopCommandWithoutNAndGroupByShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a by b"); assertNotEquals(null, tree); } 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 a8cf235493..9063693a18 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 @@ -305,6 +305,16 @@ public void testIndexName() { @Test public void testRareCommand() { + assertEqual("source=t | rare a", + rare( + relation("t"), + emptyList(), + field("a") + )); + } + + @Test + public void testRareCommandWithGroupBy() { assertEqual("source=t | rare a by b", rare( relation("t"), @@ -326,38 +336,49 @@ public void testRareCommandWithMultipleFields() { @Test public void testTopCommandWithN() { - assertEqual("source=t | top 1 a by b", + assertEqual("source=t | top 1 a", top( relation("t"), exprList(argument("noOfResults", intLiteral(1))), - exprList(field("b")), + emptyList(), field("a") )); } @Test - public void testTopCommandWithMultipleFields() { - assertEqual("source=t | top 1 `a`, `b` by `c`", + public void testTopCommandWithoutNAndGroupBy() { + assertEqual("source=t | top a", top( relation("t"), - exprList(argument("noOfResults", intLiteral(1))), - exprList(field("c")), - field("a"), - field("b") + exprList(argument("noOfResults", intLiteral(10))), + emptyList(), + field("a") )); } @Test - public void testTopCommandWithoutN() { - assertEqual("source=t | top a by b", + public void testTopCommandWithNAndGroupBy() { + assertEqual("source=t | top 1 a by b", top( relation("t"), - exprList(argument("noOfResults", intLiteral(10))), + exprList(argument("noOfResults", intLiteral(1))), exprList(field("b")), field("a") )); } + @Test + public void testTopCommandWithMultipleFields() { + assertEqual("source=t | top 1 `a`, `b` by `c`", + top( + relation("t"), + exprList(argument("noOfResults", intLiteral(1))), + exprList(field("c")), + field("a"), + field("b") + )); + } + protected void assertEqual(String query, Node expectedPlan) { Node actualPlan = plan(query); assertEquals(expectedPlan, actualPlan); From 016a886fe57dad1662b4bb54aef10f046cb930e9 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Tue, 25 Aug 2020 20:40:44 -0700 Subject: [PATCH 04/15] add logical, physical plan for rare & top --- .../sql/analysis/Analyzer.java | 67 +++++ .../sql/ast/tree/Rare.java | 2 + .../sql/ast/tree/Top.java | 2 + .../sql/planner/DefaultImplementor.java | 23 ++ .../sql/planner/logical/LogicalPlanDSL.java | 11 + .../logical/LogicalPlanNodeVisitor.java | 10 + .../sql/planner/logical/LogicalRare.java | 35 +++ .../sql/planner/logical/LogicalTop.java | 36 +++ .../sql/planner/physical/PhysicalPlanDSL.java | 15 ++ .../physical/PhysicalPlanNodeVisitor.java | 8 + .../sql/planner/physical/RareOperator.java | 232 +++++++++++++++++ .../sql/planner/physical/TopOperator.java | 239 ++++++++++++++++++ .../ElasticsearchExecutionProtector.java | 14 + 13 files changed, 694 insertions(+) create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.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 bc40def04f..b4c6b01738 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,10 +29,12 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprMissingValue; @@ -49,10 +51,12 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRare; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRename; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalSort; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalTop; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalValues; import com.amazon.opendistroforelasticsearch.sql.storage.StorageEngine; import com.amazon.opendistroforelasticsearch.sql.storage.Table; @@ -166,6 +170,69 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) { return new LogicalAggregation(child, aggregators, groupBys); } + /** + * Build {@link LogicalRare}. + */ + @Override + public LogicalPlan visitRare(Rare node, AnalysisContext context) { + final LogicalPlan child = node.getChild().get(0).accept(this, context); + + ImmutableList.Builder groupbyBuilder = new ImmutableList.Builder<>(); + for (UnresolvedExpression expr : node.getGroupExprList()) { + groupbyBuilder.add(expressionAnalyzer.analyze(expr, context)); + } + ImmutableList groupBys = groupbyBuilder.build(); + + ImmutableList.Builder fieldsBuilder = new ImmutableList.Builder<>(); + for (Field f : node.getFields()) { + fieldsBuilder.add(expressionAnalyzer.analyze(f, context)); + } + ImmutableList fields = fieldsBuilder.build(); + + // new context + context.push(); + TypeEnvironment newEnv = context.peek(); + groupBys.forEach(group -> newEnv.define(new Symbol(Namespace.FIELD_NAME, + group.toString()), group.type())); + fields.forEach(field -> newEnv.define(new Symbol(Namespace.FIELD_NAME, + field.toString()), field.type())); + + return new LogicalRare(child, fields, groupBys); + } + + /** + * Build {@link LogicalTop}. + */ + @Override + public LogicalPlan visitTop(Top node, AnalysisContext context) { + final LogicalPlan child = node.getChild().get(0).accept(this, context); + + ImmutableList.Builder groupbyBuilder = new ImmutableList.Builder<>(); + for (UnresolvedExpression expr : node.getGroupExprList()) { + groupbyBuilder.add(expressionAnalyzer.analyze(expr, context)); + } + ImmutableList groupBys = groupbyBuilder.build(); + + ImmutableList.Builder fieldsBuilder = new ImmutableList.Builder<>(); + for (Field f : node.getFields()) { + fieldsBuilder.add(expressionAnalyzer.analyze(f, context)); + } + ImmutableList fields = fieldsBuilder.build(); + + // new context + context.push(); + TypeEnvironment newEnv = context.peek(); + groupBys.forEach(group -> newEnv.define(new Symbol(Namespace.FIELD_NAME, + group.toString()), group.type())); + fields.forEach(field -> newEnv.define(new Symbol(Namespace.FIELD_NAME, + field.toString()), field.type())); + + List options = node.getNoOfResults(); + Integer noOfResults = (Integer) options.get(0).getValue().getValue(); + + return new LogicalTop(child, noOfResults, fields, groupBys); + } + /** * Build {@link LogicalProject} or {@link LogicalRemove} from {@link Field}. * diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java index 2d70b2aa45..3f9a95b850 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java @@ -9,6 +9,7 @@ import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.NonNull; import lombok.Setter; import lombok.ToString; @@ -28,6 +29,7 @@ public class Rare extends UnresolvedPlan { /** * Rare Constructor. */ + @NonNull public Rare(List fields, List groupExprList) { this.fields = fields; this.groupExprList = groupExprList; diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java index 42a09ffd50..325de04751 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java @@ -8,6 +8,7 @@ import java.util.List; import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.NonNull; import lombok.Setter; import lombok.ToString; @@ -28,6 +29,7 @@ public class Top extends UnresolvedPlan { /** * Top Constructors. */ + @NonNull public Top( List noOfResults, List fields, List groupExprList) { this.noOfResults = noOfResults; 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 090cd0c297..5f3b746960 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 @@ -23,10 +23,12 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanNodeVisitor; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRare; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRename; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalSort; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalTop; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalValues; import com.amazon.opendistroforelasticsearch.sql.planner.physical.AggregationOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.DedupeOperator; @@ -34,9 +36,11 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RemoveOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RenameOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.SortOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.TopOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ValuesOperator; /** @@ -51,6 +55,25 @@ */ public class DefaultImplementor extends LogicalPlanNodeVisitor { + @Override + public PhysicalPlan visitRare(LogicalRare node, C context) { + return new RareOperator( + visitChild(node, context), + node.getFieldList(), + node.getGroupByList() + ); + } + + @Override + public PhysicalPlan visitTop(LogicalTop node, C context) { + return new TopOperator( + visitChild(node, context), + node.getNoOfResults(), + node.getFieldList(), + node.getGroupByList() + ); + } + @Override public PhysicalPlan visitDedupe(LogicalDedupe node, C context) { return new DedupeOperator( 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 1066b279fc..fa5c148d77 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 @@ -33,6 +33,7 @@ */ @UtilityClass public class LogicalPlanDSL { + public static LogicalPlan aggregation( LogicalPlan input, List aggregatorList, List groupByList) { return new LogicalAggregation(input, aggregatorList, groupByList); @@ -83,6 +84,16 @@ public static LogicalPlan dedupe( input, Arrays.asList(fields), allowedDuplication, keepEmpty, consecutive); } + public static LogicalPlan rare(LogicalPlan input, List groupByList, + Expression... fields) { + return new LogicalRare(input, Arrays.asList(fields), groupByList); + } + + public static LogicalPlan top(LogicalPlan input, int noOfResults, List groupByList, + Expression... fields) { + return new LogicalTop(input, noOfResults, Arrays.asList(fields), groupByList); + } + @SafeVarargs public LogicalPlan values(List... values) { return new LogicalValues(Arrays.asList(values)); 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 bd63b84e5d..012027e1d7 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 @@ -22,6 +22,7 @@ * @param context type. */ public abstract class LogicalPlanNodeVisitor { + protected R visitNode(LogicalPlan plan, C context) { return null; } @@ -65,4 +66,13 @@ public R visitSort(LogicalSort plan, C context) { public R visitValues(LogicalValues plan, C context) { return visitNode(plan, context); } + + public R visitRare(LogicalRare plan, C context) { + return visitNode(plan, context); + } + + public R visitTop(LogicalTop plan, C context) { + return visitNode(plan, context); + } + } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java new file mode 100644 index 0000000000..4dc7be5d70 --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java @@ -0,0 +1,35 @@ +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; + +/** + * Logical Rare Plan. + */ +@Getter +@ToString +@EqualsAndHashCode(callSuper = false) +@RequiredArgsConstructor +public class LogicalRare extends LogicalPlan { + + private final LogicalPlan child; + private final List fieldList; + @Getter + private final List groupByList; + + @Override + public List getChild() { + return Collections.singletonList(child); + } + + @Override + public R accept(LogicalPlanNodeVisitor visitor, C context) { + return visitor.visitRare(this, context); + } +} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java new file mode 100644 index 0000000000..1909a07582 --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java @@ -0,0 +1,36 @@ +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; + +/** + * Logical Top Plan. + */ +@Getter +@ToString +@EqualsAndHashCode(callSuper = false) +@RequiredArgsConstructor +public class LogicalTop extends LogicalPlan { + + private final LogicalPlan child; + private final Integer noOfResults; + private final List fieldList; + @Getter + private final List groupByList; + + @Override + public List getChild() { + return Collections.singletonList(child); + } + + @Override + public R accept(LogicalPlanNodeVisitor visitor, C context) { + return visitor.visitTop(this, context); + } +} 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 40a8348976..efa262f1d3 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 @@ -80,6 +80,21 @@ public static DedupeOperator dedupe( input, Arrays.asList(expressions), allowedDuplication, keepEmpty, consecutive); } + public static RareOperator rare(PhysicalPlan input, List groups, + Expression... expressions) { + return new RareOperator(input, Arrays.asList(expressions), groups); + } + + public static TopOperator top(PhysicalPlan input, List groups, + Expression... expressions) { + return new TopOperator(input, Arrays.asList(expressions), groups); + } + + public static TopOperator top(PhysicalPlan input, int noOfResults, List groups, + Expression... expressions) { + return new TopOperator(input, noOfResults, Arrays.asList(expressions), groups); + } + @SafeVarargs public ValuesOperator values(List... values) { return new ValuesOperator(Arrays.asList(values)); 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 9756b57cbb..3fc4685be6 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 @@ -68,4 +68,12 @@ public R visitValues(ValuesOperator node, C context) { public R visitSort(SortOperator node, C context) { return visitNode(node, context); } + + public R visitRare(RareOperator node, C context) { + return visitNode(node, context); + } + + public R visitTop(TopOperator node, C context) { + return visitNode(node, context); + } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java new file mode 100644 index 0000000000..b1e686d210 --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java @@ -0,0 +1,232 @@ +package com.amazon.opendistroforelasticsearch.sql.planner.physical; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.expression.Expression; +import com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple.BindingTuple; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.PriorityQueue; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.ToString; + +/** + * Group the all the input {@link BindingTuple} by {@link RareOperator#groupByExprList}, Calculate + * the rare result by using the {@link RareOperator#fieldExprList}. + */ +@EqualsAndHashCode +@ToString +public class RareOperator extends PhysicalPlan { + + @Getter + private final PhysicalPlan input; + @Getter + private final List fieldExprList; + @Getter + private final List groupByExprList; + + @EqualsAndHashCode.Exclude + private final Group group; + @EqualsAndHashCode.Exclude + private Iterator iterator; + + private static final Integer DEFAULT_NO_OF_RESULTS = 10; + + /** + * RareOperator Constructor. + * + * @param input Input {@link PhysicalPlan} + * @param fieldExprList List of {@link Expression} + * @param groupByExprList List of group by {@link Expression} + */ + public RareOperator(PhysicalPlan input, List fieldExprList, + List groupByExprList) { + this.input = input; + this.fieldExprList = fieldExprList; + this.groupByExprList = groupByExprList; + this.group = new Group(); + } + + @Override + public R accept(PhysicalPlanNodeVisitor visitor, C context) { + return visitor.visitRare(this, context); + } + + @Override + public List getChild() { + return Collections.singletonList(input); + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public ExprValue next() { + return iterator.next(); + } + + @Override + public void open() { + super.open(); + while (input.hasNext()) { + group.push(input.next()); + } + iterator = group.result().iterator(); + } + + @VisibleForTesting + @RequiredArgsConstructor + public class Group { + + private final Map>> groupListMap = new HashMap<>(); + + /** + * Push the BindingTuple to Group. Two functions will be applied to each BindingTuple to + * generate the {@link GroupKey} and {@link FieldKey} GroupKey = GroupKey(bindingTuple), + * FieldKey = FieldKey(bindingTuple) + */ + public void push(ExprValue inputValue) { + GroupKey groupKey = new GroupKey(inputValue); + FieldKey fieldKey = new FieldKey(inputValue); + groupListMap.computeIfAbsent(groupKey, k -> { + List> list = new ArrayList<>(); + HashMap map = new HashMap<>(); + map.put(fieldKey, 1); + list.add(map); + return list; + }); + groupListMap.computeIfPresent(groupKey, (key, fieldList) -> { + fieldList.forEach(map -> { + map.computeIfAbsent(fieldKey, f -> 1); + map.computeIfPresent(fieldKey, (field, count) -> { + return count + 1; + }); + }); + return fieldList; + }); + } + + /** + * Get the list of {@link BindingTuple} for each group. + */ + public List result() { + ImmutableList.Builder resultBuilder = new ImmutableList.Builder<>(); + + groupListMap.forEach((groups, list) -> { + LinkedHashMap map = new LinkedHashMap<>(); + list.forEach(fieldMap -> { + List rareList = findRare(fieldMap); + rareList.forEach(rareField -> { + map.putAll(groups.groupKeyMap()); + map.putAll(rareField.fieldKeyMap()); + resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); + }); + }); + }); + + return resultBuilder.build(); + } + + /** + * Get a list of rare values. + */ + public List findRare(HashMap map) { + PriorityQueue rareQueue = new PriorityQueue<>(new Comparator() { + @Override + public int compare(FieldKey e1, FieldKey e2) { + return map.get(e2) - map.get(e1); + } + }); + + for (Map.Entry entry : map.entrySet()) { + rareQueue.add(entry.getKey()); + if (rareQueue.size() > DEFAULT_NO_OF_RESULTS) { + rareQueue.poll(); + } + } + + List rareList = new ArrayList<>(); + while (!rareQueue.isEmpty()) { + rareList.add(rareQueue.poll()); + } + + Collections.reverse(rareList); + return rareList; + } + } + + /** + * Field Key. + */ + @EqualsAndHashCode + @VisibleForTesting + public class FieldKey { + + private final List fieldByValueList; + + /** + * FieldKey constructor. + */ + public FieldKey(ExprValue value) { + this.fieldByValueList = new ArrayList<>(); + for (Expression fieldExpr : fieldExprList) { + this.fieldByValueList.add(fieldExpr.valueOf(value.bindingTuples())); + } + } + + /** + * Return the Map of field and field value. + */ + public LinkedHashMap fieldKeyMap() { + LinkedHashMap map = new LinkedHashMap<>(); + for (int i = 0; i < fieldExprList.size(); i++) { + map.put(fieldExprList.get(i).toString(), fieldByValueList.get(i)); + } + return map; + } + } + + /** + * Group Key. + */ + @EqualsAndHashCode + @VisibleForTesting + public class GroupKey { + + private final List groupByValueList; + + /** + * GroupKey constructor. + */ + public GroupKey(ExprValue value) { + this.groupByValueList = new ArrayList<>(); + for (Expression groupExpr : groupByExprList) { + this.groupByValueList.add(groupExpr.valueOf(value.bindingTuples())); + } + } + + /** + * Return the Map of group field and group field value. + */ + public LinkedHashMap groupKeyMap() { + LinkedHashMap map = new LinkedHashMap<>(); + for (int i = 0; i < groupByExprList.size(); i++) { + map.put(groupByExprList.get(i).toString(), groupByValueList.get(i)); + } + return map; + } + } + +} \ No newline at end of file diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java new file mode 100644 index 0000000000..dfed66287d --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java @@ -0,0 +1,239 @@ +package com.amazon.opendistroforelasticsearch.sql.planner.physical; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.expression.Expression; +import com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple.BindingTuple; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.PriorityQueue; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.RequiredArgsConstructor; + +/** + * Group the all the input {@link BindingTuple} by {@link TopOperator#groupByExprList}, Calculate + * the rare result by using the {@link TopOperator#fieldExprList}. + */ +public class TopOperator extends PhysicalPlan { + + @Getter + private final PhysicalPlan input; + @Getter + private final Integer noOfResults; + @Getter + private final List fieldExprList; + @Getter + private final List groupByExprList; + + @EqualsAndHashCode.Exclude + private final Group group; + @EqualsAndHashCode.Exclude + private Iterator iterator; + + private static final Integer DEFAULT_NO_OF_RESULTS = 10; + + + public TopOperator(PhysicalPlan input, List fieldExprList, + List groupByExprList) { + this(input, DEFAULT_NO_OF_RESULTS, fieldExprList, groupByExprList); + } + + /** + * TopOperator Constructor. + * + * @param input Input {@link PhysicalPlan} + * @param noOfResults Number of results + * @param fieldExprList List of {@link Expression} + * @param groupByExprList List of group by {@link Expression} + */ + public TopOperator(PhysicalPlan input, int noOfResults, List fieldExprList, + List groupByExprList) { + this.input = input; + this.noOfResults = noOfResults; + this.fieldExprList = fieldExprList; + this.groupByExprList = groupByExprList; + this.group = new Group(); + } + + @Override + public R accept(PhysicalPlanNodeVisitor visitor, C context) { + return visitor.visitTop(this, context); + } + + @Override + public List getChild() { + return Collections.singletonList(input); + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public ExprValue next() { + return iterator.next(); + } + + @Override + public void open() { + super.open(); + while (input.hasNext()) { + group.push(input.next()); + } + iterator = group.result().iterator(); + } + + @VisibleForTesting + @RequiredArgsConstructor + public class Group { + + private final Map>> groupListMap = new HashMap<>(); + + /** + * Push the BindingTuple to Group. Two functions will be applied to each BindingTuple to + * generate the {@link GroupKey} and {@link FieldKey} GroupKey = GroupKey(bindingTuple), + * FieldKey = FieldKey(bindingTuple) + */ + public void push(ExprValue inputValue) { + GroupKey groupKey = new GroupKey(inputValue); + FieldKey fieldKey = new FieldKey(inputValue); + groupListMap.computeIfAbsent(groupKey, k -> { + List> list = new ArrayList<>(); + HashMap map = new HashMap<>(); + map.put(fieldKey, 1); + list.add(map); + return list; + }); + groupListMap.computeIfPresent(groupKey, (key, fieldList) -> { + fieldList.forEach(map -> { + map.computeIfAbsent(fieldKey, f -> 1); + map.computeIfPresent(fieldKey, (field, count) -> { + return count + 1; + }); + }); + return fieldList; + }); + } + + /** + * Get the list of {@link BindingTuple} for each group. + */ + public List result() { + ImmutableList.Builder resultBuilder = new ImmutableList.Builder<>(); + + groupListMap.forEach((groups, list) -> { + LinkedHashMap map = new LinkedHashMap<>(); + list.forEach(fieldMap -> { + List topList = findTop(fieldMap); + topList.forEach(topField -> { + map.putAll(groups.groupKeyMap()); + map.putAll(topField.fieldKeyMap()); + resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); + }); + }); + }); + + return resultBuilder.build(); + } + + /** + * Get a list of top values. + */ + public List findTop(HashMap map) { + PriorityQueue topQueue = new PriorityQueue<>(new Comparator() { + @Override + public int compare(FieldKey e1, FieldKey e2) { + return map.get(e1) - map.get(e2); + } + }); + + for (Map.Entry entry : map.entrySet()) { + topQueue.add(entry.getKey()); + if (topQueue.size() > noOfResults) { + topQueue.poll(); + } + } + + List topList = new ArrayList<>(); + while (!topQueue.isEmpty()) { + topList.add(topQueue.poll()); + } + + Collections.reverse(topList); + return topList; + } + } + + /** + * Field Key. + */ + @EqualsAndHashCode + @VisibleForTesting + public class FieldKey { + + private final List fieldByValueList; + + /** + * FieldKey constructor. + */ + public FieldKey(ExprValue value) { + this.fieldByValueList = new ArrayList<>(); + for (Expression fieldExpr : fieldExprList) { + this.fieldByValueList.add(fieldExpr.valueOf(value.bindingTuples())); + } + } + + /** + * Return the Map of field and field value. + */ + public LinkedHashMap fieldKeyMap() { + LinkedHashMap map = new LinkedHashMap<>(); + for (int i = 0; i < fieldExprList.size(); i++) { + map.put(fieldExprList.get(i).toString(), fieldByValueList.get(i)); + } + return map; + } + } + + /** + * Group Key. + */ + @EqualsAndHashCode + @VisibleForTesting + public class GroupKey { + + private final List groupByValueList; + + /** + * GroupKey constructor. + */ + public GroupKey(ExprValue value) { + this.groupByValueList = new ArrayList<>(); + for (Expression groupExpr : groupByExprList) { + this.groupByValueList.add(groupExpr.valueOf(value.bindingTuples())); + } + } + + /** + * Return the Map of group field and group field value. + */ + public LinkedHashMap groupKeyMap() { + LinkedHashMap map = new LinkedHashMap<>(); + for (int i = 0; i < groupByExprList.size(); i++) { + map.put(groupByExprList.get(i).toString(), groupByValueList.get(i)); + } + return map; + } + } + +} 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 6b8942b9b1..b543cd1039 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 @@ -24,9 +24,11 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RemoveOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RenameOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.SortOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.TopOperator; import com.amazon.opendistroforelasticsearch.sql.storage.TableScanOperator; import lombok.RequiredArgsConstructor; @@ -56,6 +58,18 @@ public PhysicalPlan visitAggregation(AggregationOperator node, Object context) { node.getGroupByExprList()); } + @Override + public PhysicalPlan visitRare(RareOperator node, Object context) { + return new RareOperator(visitInput(node.getInput(), context), node.getFieldExprList(), + node.getGroupByExprList()); + } + + @Override + public PhysicalPlan visitTop(TopOperator node, Object context) { + return new TopOperator(visitInput(node.getInput(), context), node.getNoOfResults(), + node.getFieldExprList(), node.getGroupByExprList()); + } + @Override public PhysicalPlan visitRename(RenameOperator node, Object context) { return new RenameOperator(visitInput(node.getInput(), context), node.getMapping()); From b4aa9151583114345f057b84d3f24f009bcfb034 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Wed, 26 Aug 2020 13:59:39 -0700 Subject: [PATCH 05/15] merge rare & top operator since both commands operate identical to each other --- .../sql/analysis/Analyzer.java | 11 +- .../sql/planner/DefaultImplementor.java | 20 +- .../sql/planner/logical/LogicalPlanDSL.java | 10 +- .../logical/LogicalPlanNodeVisitor.java | 6 +- .../sql/planner/logical/LogicalRare.java | 35 --- .../{LogicalTop.java => LogicalRareTopN.java} | 7 +- .../sql/planner/physical/PhysicalPlanDSL.java | 16 +- .../physical/PhysicalPlanNodeVisitor.java | 6 +- .../sql/planner/physical/RareOperator.java | 232 ------------------ ...TopOperator.java => RareTopNOperator.java} | 72 ++++-- .../ElasticsearchExecutionProtector.java | 13 +- 11 files changed, 82 insertions(+), 346 deletions(-) delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java rename core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/{LogicalTop.java => LogicalRareTopN.java} (82%) delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java rename core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/{TopOperator.java => RareTopNOperator.java} (75%) 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 6084f77f94..296507989b 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 @@ -53,12 +53,11 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalFilter; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRare; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRareTopN; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRename; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalSort; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalTop; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalValues; import com.amazon.opendistroforelasticsearch.sql.storage.StorageEngine; import com.amazon.opendistroforelasticsearch.sql.storage.Table; @@ -178,7 +177,7 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) { } /** - * Build {@link LogicalRare}. + * Build {@link LogicalRareTopN}. */ @Override public LogicalPlan visitRare(Rare node, AnalysisContext context) { @@ -204,11 +203,11 @@ public LogicalPlan visitRare(Rare node, AnalysisContext context) { fields.forEach(field -> newEnv.define(new Symbol(Namespace.FIELD_NAME, field.toString()), field.type())); - return new LogicalRare(child, fields, groupBys); + return new LogicalRareTopN(child, Boolean.FALSE, 10, fields, groupBys); } /** - * Build {@link LogicalTop}. + * Build {@link LogicalRareTopN}. */ @Override public LogicalPlan visitTop(Top node, AnalysisContext context) { @@ -237,7 +236,7 @@ public LogicalPlan visitTop(Top node, AnalysisContext context) { List options = node.getNoOfResults(); Integer noOfResults = (Integer) options.get(0).getValue().getValue(); - return new LogicalTop(child, noOfResults, fields, groupBys); + return new LogicalRareTopN(child, Boolean.TRUE, noOfResults, fields, groupBys); } /** 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 5f3b746960..a3a0980349 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 @@ -23,12 +23,11 @@ import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanNodeVisitor; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalProject; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRare; +import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRareTopN; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRelation; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRemove; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalRename; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalSort; -import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalTop; import com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalValues; import com.amazon.opendistroforelasticsearch.sql.planner.physical.AggregationOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.DedupeOperator; @@ -36,11 +35,10 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareTopNOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RemoveOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RenameOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.SortOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.TopOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ValuesOperator; /** @@ -56,18 +54,10 @@ public class DefaultImplementor extends LogicalPlanNodeVisitor { @Override - public PhysicalPlan visitRare(LogicalRare node, C context) { - return new RareOperator( - visitChild(node, context), - node.getFieldList(), - node.getGroupByList() - ); - } - - @Override - public PhysicalPlan visitTop(LogicalTop node, C context) { - return new TopOperator( + public PhysicalPlan visitRareTopN(LogicalRareTopN node, C context) { + return new RareTopNOperator( visitChild(node, context), + node.getRareTopFlag(), node.getNoOfResults(), node.getFieldList(), node.getGroupByList() 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 fa5c148d77..48c033d47b 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 @@ -84,14 +84,10 @@ public static LogicalPlan dedupe( input, Arrays.asList(fields), allowedDuplication, keepEmpty, consecutive); } - public static LogicalPlan rare(LogicalPlan input, List groupByList, + public static LogicalPlan rareTopN(LogicalPlan input, Boolean rareTopFlag, int noOfResults, + List groupByList, Expression... fields) { - return new LogicalRare(input, Arrays.asList(fields), groupByList); - } - - public static LogicalPlan top(LogicalPlan input, int noOfResults, List groupByList, - Expression... fields) { - return new LogicalTop(input, noOfResults, Arrays.asList(fields), groupByList); + return new LogicalRareTopN(input, rareTopFlag, noOfResults, Arrays.asList(fields), groupByList); } @SafeVarargs 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 012027e1d7..1b9ce5208f 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 @@ -67,11 +67,7 @@ public R visitValues(LogicalValues plan, C context) { return visitNode(plan, context); } - public R visitRare(LogicalRare plan, C context) { - return visitNode(plan, context); - } - - public R visitTop(LogicalTop plan, C context) { + public R visitRareTopN(LogicalRareTopN plan, C context) { return visitNode(plan, context); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java deleted file mode 100644 index 4dc7be5d70..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRare.java +++ /dev/null @@ -1,35 +0,0 @@ -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; - -/** - * Logical Rare Plan. - */ -@Getter -@ToString -@EqualsAndHashCode(callSuper = false) -@RequiredArgsConstructor -public class LogicalRare extends LogicalPlan { - - private final LogicalPlan child; - private final List fieldList; - @Getter - private final List groupByList; - - @Override - public List getChild() { - return Collections.singletonList(child); - } - - @Override - public R accept(LogicalPlanNodeVisitor visitor, C context) { - return visitor.visitRare(this, context); - } -} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java similarity index 82% rename from core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java rename to core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java index 1909a07582..2f9b5aba35 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalTop.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java @@ -10,15 +10,16 @@ import lombok.ToString; /** - * Logical Top Plan. + * Logical Rare and TopN Plan. */ @Getter @ToString @EqualsAndHashCode(callSuper = false) @RequiredArgsConstructor -public class LogicalTop extends LogicalPlan { +public class LogicalRareTopN extends LogicalPlan { private final LogicalPlan child; + private final Boolean rareTopFlag; private final Integer noOfResults; private final List fieldList; @Getter @@ -31,6 +32,6 @@ public List getChild() { @Override public R accept(LogicalPlanNodeVisitor visitor, C context) { - return visitor.visitTop(this, context); + return visitor.visitRareTopN(this, context); } } 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 efa262f1d3..1e2fcdbe4c 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 @@ -80,19 +80,17 @@ public static DedupeOperator dedupe( input, Arrays.asList(expressions), allowedDuplication, keepEmpty, consecutive); } - public static RareOperator rare(PhysicalPlan input, List groups, + public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, + List groups, Expression... expressions) { - return new RareOperator(input, Arrays.asList(expressions), groups); + return new RareTopNOperator(input, rareTopFlag, Arrays.asList(expressions), groups); } - public static TopOperator top(PhysicalPlan input, List groups, + public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, + List groups, Expression... expressions) { - return new TopOperator(input, Arrays.asList(expressions), groups); - } - - public static TopOperator top(PhysicalPlan input, int noOfResults, List groups, - Expression... expressions) { - return new TopOperator(input, noOfResults, Arrays.asList(expressions), groups); + return new RareTopNOperator(input, rareTopFlag, noOfResults, Arrays.asList(expressions), + groups); } @SafeVarargs 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 3fc4685be6..62b35af003 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 @@ -69,11 +69,7 @@ public R visitSort(SortOperator node, C context) { return visitNode(node, context); } - public R visitRare(RareOperator node, C context) { - return visitNode(node, context); - } - - public R visitTop(TopOperator node, C context) { + public R visitRareTopN(RareTopNOperator node, C context) { return visitNode(node, context); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java deleted file mode 100644 index b1e686d210..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareOperator.java +++ /dev/null @@ -1,232 +0,0 @@ -package com.amazon.opendistroforelasticsearch.sql.planner.physical; - -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.expression.Expression; -import com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple.BindingTuple; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.PriorityQueue; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import lombok.ToString; - -/** - * Group the all the input {@link BindingTuple} by {@link RareOperator#groupByExprList}, Calculate - * the rare result by using the {@link RareOperator#fieldExprList}. - */ -@EqualsAndHashCode -@ToString -public class RareOperator extends PhysicalPlan { - - @Getter - private final PhysicalPlan input; - @Getter - private final List fieldExprList; - @Getter - private final List groupByExprList; - - @EqualsAndHashCode.Exclude - private final Group group; - @EqualsAndHashCode.Exclude - private Iterator iterator; - - private static final Integer DEFAULT_NO_OF_RESULTS = 10; - - /** - * RareOperator Constructor. - * - * @param input Input {@link PhysicalPlan} - * @param fieldExprList List of {@link Expression} - * @param groupByExprList List of group by {@link Expression} - */ - public RareOperator(PhysicalPlan input, List fieldExprList, - List groupByExprList) { - this.input = input; - this.fieldExprList = fieldExprList; - this.groupByExprList = groupByExprList; - this.group = new Group(); - } - - @Override - public R accept(PhysicalPlanNodeVisitor visitor, C context) { - return visitor.visitRare(this, context); - } - - @Override - public List getChild() { - return Collections.singletonList(input); - } - - @Override - public boolean hasNext() { - return iterator.hasNext(); - } - - @Override - public ExprValue next() { - return iterator.next(); - } - - @Override - public void open() { - super.open(); - while (input.hasNext()) { - group.push(input.next()); - } - iterator = group.result().iterator(); - } - - @VisibleForTesting - @RequiredArgsConstructor - public class Group { - - private final Map>> groupListMap = new HashMap<>(); - - /** - * Push the BindingTuple to Group. Two functions will be applied to each BindingTuple to - * generate the {@link GroupKey} and {@link FieldKey} GroupKey = GroupKey(bindingTuple), - * FieldKey = FieldKey(bindingTuple) - */ - public void push(ExprValue inputValue) { - GroupKey groupKey = new GroupKey(inputValue); - FieldKey fieldKey = new FieldKey(inputValue); - groupListMap.computeIfAbsent(groupKey, k -> { - List> list = new ArrayList<>(); - HashMap map = new HashMap<>(); - map.put(fieldKey, 1); - list.add(map); - return list; - }); - groupListMap.computeIfPresent(groupKey, (key, fieldList) -> { - fieldList.forEach(map -> { - map.computeIfAbsent(fieldKey, f -> 1); - map.computeIfPresent(fieldKey, (field, count) -> { - return count + 1; - }); - }); - return fieldList; - }); - } - - /** - * Get the list of {@link BindingTuple} for each group. - */ - public List result() { - ImmutableList.Builder resultBuilder = new ImmutableList.Builder<>(); - - groupListMap.forEach((groups, list) -> { - LinkedHashMap map = new LinkedHashMap<>(); - list.forEach(fieldMap -> { - List rareList = findRare(fieldMap); - rareList.forEach(rareField -> { - map.putAll(groups.groupKeyMap()); - map.putAll(rareField.fieldKeyMap()); - resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); - }); - }); - }); - - return resultBuilder.build(); - } - - /** - * Get a list of rare values. - */ - public List findRare(HashMap map) { - PriorityQueue rareQueue = new PriorityQueue<>(new Comparator() { - @Override - public int compare(FieldKey e1, FieldKey e2) { - return map.get(e2) - map.get(e1); - } - }); - - for (Map.Entry entry : map.entrySet()) { - rareQueue.add(entry.getKey()); - if (rareQueue.size() > DEFAULT_NO_OF_RESULTS) { - rareQueue.poll(); - } - } - - List rareList = new ArrayList<>(); - while (!rareQueue.isEmpty()) { - rareList.add(rareQueue.poll()); - } - - Collections.reverse(rareList); - return rareList; - } - } - - /** - * Field Key. - */ - @EqualsAndHashCode - @VisibleForTesting - public class FieldKey { - - private final List fieldByValueList; - - /** - * FieldKey constructor. - */ - public FieldKey(ExprValue value) { - this.fieldByValueList = new ArrayList<>(); - for (Expression fieldExpr : fieldExprList) { - this.fieldByValueList.add(fieldExpr.valueOf(value.bindingTuples())); - } - } - - /** - * Return the Map of field and field value. - */ - public LinkedHashMap fieldKeyMap() { - LinkedHashMap map = new LinkedHashMap<>(); - for (int i = 0; i < fieldExprList.size(); i++) { - map.put(fieldExprList.get(i).toString(), fieldByValueList.get(i)); - } - return map; - } - } - - /** - * Group Key. - */ - @EqualsAndHashCode - @VisibleForTesting - public class GroupKey { - - private final List groupByValueList; - - /** - * GroupKey constructor. - */ - public GroupKey(ExprValue value) { - this.groupByValueList = new ArrayList<>(); - for (Expression groupExpr : groupByExprList) { - this.groupByValueList.add(groupExpr.valueOf(value.bindingTuples())); - } - } - - /** - * Return the Map of group field and group field value. - */ - public LinkedHashMap groupKeyMap() { - LinkedHashMap map = new LinkedHashMap<>(); - for (int i = 0; i < groupByExprList.size(); i++) { - map.put(groupByExprList.get(i).toString(), groupByValueList.get(i)); - } - return map; - } - } - -} \ No newline at end of file diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java similarity index 75% rename from core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java rename to core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index dfed66287d..271bab2e1e 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/TopOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -20,14 +20,16 @@ import lombok.RequiredArgsConstructor; /** - * Group the all the input {@link BindingTuple} by {@link TopOperator#groupByExprList}, Calculate - * the rare result by using the {@link TopOperator#fieldExprList}. + * Group the all the input {@link BindingTuple} by {@link RareTopNOperator#groupByExprList}, + * Calculate the rare result by using the {@link RareTopNOperator#fieldExprList}. */ -public class TopOperator extends PhysicalPlan { +public class RareTopNOperator extends PhysicalPlan { @Getter private final PhysicalPlan input; @Getter + private final Boolean rareTopFlag; + @Getter private final Integer noOfResults; @Getter private final List fieldExprList; @@ -42,22 +44,25 @@ public class TopOperator extends PhysicalPlan { private static final Integer DEFAULT_NO_OF_RESULTS = 10; - public TopOperator(PhysicalPlan input, List fieldExprList, + public RareTopNOperator(PhysicalPlan input, Boolean rareTopFlag, List fieldExprList, List groupByExprList) { - this(input, DEFAULT_NO_OF_RESULTS, fieldExprList, groupByExprList); + this(input, rareTopFlag, DEFAULT_NO_OF_RESULTS, fieldExprList, groupByExprList); } /** - * TopOperator Constructor. + * RareTopNOperator Constructor. * * @param input Input {@link PhysicalPlan} + * @param rareTopFlag Flag for Rare/TopN command. * @param noOfResults Number of results * @param fieldExprList List of {@link Expression} * @param groupByExprList List of group by {@link Expression} */ - public TopOperator(PhysicalPlan input, int noOfResults, List fieldExprList, + public RareTopNOperator(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, + List fieldExprList, List groupByExprList) { this.input = input; + this.rareTopFlag = rareTopFlag; this.noOfResults = noOfResults; this.fieldExprList = fieldExprList; this.groupByExprList = groupByExprList; @@ -66,7 +71,7 @@ public TopOperator(PhysicalPlan input, int noOfResults, List fieldEx @Override public R accept(PhysicalPlanNodeVisitor visitor, C context) { - return visitor.visitTop(this, context); + return visitor.visitRareTopN(this, context); } @Override @@ -134,10 +139,16 @@ public List result() { groupListMap.forEach((groups, list) -> { LinkedHashMap map = new LinkedHashMap<>(); list.forEach(fieldMap -> { - List topList = findTop(fieldMap); - topList.forEach(topField -> { + + List resultlist = new ArrayList<>(); + if (rareTopFlag) { + resultlist = findTop(fieldMap); + } else { + resultlist = findRare(fieldMap); + } + resultlist.forEach(field -> { map.putAll(groups.groupKeyMap()); - map.putAll(topField.fieldKeyMap()); + map.putAll(field.fieldKeyMap()); resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); }); }); @@ -157,21 +168,44 @@ public int compare(FieldKey e1, FieldKey e2) { } }); + return getList(map, topQueue, noOfResults); + } + + /** + * Get a list of rare values. + */ + public List findRare(HashMap map) { + PriorityQueue rareQueue = new PriorityQueue<>(new Comparator() { + @Override + public int compare(FieldKey e1, FieldKey e2) { + return map.get(e2) - map.get(e1); + } + }); + + return getList(map, rareQueue, noOfResults); + } + + /** + * Get a list of result. + */ + public List getList(HashMap map, PriorityQueue queue, + Integer size) { for (Map.Entry entry : map.entrySet()) { - topQueue.add(entry.getKey()); - if (topQueue.size() > noOfResults) { - topQueue.poll(); + queue.add(entry.getKey()); + if (queue.size() > size) { + queue.poll(); } } - List topList = new ArrayList<>(); - while (!topQueue.isEmpty()) { - topList.add(topQueue.poll()); + List list = new ArrayList<>(); + while (!queue.isEmpty()) { + list.add(queue.poll()); } - Collections.reverse(topList); - return topList; + Collections.reverse(list); + return list; } + } /** 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 b543cd1039..9ba9eb01ac 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 @@ -24,11 +24,10 @@ import com.amazon.opendistroforelasticsearch.sql.planner.physical.FilterOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.PhysicalPlan; import com.amazon.opendistroforelasticsearch.sql.planner.physical.ProjectOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareOperator; +import com.amazon.opendistroforelasticsearch.sql.planner.physical.RareTopNOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RemoveOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.RenameOperator; import com.amazon.opendistroforelasticsearch.sql.planner.physical.SortOperator; -import com.amazon.opendistroforelasticsearch.sql.planner.physical.TopOperator; import com.amazon.opendistroforelasticsearch.sql.storage.TableScanOperator; import lombok.RequiredArgsConstructor; @@ -59,14 +58,8 @@ public PhysicalPlan visitAggregation(AggregationOperator node, Object context) { } @Override - public PhysicalPlan visitRare(RareOperator node, Object context) { - return new RareOperator(visitInput(node.getInput(), context), node.getFieldExprList(), - node.getGroupByExprList()); - } - - @Override - public PhysicalPlan visitTop(TopOperator node, Object context) { - return new TopOperator(visitInput(node.getInput(), context), node.getNoOfResults(), + public PhysicalPlan visitRareTopN(RareTopNOperator node, Object context) { + return new RareTopNOperator(visitInput(node.getInput(), context), node.getRareTopFlag() , node.getNoOfResults(), node.getFieldExprList(), node.getGroupByExprList()); } From 372bf314472a53bfe7dcb0ef46bc41352e5acb9b Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Thu, 27 Aug 2020 19:55:19 -0700 Subject: [PATCH 06/15] add tests --- .../sql/analysis/Analyzer.java | 37 +------- .../sql/ast/AbstractNodeVisitor.java | 9 +- .../sql/ast/dsl/AstDSL.java | 18 ++-- .../sql/ast/tree/Rare.java | 53 ----------- .../sql/ast/tree/{Top.java => RareTopN.java} | 25 ++---- .../sql/planner/logical/LogicalPlanDSL.java | 10 ++- .../sql/planner/logical/LogicalRareTopN.java | 1 - .../sql/planner/physical/PhysicalPlanDSL.java | 8 +- .../planner/physical/RareTopNOperator.java | 4 +- .../sql/analysis/AnalyzerTest.java | 41 +++++++++ .../sql/planner/DefaultImplementorTest.java | 67 ++++++++------ .../logical/LogicalPlanNodeVisitorTest.java | 23 ++++- .../physical/PhysicalPlanNodeVisitorTest.java | 26 +++++- .../physical/RareTopNOperatorTest.java | 90 +++++++++++++++++++ .../ElasticsearchExecutionProtector.java | 4 +- .../ElasticsearchExecutionProtectorTest.java | 57 +++++++----- .../sql/ppl/RareCommandIT.java | 73 +++++++++++++++ .../sql/ppl/TopCommandIT.java | 62 +++++++++++++ .../sql/ppl/parser/AstBuilder.java | 63 +++++++------ .../sql/ppl/parser/AstBuilderTest.java | 27 ++++-- 20 files changed, 470 insertions(+), 228 deletions(-) delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java rename core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/{Top.java => RareTopN.java} (69%) create mode 100644 core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java create mode 100644 integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java create mode 100644 integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TopCommandIT.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 296507989b..3b3878da0c 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 @@ -31,12 +31,11 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprMissingValue; @@ -180,37 +179,7 @@ public LogicalPlan visitAggregation(Aggregation node, AnalysisContext context) { * Build {@link LogicalRareTopN}. */ @Override - public LogicalPlan visitRare(Rare node, AnalysisContext context) { - final LogicalPlan child = node.getChild().get(0).accept(this, context); - - ImmutableList.Builder groupbyBuilder = new ImmutableList.Builder<>(); - for (UnresolvedExpression expr : node.getGroupExprList()) { - groupbyBuilder.add(expressionAnalyzer.analyze(expr, context)); - } - ImmutableList groupBys = groupbyBuilder.build(); - - ImmutableList.Builder fieldsBuilder = new ImmutableList.Builder<>(); - for (Field f : node.getFields()) { - fieldsBuilder.add(expressionAnalyzer.analyze(f, context)); - } - ImmutableList fields = fieldsBuilder.build(); - - // new context - context.push(); - TypeEnvironment newEnv = context.peek(); - groupBys.forEach(group -> newEnv.define(new Symbol(Namespace.FIELD_NAME, - group.toString()), group.type())); - fields.forEach(field -> newEnv.define(new Symbol(Namespace.FIELD_NAME, - field.toString()), field.type())); - - return new LogicalRareTopN(child, Boolean.FALSE, 10, fields, groupBys); - } - - /** - * Build {@link LogicalRareTopN}. - */ - @Override - public LogicalPlan visitTop(Top node, AnalysisContext context) { + public LogicalPlan visitRareTopN(RareTopN node, AnalysisContext context) { final LogicalPlan child = node.getChild().get(0).accept(this, context); ImmutableList.Builder groupbyBuilder = new ImmutableList.Builder<>(); @@ -236,7 +205,7 @@ public LogicalPlan visitTop(Top node, AnalysisContext context) { List options = node.getNoOfResults(); Integer noOfResults = (Integer) options.get(0).getValue().getValue(); - return new LogicalRareTopN(child, Boolean.TRUE, noOfResults, fields, groupBys); + return new LogicalRareTopN(child, node.getRareTopFlag(), noOfResults, fields, groupBys); } /** diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java index 15613847d1..ac099b5f94 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java @@ -40,11 +40,10 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; /** @@ -180,11 +179,7 @@ public T visitDedupe(Dedupe node, C context) { return visitChildren(node, context); } - public T visitRare(Rare node, C context) { - return visitChildren(node, context); - } - - public T visitTop(Top node, C context) { + public T visitRareTopN(RareTopN node, C context) { return visitChildren(node, context); } 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 1faf428dd4..c8604d8332 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 @@ -26,7 +26,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function; import com.amazon.opendistroforelasticsearch.sql.ast.expression.In; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Interval; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.IntervalUnit; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Map; @@ -41,11 +40,10 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Values; import java.util.Arrays; @@ -299,14 +297,14 @@ public static Dedupe dedupe(UnresolvedPlan input, List options, Field. return new Dedupe(input, options, Arrays.asList(fields)); } - public static Rare rare( - UnresolvedPlan input, List groupList, Field... fields) { - return new Rare(Arrays.asList(fields), groupList).attach(input); + public static List defaultTopArgs() { + return exprList( + argument("noOfResults", intLiteral(10))); } - public static Top top( - UnresolvedPlan input, List options, - List groupList, Field... fields) { - return new Top(options, Arrays.asList(fields), groupList).attach(input); + public static RareTopN rareTopN(UnresolvedPlan input, Boolean rareTopFlag, + List noOfResults, List groupList, Field... fields) { + return new RareTopN(input, rareTopFlag, noOfResults, Arrays.asList(fields), groupList) + .attach(input); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java deleted file mode 100644 index 3f9a95b850..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Rare.java +++ /dev/null @@ -1,53 +0,0 @@ -package com.amazon.opendistroforelasticsearch.sql.ast.tree; - -import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; -import com.google.common.collect.ImmutableList; - -import java.util.List; - -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.NonNull; -import lombok.Setter; -import lombok.ToString; - -/** - * AST node represent Rare operation. - */ -@Getter -@Setter -@ToString -@EqualsAndHashCode(callSuper = false) -public class Rare extends UnresolvedPlan { - - private UnresolvedPlan child; - private final List fields; - private final List groupExprList; - - /** - * Rare Constructor. - */ - @NonNull - public Rare(List fields, List groupExprList) { - this.fields = fields; - this.groupExprList = groupExprList; - } - - @Override - public Rare attach(UnresolvedPlan child) { - this.child = child; - return this; - } - - @Override - public List getChild() { - return ImmutableList.of(this.child); - } - - @Override - public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitRare(this, context); - } -} diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java similarity index 69% rename from core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java rename to core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java index 325de04751..eba5545543 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/Top.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java @@ -6,39 +6,32 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; import com.google.common.collect.ImmutableList; import java.util.List; +import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; -import lombok.NonNull; +import lombok.RequiredArgsConstructor; import lombok.Setter; import lombok.ToString; /** - * AST node represent Top operation. + * AST node represent RareTopN operation. */ @Getter @Setter @ToString @EqualsAndHashCode(callSuper = false) -public class Top extends UnresolvedPlan { +@RequiredArgsConstructor +@AllArgsConstructor +public class RareTopN extends UnresolvedPlan { private UnresolvedPlan child; + private final Boolean rareTopFlag; private final List noOfResults; private final List fields; private final List groupExprList; - /** - * Top Constructors. - */ - @NonNull - public Top( - List noOfResults, List fields, List groupExprList) { - this.noOfResults = noOfResults; - this.fields = fields; - this.groupExprList = groupExprList; - } - @Override - public Top attach(UnresolvedPlan child) { + public RareTopN attach(UnresolvedPlan child) { this.child = child; return this; } @@ -50,7 +43,7 @@ public List getChild() { @Override public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitTop(this, context); + return nodeVisitor.visitRareTopN(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 48c033d47b..162fbe761c 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 @@ -84,9 +84,13 @@ public static LogicalPlan dedupe( input, Arrays.asList(fields), allowedDuplication, keepEmpty, consecutive); } - public static LogicalPlan rareTopN(LogicalPlan input, Boolean rareTopFlag, int noOfResults, - List groupByList, - Expression... fields) { + public static LogicalPlan rareTopN(LogicalPlan input, Boolean rareTopFlag, + List groupByList, Expression... fields) { + return rareTopN(input, rareTopFlag, 10, groupByList, fields); + } + + public static LogicalPlan rareTopN(LogicalPlan input, boolean rareTopFlag, int noOfResults, + List groupByList, Expression... fields) { return new LogicalRareTopN(input, rareTopFlag, noOfResults, Arrays.asList(fields), groupByList); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java index 2f9b5aba35..456e1b9fe5 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java @@ -22,7 +22,6 @@ public class LogicalRareTopN extends LogicalPlan { private final Boolean rareTopFlag; private final Integer noOfResults; private final List fieldList; - @Getter private final List groupByList; @Override 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 1e2fcdbe4c..9da8a77d18 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 @@ -81,14 +81,12 @@ public static DedupeOperator dedupe( } public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, - List groups, - Expression... expressions) { - return new RareTopNOperator(input, rareTopFlag, Arrays.asList(expressions), groups); + List groups, Expression... expressions) { + return rareTopN(input, rareTopFlag, 10, groups, expressions); } public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, - List groups, - Expression... expressions) { + List groups, Expression... expressions) { return new RareTopNOperator(input, rareTopFlag, noOfResults, Arrays.asList(expressions), groups); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index 271bab2e1e..f0359eb66a 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -18,13 +18,15 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; +import lombok.ToString; /** * Group the all the input {@link BindingTuple} by {@link RareTopNOperator#groupByExprList}, * Calculate the rare result by using the {@link RareTopNOperator#fieldExprList}. */ +@ToString +@EqualsAndHashCode public class RareTopNOperator extends PhysicalPlan { - @Getter private final PhysicalPlan input; @Getter 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 df804cd262..1b62f653f0 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 @@ -48,6 +48,7 @@ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = {ExpressionConfig.class, AnalyzerTest.class}) class AnalyzerTest extends AnalyzerTestBase { + @Test public void filter_relation() { assertAnalyzeEqual( @@ -113,6 +114,46 @@ public void stats_source() { AstDSL.defaultStatsArgs())); } + @Test + public void rare_source() { + assertAnalyzeEqual( + LogicalPlanDSL.rareTopN( + LogicalPlanDSL.relation("schema"), + Boolean.FALSE, + 10, + ImmutableList.of(DSL.ref("string_value", STRING)), + DSL.ref("integer_value", INTEGER) + ), + AstDSL.rareTopN( + AstDSL.relation("schema"), + Boolean.FALSE, + ImmutableList.of(argument("noOfResults", intLiteral(10))), + ImmutableList.of(field("string_value")), + field("integer_value") + ) + ); + } + + @Test + public void top_source() { + assertAnalyzeEqual( + LogicalPlanDSL.rareTopN( + LogicalPlanDSL.relation("schema"), + Boolean.TRUE, + 5, + ImmutableList.of(DSL.ref("string_value", STRING)), + DSL.ref("integer_value", INTEGER) + ), + AstDSL.rareTopN( + AstDSL.relation("schema"), + Boolean.TRUE, + ImmutableList.of(argument("noOfResults", intLiteral(5))), + ImmutableList.of(field("string_value")), + field("integer_value") + ) + ); + } + @Test public void rename_to_invalid_expression() { SemanticCheckException exception = 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 427883c1d9..f250bb00fc 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 @@ -25,6 +25,7 @@ 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.project; +import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.rareTopN; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.remove; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.rename; import static com.amazon.opendistroforelasticsearch.sql.planner.logical.LogicalPlanDSL.sort; @@ -66,6 +67,8 @@ public void visitShouldReturnDefaultPhysicalOperator() { ReferenceExpression dedupeField = ref("name", STRING); Expression filterExpr = literal(ExprBooleanValue.of(true)); List groupByExprs = Arrays.asList(ref("age", INTEGER)); + ReferenceExpression rareTopNField = ref("age", INTEGER); + Boolean topFlag = Boolean.TRUE; List aggregators = Arrays.asList(new AvgAggregator(groupByExprs, ExprCoreType.DOUBLE)); Map mappings = @@ -79,19 +82,23 @@ public void visitShouldReturnDefaultPhysicalOperator() { LogicalPlan plan = project( LogicalPlanDSL.dedupe( - sort( - eval( - remove( - rename( - aggregation( - filter(values(emptyList()), filterExpr), - aggregators, - groupByExprs), - mappings), - exclude), - newEvalField), - sortCount, - sortField), + rareTopN( + sort( + eval( + remove( + rename( + aggregation( + filter(values(emptyList()), filterExpr), + aggregators, + groupByExprs), + mappings), + exclude), + newEvalField), + sortCount, + sortField), + topFlag, + groupByExprs, + rareTopNField), dedupeField), include); @@ -100,21 +107,25 @@ public void visitShouldReturnDefaultPhysicalOperator() { assertEquals( PhysicalPlanDSL.project( PhysicalPlanDSL.dedupe( - PhysicalPlanDSL.sort( - PhysicalPlanDSL.eval( - PhysicalPlanDSL.remove( - PhysicalPlanDSL.rename( - PhysicalPlanDSL.agg( - PhysicalPlanDSL.filter( - PhysicalPlanDSL.values(emptyList()), - filterExpr), - aggregators, - groupByExprs), - mappings), - exclude), - newEvalField), - sortCount, - sortField), + PhysicalPlanDSL.rareTopN( + PhysicalPlanDSL.sort( + PhysicalPlanDSL.eval( + PhysicalPlanDSL.remove( + PhysicalPlanDSL.rename( + PhysicalPlanDSL.agg( + PhysicalPlanDSL.filter( + PhysicalPlanDSL.values(emptyList()), + filterExpr), + aggregators, + groupByExprs), + mappings), + exclude), + newEvalField), + sortCount, + sortField), + topFlag, + groupByExprs, + rareTopNField), dedupeField), include), actual); 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 fd360850c6..06aca69de7 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 @@ -37,6 +37,7 @@ */ @ExtendWith(MockitoExtension.class) class LogicalPlanNodeVisitorTest { + @Mock Expression expression; @Mock @@ -49,13 +50,17 @@ public void logicalPlanShouldTraversable() { LogicalPlan logicalPlan = LogicalPlanDSL.rename( LogicalPlanDSL.aggregation( - LogicalPlanDSL.filter(LogicalPlanDSL.relation("schema"), expression), + LogicalPlanDSL.rareTopN( + LogicalPlanDSL.filter(LogicalPlanDSL.relation("schema"), expression), + true, + ImmutableList.of(expression), + expression), ImmutableList.of(aggregator), ImmutableList.of(expression)), ImmutableMap.of(ref, ref)); Integer result = logicalPlan.accept(new NodesCount(), null); - assertEquals(4, result); + assertEquals(5, result); } @Test @@ -97,9 +102,15 @@ public void testAbstractPlanNodeVisitorShouldReturnNull() { LogicalPlan dedup = LogicalPlanDSL.dedupe(relation, 1, false, false, expression); assertNull(dedup.accept(new LogicalPlanNodeVisitor() { }, null)); + + LogicalPlan rareTopN = LogicalPlanDSL.rareTopN( + relation, true, ImmutableList.of(expression), expression); + assertNull(rareTopN.accept(new LogicalPlanNodeVisitor() { + }, null)); } private static class NodesCount extends LogicalPlanNodeVisitor { + @Override public Integer visitRelation(LogicalRelation plan, Object context) { return 1; @@ -128,5 +139,13 @@ public Integer visitRename(LogicalRename plan, Object context) { .map(child -> child.accept(this, context)) .collect(Collectors.summingInt(Integer::intValue)); } + + @Override + public Integer visitRareTopN(LogicalRareTopN plan, Object context) { + return 1 + + plan.getChild().stream() + .map(child -> child.accept(this, context)) + .collect(Collectors.summingInt(Integer::intValue)); + } } } 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 2feaf09f7f..788e5411ea 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 @@ -21,6 +21,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.In; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; @@ -39,6 +41,7 @@ */ @ExtendWith(MockitoExtension.class) class PhysicalPlanNodeVisitorTest extends PhysicalPlanTestBase { + @Mock PhysicalPlan plan; @Mock @@ -51,9 +54,13 @@ public void print_physical_plan() { PhysicalPlanDSL.project( PhysicalPlanDSL.rename( PhysicalPlanDSL.agg( - PhysicalPlanDSL.filter( - new TestScan(), - dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10))), + PhysicalPlanDSL.rareTopN( + PhysicalPlanDSL.filter( + new TestScan(), + dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10))), + true, + ImmutableList.of(), + DSL.ref("response", INTEGER)), ImmutableList.of(dsl.avg(DSL.ref("response", INTEGER))), ImmutableList.of()), ImmutableMap.of(DSL.ref("ivalue", INTEGER), DSL.ref("avg(response)", DOUBLE))), @@ -66,7 +73,8 @@ public void print_physical_plan() { + "\tProject->\n" + "\t\tRename->\n" + "\t\t\tAggregation->\n" - + "\t\t\t\tFilter->", + + "\t\t\t\tRareTopN->\n" + + "\t\t\t\t\tFilter->", printer.print(plan)); } @@ -114,6 +122,11 @@ public void test_PhysicalPlanVisitor_should_return_null() { PhysicalPlan values = PhysicalPlanDSL.values(Collections.emptyList()); assertNull(values.accept(new PhysicalPlanNodeVisitor() { }, null)); + + PhysicalPlan rareTopN = + PhysicalPlanDSL.rareTopN(plan, true, 5, ImmutableList.of(), ref); + assertNull(rareTopN.accept(new PhysicalPlanNodeVisitor() { + }, null)); } public static class PhysicalPlanPrinter extends PhysicalPlanNodeVisitor { @@ -147,6 +160,11 @@ public String visitRemove(RemoveOperator node, Integer tabs) { return name(node, "Remove->", tabs); } + @Override + public String visitRareTopN(RareTopNOperator node, Integer tabs) { + return name(node, "RareTopN->", tabs); + } + private String name(PhysicalPlan node, String current, int tabs) { String child = node.getChild().get(0).accept(this, tabs + 1); StringBuilder sb = new StringBuilder(); diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java new file mode 100644 index 0000000000..362882168c --- /dev/null +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java @@ -0,0 +1,90 @@ +package com.amazon.opendistroforelasticsearch.sql.planner.physical; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; +import com.amazon.opendistroforelasticsearch.sql.expression.DSL; +import com.google.common.collect.ImmutableMap; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; + +public class RareTopNOperatorTest extends PhysicalPlanTestBase { + + @Test + public void rare_without_group() { + PhysicalPlan plan = new RareTopNOperator(new TestScan(), + Boolean.FALSE, + Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), + Collections.emptyList()); + List result = execute(plan); + assertEquals(2, result.size()); + assertThat(result, containsInAnyOrder( + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST")), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET")) + )); + } + + @Test + public void rare_with_group() { + PhysicalPlan plan = new RareTopNOperator(new TestScan(), + Boolean.FALSE, + Collections.singletonList(DSL.ref("response", ExprCoreType.INTEGER)), + Collections.singletonList(DSL.ref("action", ExprCoreType.STRING))); + List result = execute(plan); + assertEquals(4, result.size()); + assertThat(result, containsInAnyOrder( + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST", "response", 200)), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST", "response", 500)), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET", "response", 404)), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET", "response", 200)) + )); + } + + @Test + public void top_without_group() { + PhysicalPlan plan = new RareTopNOperator(new TestScan(), + Boolean.TRUE, + Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), + Collections.emptyList()); + List result = execute(plan); + assertEquals(2, result.size()); + assertThat(result, containsInAnyOrder( + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET")), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST")) + )); + } + + @Test + public void top_n_without_group() { + PhysicalPlan plan = new RareTopNOperator(new TestScan(), + Boolean.TRUE, + 1, + Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), + Collections.emptyList()); + List result = execute(plan); + assertEquals(1, result.size()); + assertThat(result, containsInAnyOrder( + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET")) + )); + } + + @Test + public void top_n_with_group() { + PhysicalPlan plan = new RareTopNOperator(new TestScan(), + Boolean.TRUE, + 1, + Collections.singletonList(DSL.ref("response", ExprCoreType.INTEGER)), + Collections.singletonList(DSL.ref("action", ExprCoreType.STRING))); + List result = execute(plan); + assertEquals(2, result.size()); + assertThat(result, containsInAnyOrder( + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST", "response", 500)), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET", "response", 200)) + )); + } +} 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 9ba9eb01ac..6c705ee085 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 @@ -59,8 +59,8 @@ public PhysicalPlan visitAggregation(AggregationOperator node, Object context) { @Override public PhysicalPlan visitRareTopN(RareTopNOperator node, Object context) { - return new RareTopNOperator(visitInput(node.getInput(), context), node.getRareTopFlag() , node.getNoOfResults(), - node.getFieldExprList(), node.getGroupByExprList()); + return new RareTopNOperator(visitInput(node.getInput(), context), node.getRareTopFlag(), + node.getNoOfResults(), node.getFieldExprList(), node.getGroupByExprList()); } @Override 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 a4225c5235..5fcfe62dee 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 @@ -58,6 +58,7 @@ @ExtendWith(MockitoExtension.class) class ElasticsearchExecutionProtectorTest { + @Mock private ElasticsearchClient client; @@ -99,36 +100,16 @@ public void testProtectIndexScan() { assertEquals( PhysicalPlanDSL.project( PhysicalPlanDSL.dedupe( - PhysicalPlanDSL.sort( - PhysicalPlanDSL.eval( - PhysicalPlanDSL.remove( - PhysicalPlanDSL.rename( - PhysicalPlanDSL.agg( - filter( - resourceMonitor( - new ElasticsearchIndexScan( - client, settings, indexName, exprValueFactory)), - filterExpr), - aggregators, - groupByExprs), - mappings), - exclude), - newEvalField), - sortCount, - sortField), - dedupeField), - include), - executionProtector.protect( - PhysicalPlanDSL.project( - PhysicalPlanDSL.dedupe( + PhysicalPlanDSL.rareTopN( PhysicalPlanDSL.sort( PhysicalPlanDSL.eval( PhysicalPlanDSL.remove( PhysicalPlanDSL.rename( PhysicalPlanDSL.agg( filter( - new ElasticsearchIndexScan( - client, settings, indexName, exprValueFactory), + resourceMonitor( + new ElasticsearchIndexScan( + client, settings, indexName, exprValueFactory)), filterExpr), aggregators, groupByExprs), @@ -137,6 +118,34 @@ public void testProtectIndexScan() { newEvalField), sortCount, sortField), + true, + groupByExprs, + dedupeField), + dedupeField), + include), + executionProtector.protect( + PhysicalPlanDSL.project( + PhysicalPlanDSL.dedupe( + PhysicalPlanDSL.rareTopN( + PhysicalPlanDSL.sort( + PhysicalPlanDSL.eval( + PhysicalPlanDSL.remove( + PhysicalPlanDSL.rename( + PhysicalPlanDSL.agg( + filter( + new ElasticsearchIndexScan( + client, settings, indexName, exprValueFactory), + filterExpr), + aggregators, + groupByExprs), + mappings), + exclude), + newEvalField), + sortCount, + sortField), + true, + groupByExprs, + dedupeField), dedupeField), include))); } diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java new file mode 100644 index 0000000000..91f22867b6 --- /dev/null +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java @@ -0,0 +1,73 @@ +/* + * 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.ppl; + +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.rows; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifyDataRows; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; + +public class RareCommandIT extends PPLIntegTestCase { + + @Override + public void init() throws IOException { + loadIndex(Index.ACCOUNT); + setQuerySizeLimit(2000); + } + + @Test + public void testRareWithoutGroup() throws IOException { + JSONObject result = + executeQuery(String.format("source=%s | rare gender", TEST_INDEX_ACCOUNT)); + verifyDataRows( + result, + rows("F"), + rows("M")); + } + + @Test + public void testRareWithGroup() throws IOException { + JSONObject result = + executeQuery(String.format("source=%s | rare state by gender", TEST_INDEX_ACCOUNT)); + verifyDataRows( + result, + rows("F", "DE"), + rows("F", "OR"), + rows("F", "WI"), + rows("F", "CT"), + rows("F", "SC"), + rows("F", "WA"), + rows("F", "KS"), + rows("F", "OK"), + rows("F", "CO"), + rows("F", "UT"), + rows("M", "MI"), + rows("M", "NE"), + rows("M", "RI"), + rows("M", "NV"), + rows("M", "SD"), + rows("M", "AZ"), + rows("M", "NM"), + rows("M", "MT"), + rows("M", "KY"), + rows("M", "IN")); + } + + +} diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TopCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TopCommandIT.java new file mode 100644 index 0000000000..0fce11a720 --- /dev/null +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/TopCommandIT.java @@ -0,0 +1,62 @@ +/* + * 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.ppl; + +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.rows; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifyDataRows; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; + +public class TopCommandIT extends PPLIntegTestCase{ + + @Override + public void init() throws IOException { + loadIndex(Index.ACCOUNT); + setQuerySizeLimit(2000); + } + + @Test + public void testTopWithoutGroup() throws IOException { + JSONObject result = + executeQuery(String.format("source=%s | top gender", TEST_INDEX_ACCOUNT)); + verifyDataRows( + result, + rows("M"), + rows("F")); + } + + @Test + public void testTopNWithoutGroup() throws IOException{ + JSONObject result = + executeQuery(String.format("source=%s | top 1 gender", TEST_INDEX_ACCOUNT)); + verifyDataRows( + result, + rows("M")); + } + + @Test + public void testTopNWithGroup() throws IOException { + JSONObject result = + executeQuery(String.format("source=%s | top 1 state by gender", TEST_INDEX_ACCOUNT)); + verifyDataRows( + result, + rows("F", "TX"), + rows("M", "MD")); + } +} 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 65c7bcefc7..669fe5c427 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 @@ -30,8 +30,11 @@ import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.TopCommandContext; import static com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.WhereCommandContext; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.DataType; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; 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.UnresolvedExpression; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Aggregation; @@ -39,11 +42,10 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Eval; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rare; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; -import com.amazon.opendistroforelasticsearch.sql.ast.tree.Top; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParserBaseVisitor; @@ -207,19 +209,21 @@ public UnresolvedPlan visitEvalCommand(EvalCommandContext ctx) { @Override public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : - ctx.byClause() - .fieldList() - .fieldExpression() - .stream() - .map(this::visitExpression) - .collect(Collectors.toList()); - return new Rare( - ctx.fieldList() - .fieldExpression() - .stream() - .map(field -> (Field) visitExpression(field)) - .collect(Collectors.toList()), - groupList + ctx.byClause() + .fieldList() + .fieldExpression() + .stream() + .map(this::visitExpression) + .collect(Collectors.toList()); + return new RareTopN( + Boolean.FALSE, + Collections.singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))), + ctx.fieldList() + .fieldExpression() + .stream() + .map(field -> (Field) visitExpression(field)) + .collect(Collectors.toList()), + groupList ); } @@ -229,20 +233,21 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { @Override public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : - ctx.byClause() - .fieldList() - .fieldExpression() - .stream() - .map(this::visitExpression) - .collect(Collectors.toList()); - return new Top( - ArgumentFactory.getArgumentList(ctx), - ctx.fieldList() - .fieldExpression() - .stream() - .map(field -> (Field) visitExpression(field)) - .collect(Collectors.toList()), - groupList + ctx.byClause() + .fieldList() + .fieldExpression() + .stream() + .map(this::visitExpression) + .collect(Collectors.toList()); + return new RareTopN( + Boolean.TRUE, + ArgumentFactory.getArgumentList(ctx), + ctx.fieldList() + .fieldExpression() + .stream() + .map(field -> (Field) visitExpression(field)) + .collect(Collectors.toList()), + groupList ); } 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 9063693a18..62df52cb8c 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 @@ -36,13 +36,12 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.map; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.nullLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.projectWithArg; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rare; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.rareTopN; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.relation; 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.sortOptions; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.top; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; @@ -306,8 +305,10 @@ public void testIndexName() { @Test public void testRareCommand() { assertEqual("source=t | rare a", - rare( + rareTopN( relation("t"), + false, + exprList(argument("noOfResults", intLiteral(10))), emptyList(), field("a") )); @@ -316,8 +317,10 @@ public void testRareCommand() { @Test public void testRareCommandWithGroupBy() { assertEqual("source=t | rare a by b", - rare( + rareTopN( relation("t"), + false, + exprList(argument("noOfResults", intLiteral(10))), exprList(field("b")), field("a") )); @@ -326,8 +329,10 @@ public void testRareCommandWithGroupBy() { @Test public void testRareCommandWithMultipleFields() { assertEqual("source=t | rare `a`, `b` by `c`", - rare( + rareTopN( relation("t"), + false, + exprList(argument("noOfResults", intLiteral(10))), exprList(field("c")), field("a"), field("b") @@ -337,8 +342,9 @@ public void testRareCommandWithMultipleFields() { @Test public void testTopCommandWithN() { assertEqual("source=t | top 1 a", - top( + rareTopN( relation("t"), + true, exprList(argument("noOfResults", intLiteral(1))), emptyList(), field("a") @@ -348,8 +354,9 @@ public void testTopCommandWithN() { @Test public void testTopCommandWithoutNAndGroupBy() { assertEqual("source=t | top a", - top( + rareTopN( relation("t"), + true, exprList(argument("noOfResults", intLiteral(10))), emptyList(), field("a") @@ -359,8 +366,9 @@ public void testTopCommandWithoutNAndGroupBy() { @Test public void testTopCommandWithNAndGroupBy() { assertEqual("source=t | top 1 a by b", - top( + rareTopN( relation("t"), + true, exprList(argument("noOfResults", intLiteral(1))), exprList(field("b")), field("a") @@ -370,8 +378,9 @@ public void testTopCommandWithNAndGroupBy() { @Test public void testTopCommandWithMultipleFields() { assertEqual("source=t | top 1 `a`, `b` by `c`", - top( + rareTopN( relation("t"), + true, exprList(argument("noOfResults", intLiteral(1))), exprList(field("c")), field("a"), From 01245a3a71dedac6d8bb636cab2e62700c401ef4 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Thu, 27 Aug 2020 21:09:31 -0700 Subject: [PATCH 07/15] update docs --- docs/experiment/ppl/cmd/rare.rst | 80 ++++++++++++++++++++++++++++++++ docs/experiment/ppl/cmd/top.rst | 74 +++++++++++++++++++++++++++++ docs/experiment/ppl/index.rst | 5 ++ 3 files changed, 159 insertions(+) create mode 100644 docs/experiment/ppl/cmd/rare.rst create mode 100644 docs/experiment/ppl/cmd/top.rst diff --git a/docs/experiment/ppl/cmd/rare.rst b/docs/experiment/ppl/cmd/rare.rst new file mode 100644 index 0000000000..08ed10df78 --- /dev/null +++ b/docs/experiment/ppl/cmd/rare.rst @@ -0,0 +1,80 @@ +============= +rare +============= + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 2 + + +Description +============ +| Using ``rare`` command to find the least common tuple of values of all fields in the field list. + + +Syntax +============ +rare [by-clause] + + +* field-list: mandatory. comma-delimited list of field names. +* by-clause: optional. one or more fields to group the results by. + + +Example 1: Find the least common values in a field +=========================================== + +The example finds least common gender of all the accounts. + +PPL query:: + + od> source=accounts | rare gender; + fetched rows / total rows = 2/2 + +------------+ + | gender | + |------------| + | F | + |------------| + | M | + +------------+ + + +Example 2: Find the least common values organized by gender +==================================================== + +The example finds least common age of all the accounts group by gender. + +PPL query:: + + od> source=accounts | rare age by gender; + fetched rows / total rows = 20/20 + +----------+----------+ + | gender | age | + |----------+----------| + | F | 29 | + | F | 20 | + | F | 23 | + | F | 25 | + | F | 37 | + | F | 38 | + | F | 40 | + | F | 36 | + | F | 27 | + | F | 24 | + | M | 27 | + | M | 38 | + | M | 34 | + | M | 24 | + | M | 28 | + | M | 30 | + | M | 21 | + | M | 39 | + | M | 37 | + | M | 29 | + | M | 25 | + | M | 33 | + +----------+----------+ + + diff --git a/docs/experiment/ppl/cmd/top.rst b/docs/experiment/ppl/cmd/top.rst new file mode 100644 index 0000000000..3f8e1aa27b --- /dev/null +++ b/docs/experiment/ppl/cmd/top.rst @@ -0,0 +1,74 @@ +============= +top +============= + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 2 + + +Description +============ +| Using ``top`` command to find the most frequent tuple of values of all fields in the field list. + + +Syntax +============ +top [N] [by-clause] + +* N: number of results to return. **Default**: 10 +* field-list: mandatory. comma-delimited list of field names. +* by-clause: optional. one or more fields to group the results by. + + +Example 1: Find the most common values in a field +=========================================== + +The example finds most common gender of all the accounts. + +PPL query:: + + od> source=accounts | top gender; + fetched rows / total rows = 2/2 + +------------+ + | gender | + |------------| + | M | + |------------| + | F | + +------------+ + +Example 2: Find the most common values in a field +=========================================== + +The example finds most common gender of all the accounts. + +PPL query:: + + od> source=accounts | top 1 gender; + fetched rows / total rows = 1/1 + +------------+ + | gender | + |------------| + | M | + +------------+ + +Example 2: Find the most common values organized by gender +==================================================== + +The example finds most common age of all the accounts group by gender. + +PPL query:: + + od> source=accounts | top 1 age by gender; + fetched rows / total rows = 2/2 + +----------+----------+ + | gender | age | + |----------+----------| + | F | 39 | + | M | 31 | + +----------+----------+ + + diff --git a/docs/experiment/ppl/index.rst b/docs/experiment/ppl/index.rst index 6645d9e255..fb2964ccd7 100644 --- a/docs/experiment/ppl/index.rst +++ b/docs/experiment/ppl/index.rst @@ -43,3 +43,8 @@ OpenDistro PPL Reference Manual - `where command `_ + - `rare command `_ + + - `top command `_ + + From c41319b976392d3394642d4b627404c80bb2c074 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Fri, 28 Aug 2020 10:29:44 -0700 Subject: [PATCH 08/15] add comments --- .../sql/planner/physical/RareTopNOperator.java | 5 +++++ docs/experiment/ppl/cmd/rare.rst | 12 +++++------- .../ElasticsearchExecutionProtectorTest.java | 5 +++-- .../sql/ppl/parser/AstBuilder.java | 12 ++++++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index f0359eb66a..4cf57f969f 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -29,6 +29,11 @@ public class RareTopNOperator extends PhysicalPlan { @Getter private final PhysicalPlan input; + + /** + * If rareTopFlag is true, result for top will be returned. + * If rareTopFlag is false, result for rare will be returned. + */ @Getter private final Boolean rareTopFlag; @Getter diff --git a/docs/experiment/ppl/cmd/rare.rst b/docs/experiment/ppl/cmd/rare.rst index 08ed10df78..f5b856fc2f 100644 --- a/docs/experiment/ppl/cmd/rare.rst +++ b/docs/experiment/ppl/cmd/rare.rst @@ -56,25 +56,23 @@ PPL query:: | F | 29 | | F | 20 | | F | 23 | - | F | 25 | | F | 37 | + | F | 25 | | F | 38 | - | F | 40 | | F | 36 | + | F | 40 | | F | 27 | | F | 24 | | M | 27 | - | M | 38 | - | M | 34 | | M | 24 | + | M | 34 | + | M | 38 | | M | 28 | | M | 30 | | M | 21 | | M | 39 | - | M | 37 | - | M | 29 | | M | 25 | - | M | 33 | + | M | 29 | +----------+----------+ 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 5fcfe62dee..e1d9d6900d 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 @@ -86,6 +86,7 @@ public void testProtectIndexScan() { NamedExpression include = named("age", ref("age", INTEGER)); ReferenceExpression exclude = ref("name", STRING); ReferenceExpression dedupeField = ref("name", STRING); + ReferenceExpression topField = ref("name", STRING); Expression filterExpr = literal(ExprBooleanValue.of(true)); List groupByExprs = Arrays.asList(ref("age", INTEGER)); List aggregators = Arrays.asList(new AvgAggregator(groupByExprs, DOUBLE)); @@ -120,7 +121,7 @@ public void testProtectIndexScan() { sortField), true, groupByExprs, - dedupeField), + topField), dedupeField), include), executionProtector.protect( @@ -145,7 +146,7 @@ public void testProtectIndexScan() { sortField), true, groupByExprs, - dedupeField), + topField), dedupeField), include))); } 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 669fe5c427..c9d5264807 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 @@ -216,7 +216,15 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { .map(this::visitExpression) .collect(Collectors.toList()); return new RareTopN( + + /** + * Setting rareTopFlag to FALSE will return list of rare values + */ Boolean.FALSE, + + /** + * Default number of results for rare is 10. + */ Collections.singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))), ctx.fieldList() .fieldExpression() @@ -240,6 +248,10 @@ public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { .map(this::visitExpression) .collect(Collectors.toList()); return new RareTopN( + + /** + * Setting rareTopFlag to TRUE will return list of top values + */ Boolean.TRUE, ArgumentFactory.getArgumentList(ctx), ctx.fieldList() From c9e4353428d8edbfe9f13e13b5e48516e21edef9 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Fri, 28 Aug 2020 16:34:56 -0700 Subject: [PATCH 09/15] addressing PR comments --- .../sql/ast/tree/RareTopN.java | 4 +- .../sql/planner/logical/LogicalRareTopN.java | 15 ++++ .../sql/planner/physical/PhysicalPlanDSL.java | 2 +- .../planner/physical/RareTopNOperator.java | 89 +++++++------------ .../physical/RareTopNOperatorTest.java | 2 +- docs/experiment/ppl/cmd/rare.rst | 8 +- .../sql/ppl/RareCommandIT.java | 14 +-- .../sql/ppl/parser/AstBuilder.java | 6 +- .../sql/ppl/utils/ArgumentFactory.java | 11 +++ 9 files changed, 75 insertions(+), 76 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java index eba5545543..87c1b236cf 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java @@ -4,7 +4,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Field; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; -import com.google.common.collect.ImmutableList; +import java.util.Collections; import java.util.List; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; @@ -38,7 +38,7 @@ public RareTopN attach(UnresolvedPlan child) { @Override public List getChild() { - return ImmutableList.of(this.child); + return Collections.singletonList(this.child); } @Override diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java index 456e1b9fe5..30a9eb4bad 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 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; 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 9da8a77d18..51fc791ed8 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 @@ -82,7 +82,7 @@ public static DedupeOperator dedupe( public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, List groups, Expression... expressions) { - return rareTopN(input, rareTopFlag, 10, groups, expressions); + return new RareTopNOperator(input, rareTopFlag, Arrays.asList(expressions), groups); } public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index 4cf57f969f..a7a95b58d6 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 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.ExprTupleValue; @@ -12,9 +27,11 @@ import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.PriorityQueue; +import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; @@ -27,6 +44,7 @@ @ToString @EqualsAndHashCode public class RareTopNOperator extends PhysicalPlan { + @Getter private final PhysicalPlan input; @@ -144,16 +162,10 @@ public List result() { ImmutableList.Builder resultBuilder = new ImmutableList.Builder<>(); groupListMap.forEach((groups, list) -> { - LinkedHashMap map = new LinkedHashMap<>(); + Map map = new LinkedHashMap<>(); list.forEach(fieldMap -> { - - List resultlist = new ArrayList<>(); - if (rareTopFlag) { - resultlist = findTop(fieldMap); - } else { - resultlist = findRare(fieldMap); - } - resultlist.forEach(field -> { + List result = find(fieldMap); + result.forEach(field -> { map.putAll(groups.groupKeyMap()); map.putAll(field.fieldKeyMap()); resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); @@ -164,55 +176,20 @@ public List result() { return resultBuilder.build(); } - /** - * Get a list of top values. - */ - public List findTop(HashMap map) { - PriorityQueue topQueue = new PriorityQueue<>(new Comparator() { - @Override - public int compare(FieldKey e1, FieldKey e2) { - return map.get(e1) - map.get(e2); - } - }); - - return getList(map, topQueue, noOfResults); - } - - /** - * Get a list of rare values. - */ - public List findRare(HashMap map) { - PriorityQueue rareQueue = new PriorityQueue<>(new Comparator() { - @Override - public int compare(FieldKey e1, FieldKey e2) { - return map.get(e2) - map.get(e1); - } - }); - - return getList(map, rareQueue, noOfResults); - } - /** * Get a list of result. */ - public List getList(HashMap map, PriorityQueue queue, - Integer size) { - for (Map.Entry entry : map.entrySet()) { - queue.add(entry.getKey()); - if (queue.size() > size) { - queue.poll(); - } - } - - List list = new ArrayList<>(); - while (!queue.isEmpty()) { - list.add(queue.poll()); + public List find(HashMap map) { + Comparator> valueComparator; + if (rareTopFlag) { + valueComparator = Map.Entry.comparingByValue(Comparator.reverseOrder()); + } else { + valueComparator = Map.Entry.comparingByValue(); } - Collections.reverse(list); - return list; + return map.entrySet().stream().sorted(valueComparator).limit(noOfResults) + .map(Map.Entry::getKey).collect(Collectors.toList()); } - } /** @@ -237,8 +214,8 @@ public FieldKey(ExprValue value) { /** * Return the Map of field and field value. */ - public LinkedHashMap fieldKeyMap() { - LinkedHashMap map = new LinkedHashMap<>(); + public Map fieldKeyMap() { + Map map = new LinkedHashMap<>(); for (int i = 0; i < fieldExprList.size(); i++) { map.put(fieldExprList.get(i).toString(), fieldByValueList.get(i)); } @@ -268,8 +245,8 @@ public GroupKey(ExprValue value) { /** * Return the Map of group field and group field value. */ - public LinkedHashMap groupKeyMap() { - LinkedHashMap map = new LinkedHashMap<>(); + public Map groupKeyMap() { + Map map = new LinkedHashMap<>(); for (int i = 0; i < groupByExprList.size(); i++) { map.put(groupByExprList.get(i).toString(), groupByValueList.get(i)); } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java index 362882168c..2f8196c120 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java @@ -83,7 +83,7 @@ public void top_n_with_group() { List result = execute(plan); assertEquals(2, result.size()); assertThat(result, containsInAnyOrder( - ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST", "response", 500)), + ExprValueUtils.tupleValue(ImmutableMap.of("action", "POST", "response", 200)), ExprValueUtils.tupleValue(ImmutableMap.of("action", "GET", "response", 200)) )); } diff --git a/docs/experiment/ppl/cmd/rare.rst b/docs/experiment/ppl/cmd/rare.rst index f5b856fc2f..67d2b515cf 100644 --- a/docs/experiment/ppl/cmd/rare.rst +++ b/docs/experiment/ppl/cmd/rare.rst @@ -56,21 +56,21 @@ PPL query:: | F | 29 | | F | 20 | | F | 23 | - | F | 37 | | F | 25 | + | F | 37 | | F | 38 | - | F | 36 | | F | 40 | | F | 27 | + | F | 36 | | F | 24 | | M | 27 | | M | 24 | | M | 34 | | M | 38 | | M | 28 | - | M | 30 | - | M | 21 | | M | 39 | + | M | 21 | + | M | 30 | | M | 25 | | M | 29 | +----------+----------+ diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java index 91f22867b6..ee30e972c5 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/RareCommandIT.java @@ -48,23 +48,23 @@ public void testRareWithGroup() throws IOException { verifyDataRows( result, rows("F", "DE"), - rows("F", "OR"), rows("F", "WI"), + rows("F", "OR"), rows("F", "CT"), - rows("F", "SC"), rows("F", "WA"), - rows("F", "KS"), + rows("F", "SC"), rows("F", "OK"), + rows("F", "KS"), rows("F", "CO"), - rows("F", "UT"), - rows("M", "MI"), + rows("F", "VA"), rows("M", "NE"), rows("M", "RI"), rows("M", "NV"), - rows("M", "SD"), + rows("M", "MI"), + rows("M", "MT"), rows("M", "AZ"), rows("M", "NM"), - rows("M", "MT"), + rows("M", "SD"), rows("M", "KY"), rows("M", "IN")); } 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 c9d5264807..f0bbe1b7e9 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 @@ -221,11 +221,7 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { * Setting rareTopFlag to FALSE will return list of rare values */ Boolean.FALSE, - - /** - * Default number of results for rare is 10. - */ - Collections.singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))), + ArgumentFactory.getArgumentList(ctx), ctx.fieldList() .fieldExpression() .stream() 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 26cf185ed7..fe9ac7a3cb 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 @@ -19,6 +19,7 @@ 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.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; @@ -150,6 +151,16 @@ public static List getArgumentList(TopCommandContext ctx) { ); } + /** + * Get list of {@link Argument}. + * + * @param ctx RareCommandContext instance + * @return the list of argument with default number of results for the rare command + */ + public static List getArgumentList(RareCommandContext ctx) { + return Collections.singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))); + } + private static Literal getArgumentValue(ParserRuleContext ctx) { return ctx instanceof IntegerLiteralContext ? new Literal(Integer.parseInt(ctx.getText()), DataType.INTEGER) From 8582d37c3946a510786be7c9ce8cc09a4ae0a08c Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Fri, 28 Aug 2020 16:47:59 -0700 Subject: [PATCH 10/15] fix build error --- .../amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java | 4 ++-- .../sql/ppl/utils/ArgumentFactory.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) 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 c8604d8332..34aeeb756e 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 @@ -26,6 +26,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function; import com.amazon.opendistroforelasticsearch.sql.ast.expression.In; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Interval; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.IntervalUnit; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Let; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Map; @@ -298,8 +299,7 @@ public static Dedupe dedupe(UnresolvedPlan input, List options, Field. } public static List defaultTopArgs() { - return exprList( - argument("noOfResults", intLiteral(10))); + return exprList(argument("noOfResults", intLiteral(10))); } public static RareTopN rareTopN(UnresolvedPlan input, Boolean rareTopFlag, 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 fe9ac7a3cb..abb525e06f 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 @@ -158,7 +158,8 @@ public static List getArgumentList(TopCommandContext ctx) { * @return the list of argument with default number of results for the rare command */ public static List getArgumentList(RareCommandContext ctx) { - return Collections.singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))); + return Collections + .singletonList(new Argument("noOfResults", new Literal(10, DataType.INTEGER))); } private static Literal getArgumentValue(ParserRuleContext ctx) { From a802b55e56f40655b03ca2325128a7f83fda7534 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Mon, 31 Aug 2020 13:09:55 -0700 Subject: [PATCH 11/15] address PR comments --- .../sql/ast/tree/RareTopN.java | 15 +++ .../planner/physical/RareTopNOperator.java | 91 ++++++------------- .../sql/ppl/parser/AstBuilder.java | 41 ++++----- 3 files changed, 63 insertions(+), 84 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java index 87c1b236cf..705cfde4fd 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 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.ast.tree; import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index a7a95b58d6..17b24985b8 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -21,16 +21,16 @@ import com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple.BindingTuple; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.PriorityQueue; import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -127,19 +127,17 @@ public void open() { @RequiredArgsConstructor public class Group { - private final Map>> groupListMap = new HashMap<>(); + private final Map>> groupListMap = new HashMap<>(); /** - * Push the BindingTuple to Group. Two functions will be applied to each BindingTuple to - * generate the {@link GroupKey} and {@link FieldKey} GroupKey = GroupKey(bindingTuple), - * FieldKey = FieldKey(bindingTuple) + * Push the BindingTuple to Group. */ public void push(ExprValue inputValue) { - GroupKey groupKey = new GroupKey(inputValue); - FieldKey fieldKey = new FieldKey(inputValue); + Key groupKey = new Key(inputValue, groupByExprList); + Key fieldKey = new Key(inputValue, fieldExprList); groupListMap.computeIfAbsent(groupKey, k -> { - List> list = new ArrayList<>(); - HashMap map = new HashMap<>(); + List> list = new ArrayList<>(); + HashMap map = new HashMap<>(); map.put(fieldKey, 1); list.add(map); return list; @@ -164,10 +162,10 @@ public List result() { groupListMap.forEach((groups, list) -> { Map map = new LinkedHashMap<>(); list.forEach(fieldMap -> { - List result = find(fieldMap); + List result = find(fieldMap); result.forEach(field -> { - map.putAll(groups.groupKeyMap()); - map.putAll(field.fieldKeyMap()); + map.putAll(groups.keyMap(groupByExprList)); + map.putAll(field.keyMap(fieldExprList)); resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); }); }); @@ -179,8 +177,8 @@ public List result() { /** * Get a list of result. */ - public List find(HashMap map) { - Comparator> valueComparator; + public List find(HashMap map) { + Comparator> valueComparator; if (rareTopFlag) { valueComparator = Map.Entry.comparingByValue(Comparator.reverseOrder()); } else { @@ -193,64 +191,33 @@ public List find(HashMap map) { } /** - * Field Key. + * Key. */ @EqualsAndHashCode @VisibleForTesting - public class FieldKey { + public class Key { - private final List fieldByValueList; + private final List valueList; /** - * FieldKey constructor. + * Key constructor. */ - public FieldKey(ExprValue value) { - this.fieldByValueList = new ArrayList<>(); - for (Expression fieldExpr : fieldExprList) { - this.fieldByValueList.add(fieldExpr.valueOf(value.bindingTuples())); - } + public Key(ExprValue value, List exprList) { + this.valueList = exprList.stream() + .map(expr -> expr.valueOf(value.bindingTuples())).collect(Collectors.toList()); } /** - * Return the Map of field and field value. + * Return the Map of key and key value. */ - public Map fieldKeyMap() { - Map map = new LinkedHashMap<>(); - for (int i = 0; i < fieldExprList.size(); i++) { - map.put(fieldExprList.get(i).toString(), fieldByValueList.get(i)); - } - return map; - } - } - - /** - * Group Key. - */ - @EqualsAndHashCode - @VisibleForTesting - public class GroupKey { - - private final List groupByValueList; - - /** - * GroupKey constructor. - */ - public GroupKey(ExprValue value) { - this.groupByValueList = new ArrayList<>(); - for (Expression groupExpr : groupByExprList) { - this.groupByValueList.add(groupExpr.valueOf(value.bindingTuples())); - } - } - - /** - * Return the Map of group field and group field value. - */ - public Map groupKeyMap() { - Map map = new LinkedHashMap<>(); - for (int i = 0; i < groupByExprList.size(); i++) { - map.put(groupByExprList.get(i).toString(), groupByValueList.get(i)); - } - return map; + public Map keyMap(List exprList) { + + return Streams.zip( + exprList.stream().map( + expression -> expression.toString()), + valueList.stream(), + AbstractMap.SimpleEntry::new + ).collect(Collectors.toMap(key -> key.getKey(), key -> key.getValue())); } } 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 f0bbe1b7e9..eb5594ee47 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 @@ -47,7 +47,10 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.ast.tree.UnresolvedPlan; +import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser; +import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.ByClauseContext; +import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParser.FieldListContext; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.parser.OpenDistroPPLParserBaseVisitor; import com.amazon.opendistroforelasticsearch.sql.ppl.utils.ArgumentFactory; import com.google.common.collect.ImmutableList; @@ -203,18 +206,25 @@ public UnresolvedPlan visitEvalCommand(EvalCommandContext ctx) { ); } + private List getGroupByList(ByClauseContext ctx) { + return ctx.fieldList().fieldExpression().stream().map(this::visitExpression) + .collect(Collectors.toList()); + } + + private List getFieldList(FieldListContext ctx) { + return ctx.fieldExpression() + .stream() + .map(field -> (Field) visitExpression(field)) + .collect(Collectors.toList()); + } + /** * Rare command. */ @Override public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : - ctx.byClause() - .fieldList() - .fieldExpression() - .stream() - .map(this::visitExpression) - .collect(Collectors.toList()); + getGroupByList(ctx.byClause()); return new RareTopN( /** @@ -222,11 +232,7 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { */ Boolean.FALSE, ArgumentFactory.getArgumentList(ctx), - ctx.fieldList() - .fieldExpression() - .stream() - .map(field -> (Field) visitExpression(field)) - .collect(Collectors.toList()), + getFieldList(ctx.fieldList()), groupList ); } @@ -237,12 +243,7 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { @Override public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : - ctx.byClause() - .fieldList() - .fieldExpression() - .stream() - .map(this::visitExpression) - .collect(Collectors.toList()); + getGroupByList(ctx.byClause()); return new RareTopN( /** @@ -250,11 +251,7 @@ public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { */ Boolean.TRUE, ArgumentFactory.getArgumentList(ctx), - ctx.fieldList() - .fieldExpression() - .stream() - .map(field -> (Field) visitExpression(field)) - .collect(Collectors.toList()), + getFieldList(ctx.fieldList()), groupList ); } From 0fe9d9baa59577fd1733a16bee716e84014f7a08 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Tue, 1 Sep 2020 15:35:26 -0700 Subject: [PATCH 12/15] address PR comments: - remove list - use generic getGroupByList function for stats - use generic getFieldList function for dedup --- .../planner/physical/RareTopNOperator.java | 32 ++++++++----------- .../sql/ppl/parser/AstBuilder.java | 13 ++------ 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index 17b24985b8..4ee5030bab 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -127,7 +127,7 @@ public void open() { @RequiredArgsConstructor public class Group { - private final Map>> groupListMap = new HashMap<>(); + private final Map> groupListMap = new HashMap<>(); /** * Push the BindingTuple to Group. @@ -136,20 +136,16 @@ public void push(ExprValue inputValue) { Key groupKey = new Key(inputValue, groupByExprList); Key fieldKey = new Key(inputValue, fieldExprList); groupListMap.computeIfAbsent(groupKey, k -> { - List> list = new ArrayList<>(); HashMap map = new HashMap<>(); map.put(fieldKey, 1); - list.add(map); - return list; + return map; }); - groupListMap.computeIfPresent(groupKey, (key, fieldList) -> { - fieldList.forEach(map -> { - map.computeIfAbsent(fieldKey, f -> 1); - map.computeIfPresent(fieldKey, (field, count) -> { - return count + 1; - }); + groupListMap.computeIfPresent(groupKey, (key, map) -> { + map.computeIfAbsent(fieldKey, f -> 1); + map.computeIfPresent(fieldKey, (field, count) -> { + return count + 1; }); - return fieldList; + return map; }); } @@ -159,15 +155,13 @@ public void push(ExprValue inputValue) { public List result() { ImmutableList.Builder resultBuilder = new ImmutableList.Builder<>(); - groupListMap.forEach((groups, list) -> { + groupListMap.forEach((groups, fieldMap) -> { Map map = new LinkedHashMap<>(); - list.forEach(fieldMap -> { - List result = find(fieldMap); - result.forEach(field -> { - map.putAll(groups.keyMap(groupByExprList)); - map.putAll(field.keyMap(fieldExprList)); - resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); - }); + List result = find(fieldMap); + result.forEach(field -> { + map.putAll(groups.keyMap(groupByExprList)); + map.putAll(field.keyMap(fieldExprList)); + resultBuilder.add(ExprTupleValue.fromExprValueMap(map)); }); }); 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 eb5594ee47..9ec6f3bf62 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 @@ -147,12 +147,7 @@ public UnresolvedPlan visitStatsCommand(StatsCommandContext ctx) { } } List groupList = ctx.byClause() == null ? Collections.emptyList() : - ctx.byClause() - .fieldList() - .fieldExpression() - .stream() - .map(this::visitExpression) - .collect(Collectors.toList()); + getGroupByList(ctx.byClause()); Aggregation aggregation = new Aggregation( aggListBuilder.build(), Collections.emptyList(), @@ -170,11 +165,7 @@ public UnresolvedPlan visitStatsCommand(StatsCommandContext ctx) { public UnresolvedPlan visitDedupCommand(DedupCommandContext ctx) { return new Dedupe( ArgumentFactory.getArgumentList(ctx), - ctx.fieldList() - .fieldExpression() - .stream() - .map(field -> (Field) visitExpression(field)) - .collect(Collectors.toList()) + getFieldList(ctx.fieldList()) ); } From 8c3300618281f939d0fdc96099b14e8893a5611b Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Tue, 1 Sep 2020 15:48:11 -0700 Subject: [PATCH 13/15] use interface instead of specific type for input of find() --- .../sql/planner/physical/RareTopNOperator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index 4ee5030bab..981721f2df 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -127,7 +127,7 @@ public void open() { @RequiredArgsConstructor public class Group { - private final Map> groupListMap = new HashMap<>(); + private final Map> groupListMap = new HashMap<>(); /** * Push the BindingTuple to Group. @@ -136,7 +136,7 @@ public void push(ExprValue inputValue) { Key groupKey = new Key(inputValue, groupByExprList); Key fieldKey = new Key(inputValue, fieldExprList); groupListMap.computeIfAbsent(groupKey, k -> { - HashMap map = new HashMap<>(); + Map map = new HashMap<>(); map.put(fieldKey, 1); return map; }); @@ -171,7 +171,7 @@ public List result() { /** * Get a list of result. */ - public List find(HashMap map) { + public List find(Map map) { Comparator> valueComparator; if (rareTopFlag) { valueComparator = Map.Entry.comparingByValue(Comparator.reverseOrder()); From 1612815dc5fc4edb4f392d8f87fae06955586560 Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Tue, 1 Sep 2020 17:32:16 -0700 Subject: [PATCH 14/15] update doc --- docs/experiment/ppl/cmd/rare.rst | 2 +- docs/experiment/ppl/cmd/top.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/experiment/ppl/cmd/rare.rst b/docs/experiment/ppl/cmd/rare.rst index 67d2b515cf..90611b408c 100644 --- a/docs/experiment/ppl/cmd/rare.rst +++ b/docs/experiment/ppl/cmd/rare.rst @@ -13,12 +13,12 @@ Description ============ | Using ``rare`` command to find the least common tuple of values of all fields in the field list. +A maximum of 10 results is returned for each distinct tuple of values of the group-by fields. Syntax ============ rare [by-clause] - * field-list: mandatory. comma-delimited list of field names. * by-clause: optional. one or more fields to group the results by. diff --git a/docs/experiment/ppl/cmd/top.rst b/docs/experiment/ppl/cmd/top.rst index 3f8e1aa27b..8093b5d296 100644 --- a/docs/experiment/ppl/cmd/top.rst +++ b/docs/experiment/ppl/cmd/top.rst @@ -11,7 +11,7 @@ top Description ============ -| Using ``top`` command to find the most frequent tuple of values of all fields in the field list. +| Using ``top`` command to find the most common tuple of values of all fields in the field list. Syntax From 77d6bf32a868174b6dfa314c2d722c15d393696f Mon Sep 17 00:00:00 2001 From: Rupal Mahajan <> Date: Wed, 2 Sep 2020 15:49:37 -0700 Subject: [PATCH 15/15] replace rareTopFlag with enum --- .../sql/analysis/Analyzer.java | 2 +- .../sql/ast/dsl/AstDSL.java | 5 +++-- .../sql/ast/tree/RareTopN.java | 7 +++++- .../sql/planner/DefaultImplementor.java | 2 +- .../sql/planner/logical/LogicalPlanDSL.java | 9 ++++---- .../sql/planner/logical/LogicalRareTopN.java | 3 ++- .../sql/planner/physical/PhysicalPlanDSL.java | 10 +++++---- .../planner/physical/RareTopNOperator.java | 22 ++++++++----------- .../sql/analysis/AnalyzerTest.java | 9 ++++---- .../sql/planner/DefaultImplementorTest.java | 6 ++--- .../logical/LogicalPlanNodeVisitorTest.java | 5 +++-- .../physical/PhysicalPlanNodeVisitorTest.java | 5 +++-- .../physical/RareTopNOperatorTest.java | 11 +++++----- .../ElasticsearchExecutionProtector.java | 2 +- .../ElasticsearchExecutionProtectorTest.java | 5 +++-- .../sql/ppl/parser/AstBuilder.java | 13 +++-------- .../sql/ppl/parser/AstBuilderTest.java | 15 +++++++------ 17 files changed, 68 insertions(+), 63 deletions(-) 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 3b3878da0c..a3c3f37365 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 @@ -205,7 +205,7 @@ public LogicalPlan visitRareTopN(RareTopN node, AnalysisContext context) { List options = node.getNoOfResults(); Integer noOfResults = (Integer) options.get(0).getValue().getValue(); - return new LogicalRareTopN(child, node.getRareTopFlag(), noOfResults, fields, groupBys); + return new LogicalRareTopN(child, node.getCommandType(), noOfResults, fields, groupBys); } /** 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 34aeeb756e..2d6b70920c 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 @@ -42,6 +42,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; @@ -302,9 +303,9 @@ public static List defaultTopArgs() { return exprList(argument("noOfResults", intLiteral(10))); } - public static RareTopN rareTopN(UnresolvedPlan input, Boolean rareTopFlag, + public static RareTopN rareTopN(UnresolvedPlan input, CommandType commandType, List noOfResults, List groupList, Field... fields) { - return new RareTopN(input, rareTopFlag, noOfResults, Arrays.asList(fields), groupList) + return new RareTopN(input, commandType, noOfResults, Arrays.asList(fields), groupList) .attach(input); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java index 705cfde4fd..8f04d6bea1 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/tree/RareTopN.java @@ -40,7 +40,7 @@ public class RareTopN extends UnresolvedPlan { private UnresolvedPlan child; - private final Boolean rareTopFlag; + private final CommandType commandType; private final List noOfResults; private final List fields; private final List groupExprList; @@ -60,5 +60,10 @@ public List getChild() { public T accept(AbstractNodeVisitor nodeVisitor, C context) { return nodeVisitor.visitRareTopN(this, context); } + + public enum CommandType { + TOP, + RARE + } } 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 a3a0980349..85f358e57b 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 @@ -57,7 +57,7 @@ public class DefaultImplementor extends LogicalPlanNodeVisitor groupByList, Expression... fields) { - return rareTopN(input, rareTopFlag, 10, groupByList, fields); + return rareTopN(input, commandType, 10, groupByList, fields); } - public static LogicalPlan rareTopN(LogicalPlan input, boolean rareTopFlag, int noOfResults, + public static LogicalPlan rareTopN(LogicalPlan input, CommandType commandType, int noOfResults, List groupByList, Expression... fields) { - return new LogicalRareTopN(input, rareTopFlag, noOfResults, Arrays.asList(fields), groupByList); + return new LogicalRareTopN(input, commandType, noOfResults, Arrays.asList(fields), groupByList); } @SafeVarargs diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java index 30a9eb4bad..22de70894d 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/logical/LogicalRareTopN.java @@ -15,6 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.planner.logical; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import java.util.Arrays; import java.util.Collections; @@ -34,7 +35,7 @@ public class LogicalRareTopN extends LogicalPlan { private final LogicalPlan child; - private final Boolean rareTopFlag; + private final CommandType commandType; private final Integer noOfResults; private final List fieldList; private final List groupByList; 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 51fc791ed8..515711a84f 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 @@ -15,6 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.planner.physical; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.LiteralExpression; @@ -80,14 +81,15 @@ public static DedupeOperator dedupe( input, Arrays.asList(expressions), allowedDuplication, keepEmpty, consecutive); } - public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, + public static RareTopNOperator rareTopN(PhysicalPlan input, CommandType commandType, List groups, Expression... expressions) { - return new RareTopNOperator(input, rareTopFlag, Arrays.asList(expressions), groups); + return new RareTopNOperator(input, commandType, Arrays.asList(expressions), groups); } - public static RareTopNOperator rareTopN(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, + public static RareTopNOperator rareTopN(PhysicalPlan input, CommandType commandType, + int noOfResults, List groups, Expression... expressions) { - return new RareTopNOperator(input, rareTopFlag, noOfResults, Arrays.asList(expressions), + return new RareTopNOperator(input, commandType, noOfResults, Arrays.asList(expressions), groups); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java index 981721f2df..4e98787c76 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperator.java @@ -15,6 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.planner.physical; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; @@ -47,13 +48,8 @@ public class RareTopNOperator extends PhysicalPlan { @Getter private final PhysicalPlan input; - - /** - * If rareTopFlag is true, result for top will be returned. - * If rareTopFlag is false, result for rare will be returned. - */ @Getter - private final Boolean rareTopFlag; + private final CommandType commandType; @Getter private final Integer noOfResults; @Getter @@ -69,25 +65,25 @@ public class RareTopNOperator extends PhysicalPlan { private static final Integer DEFAULT_NO_OF_RESULTS = 10; - public RareTopNOperator(PhysicalPlan input, Boolean rareTopFlag, List fieldExprList, - List groupByExprList) { - this(input, rareTopFlag, DEFAULT_NO_OF_RESULTS, fieldExprList, groupByExprList); + public RareTopNOperator(PhysicalPlan input, CommandType commandType, + List fieldExprList, List groupByExprList) { + this(input, commandType, DEFAULT_NO_OF_RESULTS, fieldExprList, groupByExprList); } /** * RareTopNOperator Constructor. * * @param input Input {@link PhysicalPlan} - * @param rareTopFlag Flag for Rare/TopN command. + * @param commandType Enum for Rare/TopN command. * @param noOfResults Number of results * @param fieldExprList List of {@link Expression} * @param groupByExprList List of group by {@link Expression} */ - public RareTopNOperator(PhysicalPlan input, Boolean rareTopFlag, int noOfResults, + public RareTopNOperator(PhysicalPlan input, CommandType commandType, int noOfResults, List fieldExprList, List groupByExprList) { this.input = input; - this.rareTopFlag = rareTopFlag; + this.commandType = commandType; this.noOfResults = noOfResults; this.fieldExprList = fieldExprList; this.groupByExprList = groupByExprList; @@ -173,7 +169,7 @@ public List result() { */ public List find(Map map) { Comparator> valueComparator; - if (rareTopFlag) { + if (CommandType.TOP.equals(commandType)) { valueComparator = Map.Entry.comparingByValue(Comparator.reverseOrder()); } else { valueComparator = Map.Entry.comparingByValue(); 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 1b62f653f0..b41818ac5e 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 @@ -30,6 +30,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; @@ -119,14 +120,14 @@ public void rare_source() { assertAnalyzeEqual( LogicalPlanDSL.rareTopN( LogicalPlanDSL.relation("schema"), - Boolean.FALSE, + CommandType.RARE, 10, ImmutableList.of(DSL.ref("string_value", STRING)), DSL.ref("integer_value", INTEGER) ), AstDSL.rareTopN( AstDSL.relation("schema"), - Boolean.FALSE, + CommandType.RARE, ImmutableList.of(argument("noOfResults", intLiteral(10))), ImmutableList.of(field("string_value")), field("integer_value") @@ -139,14 +140,14 @@ public void top_source() { assertAnalyzeEqual( LogicalPlanDSL.rareTopN( LogicalPlanDSL.relation("schema"), - Boolean.TRUE, + CommandType.TOP, 5, ImmutableList.of(DSL.ref("string_value", STRING)), DSL.ref("integer_value", INTEGER) ), AstDSL.rareTopN( AstDSL.relation("schema"), - Boolean.TRUE, + CommandType.TOP, ImmutableList.of(argument("noOfResults", intLiteral(5))), ImmutableList.of(field("string_value")), field("integer_value") 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 f250bb00fc..06a8033764 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 @@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; @@ -68,7 +69,6 @@ public void visitShouldReturnDefaultPhysicalOperator() { Expression filterExpr = literal(ExprBooleanValue.of(true)); List groupByExprs = Arrays.asList(ref("age", INTEGER)); ReferenceExpression rareTopNField = ref("age", INTEGER); - Boolean topFlag = Boolean.TRUE; List aggregators = Arrays.asList(new AvgAggregator(groupByExprs, ExprCoreType.DOUBLE)); Map mappings = @@ -96,7 +96,7 @@ public void visitShouldReturnDefaultPhysicalOperator() { newEvalField), sortCount, sortField), - topFlag, + CommandType.TOP, groupByExprs, rareTopNField), dedupeField), @@ -123,7 +123,7 @@ public void visitShouldReturnDefaultPhysicalOperator() { newEvalField), sortCount, sortField), - topFlag, + CommandType.TOP, groupByExprs, rareTopNField), dedupeField), 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 06aca69de7..397a79c8b3 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 @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; @@ -52,7 +53,7 @@ public void logicalPlanShouldTraversable() { LogicalPlanDSL.aggregation( LogicalPlanDSL.rareTopN( LogicalPlanDSL.filter(LogicalPlanDSL.relation("schema"), expression), - true, + CommandType.TOP, ImmutableList.of(expression), expression), ImmutableList.of(aggregator), @@ -104,7 +105,7 @@ public void testAbstractPlanNodeVisitorShouldReturnNull() { }, null)); LogicalPlan rareTopN = LogicalPlanDSL.rareTopN( - relation, true, ImmutableList.of(expression), expression); + relation, CommandType.TOP, ImmutableList.of(expression), expression); assertNull(rareTopN.accept(new LogicalPlanNodeVisitor() { }, null)); } 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 788e5411ea..151afdf779 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 @@ -23,6 +23,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.In; import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort.SortOption; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.ReferenceExpression; @@ -58,7 +59,7 @@ public void print_physical_plan() { PhysicalPlanDSL.filter( new TestScan(), dsl.equal(DSL.ref("response", INTEGER), DSL.literal(10))), - true, + CommandType.TOP, ImmutableList.of(), DSL.ref("response", INTEGER)), ImmutableList.of(dsl.avg(DSL.ref("response", INTEGER))), @@ -124,7 +125,7 @@ public void test_PhysicalPlanVisitor_should_return_null() { }, null)); PhysicalPlan rareTopN = - PhysicalPlanDSL.rareTopN(plan, true, 5, ImmutableList.of(), ref); + PhysicalPlanDSL.rareTopN(plan, CommandType.TOP, 5, ImmutableList.of(), ref); assertNull(rareTopN.accept(new PhysicalPlanNodeVisitor() { }, null)); } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java index 2f8196c120..a8bf626a37 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/RareTopNOperatorTest.java @@ -4,6 +4,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; @@ -18,7 +19,7 @@ public class RareTopNOperatorTest extends PhysicalPlanTestBase { @Test public void rare_without_group() { PhysicalPlan plan = new RareTopNOperator(new TestScan(), - Boolean.FALSE, + CommandType.RARE, Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), Collections.emptyList()); List result = execute(plan); @@ -32,7 +33,7 @@ public void rare_without_group() { @Test public void rare_with_group() { PhysicalPlan plan = new RareTopNOperator(new TestScan(), - Boolean.FALSE, + CommandType.RARE, Collections.singletonList(DSL.ref("response", ExprCoreType.INTEGER)), Collections.singletonList(DSL.ref("action", ExprCoreType.STRING))); List result = execute(plan); @@ -48,7 +49,7 @@ public void rare_with_group() { @Test public void top_without_group() { PhysicalPlan plan = new RareTopNOperator(new TestScan(), - Boolean.TRUE, + CommandType.TOP, Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), Collections.emptyList()); List result = execute(plan); @@ -62,7 +63,7 @@ public void top_without_group() { @Test public void top_n_without_group() { PhysicalPlan plan = new RareTopNOperator(new TestScan(), - Boolean.TRUE, + CommandType.TOP, 1, Collections.singletonList(DSL.ref("action", ExprCoreType.STRING)), Collections.emptyList()); @@ -76,7 +77,7 @@ public void top_n_without_group() { @Test public void top_n_with_group() { PhysicalPlan plan = new RareTopNOperator(new TestScan(), - Boolean.TRUE, + CommandType.TOP, 1, Collections.singletonList(DSL.ref("response", ExprCoreType.INTEGER)), Collections.singletonList(DSL.ref("action", ExprCoreType.STRING))); 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 6c705ee085..cef0164ca0 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 @@ -59,7 +59,7 @@ public PhysicalPlan visitAggregation(AggregationOperator node, Object context) { @Override public PhysicalPlan visitRareTopN(RareTopNOperator node, Object context) { - return new RareTopNOperator(visitInput(node.getInput(), context), node.getRareTopFlag(), + return new RareTopNOperator(visitInput(node.getInput(), context), node.getCommandType(), node.getNoOfResults(), node.getFieldExprList(), node.getGroupByExprList()); } 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 e1d9d6900d..71389a4aed 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 @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; import com.amazon.opendistroforelasticsearch.sql.common.setting.Settings; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; @@ -119,7 +120,7 @@ public void testProtectIndexScan() { newEvalField), sortCount, sortField), - true, + CommandType.TOP, groupByExprs, topField), dedupeField), @@ -144,7 +145,7 @@ public void testProtectIndexScan() { newEvalField), sortCount, sortField), - true, + CommandType.TOP, groupByExprs, topField), dedupeField), 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 9ec6f3bf62..1058bacb96 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 @@ -43,6 +43,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.tree.Filter; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Project; import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Relation; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Rename; import com.amazon.opendistroforelasticsearch.sql.ast.tree.Sort; @@ -217,11 +218,7 @@ public UnresolvedPlan visitRareCommand(RareCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : getGroupByList(ctx.byClause()); return new RareTopN( - - /** - * Setting rareTopFlag to FALSE will return list of rare values - */ - Boolean.FALSE, + CommandType.RARE, ArgumentFactory.getArgumentList(ctx), getFieldList(ctx.fieldList()), groupList @@ -236,11 +233,7 @@ public UnresolvedPlan visitTopCommand(TopCommandContext ctx) { List groupList = ctx.byClause() == null ? Collections.emptyList() : getGroupByList(ctx.byClause()); return new RareTopN( - - /** - * Setting rareTopFlag to TRUE will return list of top values - */ - Boolean.TRUE, + CommandType.TOP, ArgumentFactory.getArgumentList(ctx), getFieldList(ctx.fieldList()), groupList 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 62df52cb8c..0bff1d62bf 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 @@ -46,6 +46,7 @@ import static org.junit.Assert.assertEquals; import com.amazon.opendistroforelasticsearch.sql.ast.Node; +import com.amazon.opendistroforelasticsearch.sql.ast.tree.RareTopN.CommandType; import com.amazon.opendistroforelasticsearch.sql.ppl.antlr.PPLSyntaxParser; import org.junit.Ignore; import org.junit.Test; @@ -307,7 +308,7 @@ public void testRareCommand() { assertEqual("source=t | rare a", rareTopN( relation("t"), - false, + CommandType.RARE, exprList(argument("noOfResults", intLiteral(10))), emptyList(), field("a") @@ -319,7 +320,7 @@ public void testRareCommandWithGroupBy() { assertEqual("source=t | rare a by b", rareTopN( relation("t"), - false, + CommandType.RARE, exprList(argument("noOfResults", intLiteral(10))), exprList(field("b")), field("a") @@ -331,7 +332,7 @@ public void testRareCommandWithMultipleFields() { assertEqual("source=t | rare `a`, `b` by `c`", rareTopN( relation("t"), - false, + CommandType.RARE, exprList(argument("noOfResults", intLiteral(10))), exprList(field("c")), field("a"), @@ -344,7 +345,7 @@ public void testTopCommandWithN() { assertEqual("source=t | top 1 a", rareTopN( relation("t"), - true, + CommandType.TOP, exprList(argument("noOfResults", intLiteral(1))), emptyList(), field("a") @@ -356,7 +357,7 @@ public void testTopCommandWithoutNAndGroupBy() { assertEqual("source=t | top a", rareTopN( relation("t"), - true, + CommandType.TOP, exprList(argument("noOfResults", intLiteral(10))), emptyList(), field("a") @@ -368,7 +369,7 @@ public void testTopCommandWithNAndGroupBy() { assertEqual("source=t | top 1 a by b", rareTopN( relation("t"), - true, + CommandType.TOP, exprList(argument("noOfResults", intLiteral(1))), exprList(field("b")), field("a") @@ -380,7 +381,7 @@ public void testTopCommandWithMultipleFields() { assertEqual("source=t | top 1 `a`, `b` by `c`", rareTopN( relation("t"), - true, + CommandType.TOP, exprList(argument("noOfResults", intLiteral(1))), exprList(field("c")), field("a"),