diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/Analysis.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/Analysis.java index 86818981db16d..1f00636a69264 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/Analysis.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/Analysis.java @@ -62,6 +62,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import static com.facebook.presto.SystemSessionProperties.isCheckAccessControlOnUtilizedColumnsOnly; @@ -112,6 +113,7 @@ public class Analysis private final Map, List> windowFunctions = new LinkedHashMap<>(); private final Map, List> orderByWindowFunctions = new LinkedHashMap<>(); private final Map, Long> offset = new LinkedHashMap<>(); + private final Map, OptionalLong> limit = new LinkedHashMap<>(); private final Map, Expression> joins = new LinkedHashMap<>(); private final Map, JoinUsingAnalysis> joinUsing = new LinkedHashMap<>(); @@ -349,6 +351,22 @@ public long getOffset(Offset node) return offset.get(NodeRef.of(node)); } + public void setLimit(Node node, OptionalLong rowCount) + { + limit.put(NodeRef.of(node), rowCount); + } + + public void setLimit(Node node, long rowCount) + { + limit.put(NodeRef.of(node), OptionalLong.of(rowCount)); + } + + public OptionalLong getLimit(Node node) + { + checkState(limit.containsKey(NodeRef.of(node)), "missing LIMIT value for node %s", node); + return limit.get(NodeRef.of(node)); + } + public void setOutputExpressions(Node node, List expressions) { outputExpressions.put(NodeRef.of(node), ImmutableList.copyOf(expressions)); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/SemanticErrorCode.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/SemanticErrorCode.java index 66933a527c380..3132336fb9716 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/SemanticErrorCode.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/SemanticErrorCode.java @@ -107,4 +107,7 @@ public enum SemanticErrorCode TOO_MANY_GROUPING_SETS, INVALID_OFFSET_ROW_COUNT, + INVALID_FETCH_FIRST_ROW_COUNT, + INVALID_LIMIT_ROW_COUNT, + MISSING_ORDER_BY, } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index 64e34d2891116..cdb8b6e749a67 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -60,6 +60,7 @@ import com.facebook.presto.sql.tree.AllColumns; import com.facebook.presto.sql.tree.AlterFunction; import com.facebook.presto.sql.tree.Analyze; +import com.facebook.presto.sql.tree.AstVisitor; import com.facebook.presto.sql.tree.Call; import com.facebook.presto.sql.tree.Commit; import com.facebook.presto.sql.tree.ComparisonExpression; @@ -87,6 +88,7 @@ import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.ExpressionRewriter; import com.facebook.presto.sql.tree.ExpressionTreeRewriter; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.FieldReference; import com.facebook.presto.sql.tree.FrameBound; import com.facebook.presto.sql.tree.FunctionCall; @@ -104,6 +106,7 @@ import com.facebook.presto.sql.tree.JoinOn; import com.facebook.presto.sql.tree.JoinUsing; import com.facebook.presto.sql.tree.Lateral; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.Literal; import com.facebook.presto.sql.tree.LogicalBinaryExpression; import com.facebook.presto.sql.tree.LongLiteral; @@ -169,6 +172,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -216,7 +220,9 @@ import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_PARAMETER_NAME; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_PROPERTY; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_RELATION; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_FETCH_FIRST_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_FUNCTION_NAME; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_LIMIT_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_OFFSET_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_ORDINAL; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_PROCEDURE_ARGUMENTS; @@ -229,6 +235,7 @@ import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_CATALOG; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_COLUMN; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_MATERIALIZED_VIEW; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_ORDER_BY; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_SCHEMA; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MUST_BE_WINDOW_FUNCTION; @@ -1086,6 +1093,13 @@ protected Scope visitQuery(Query node, Optional scope) if (node.getOffset().isPresent()) { analyzeOffset(node.getOffset().get()); } + + if (node.getLimit().isPresent()) { + boolean requiresOrderBy = analyzeLimit(node.getLimit().get()); + if (requiresOrderBy && !node.getOrderBy().isPresent()) { + throw new SemanticException(MISSING_ORDER_BY, node.getLimit().get(), "FETCH FIRST WITH TIES clause requires ORDER BY"); + } + } // Input fields == Output fields analysis.setOutputExpressions(node, descriptorToFields(queryBodyScope)); @@ -1362,6 +1376,7 @@ private String getMaterializedViewSQL( Map partitionPredicates = generatePartitionPredicate(materializedViewStatus.getPartitionsFromBaseTables()); Query predicateStitchedQuery = (Query) new PredicateStitcher(session, partitionPredicates).process(createSqlStatement, new PredicateStitcherContext()); + QuerySpecification materializedViewQuerySpecification = new QuerySpecification( selectList(new AllColumns()), ((QuerySpecification) statement.getQueryBody()).getFrom(), @@ -1535,6 +1550,13 @@ protected Scope visitQuerySpecification(QuerySpecification node, Optional } analysis.setOrderByExpressions(node, orderByExpressions); + if (node.getLimit().isPresent()) { + boolean requiresOrderBy = analyzeLimit(node.getLimit().get()); + if (requiresOrderBy && !node.getOrderBy().isPresent()) { + throw new SemanticException(MISSING_ORDER_BY, node.getLimit().get(), "FETCH FIRST WITH TIES clause requires ORDER BY"); + } + } + List sourceExpressions = new ArrayList<>(outputExpressions); node.getHaving().ifPresent(sourceExpressions::add); @@ -2649,6 +2671,79 @@ private void analyzeOffset(Offset node) analysis.setOffset(node, rowCount); } + private List analyzeOrderBy(QuerySpecification node, Scope orderByScope, List outputExpressions) + { + checkState(node.getOrderBy().isPresent(), "orderBy is absent"); + + List sortItems = getSortItemsFromOrderBy(node.getOrderBy()); + + if (node.getSelect().isDistinct()) { + verifySelectDistinct(node, outputExpressions); + } + + return analyzeOrderBy(node, sortItems, orderByScope); + } + + /** + * @return true if the Query / QuerySpecification containing the analyzed + * Limit or FetchFirst, must contain orderBy (i.e., for FetchFirst with ties). + */ + private boolean analyzeLimit(Node node) + { + checkState( + node instanceof FetchFirst || node instanceof Limit, + "Invalid limit node type. Expected: FetchFirst or Limit. Actual: %s", node.getClass().getName()); + + return new AstVisitor() + { + @Override + protected Boolean visitFetchFirst(FetchFirst node, Void context) + { + if (!node.getRowCount().isPresent()) { + analysis.setLimit(node, 1); + } + else { + long rowCount; + try { + rowCount = Long.parseLong(node.getRowCount().get()); + } + catch (NumberFormatException e) { + throw new SemanticException(INVALID_FETCH_FIRST_ROW_COUNT, node, "Invalid FETCH FIRST row count: %s", node.getRowCount().get()); + } + if (rowCount <= 0) { + throw new SemanticException(INVALID_FETCH_FIRST_ROW_COUNT, node, "FETCH FIRST row count must be positive (actual value: %s)", rowCount); + } + analysis.setLimit(node, rowCount); + } + + if (node.isWithTies()) { + return true; + } + + return false; + } + + @Override + protected Boolean visitLimit(Limit node, Void context) + { + if (node.getLimit().equalsIgnoreCase("all")) { + analysis.setLimit(node, OptionalLong.empty()); + } + else { + long rowCount; + try { + rowCount = Long.parseLong(node.getLimit()); + } + catch (NumberFormatException e) { + throw new SemanticException(INVALID_LIMIT_ROW_COUNT, node, "Invalid LIMIT row count: %s", node.getLimit()); + } + analysis.setLimit(node, rowCount); + } + return false; + } + }.process(node, null); + } + private void verifySelectDistinct(QuerySpecification node, List outputExpressions) { for (SortItem item : node.getOrderBy().get().getSortItems()) { diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java index 0e2c8b6d38571..94f65df4345b9 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java @@ -42,6 +42,7 @@ import com.facebook.presto.sql.planner.iterative.rule.GatherAndMergeWindows; import com.facebook.presto.sql.planner.iterative.rule.ImplementBernoulliSampleAsFilter; import com.facebook.presto.sql.planner.iterative.rule.ImplementFilteredAggregations; +import com.facebook.presto.sql.planner.iterative.rule.ImplementLimitWithTies; import com.facebook.presto.sql.planner.iterative.rule.ImplementOffset; import com.facebook.presto.sql.planner.iterative.rule.InlineProjections; import com.facebook.presto.sql.planner.iterative.rule.InlineSqlFunctions; @@ -334,7 +335,8 @@ public PlanOptimizers( estimatedExchangesCostCalculator, ImmutableSet.of( new ImplementBernoulliSampleAsFilter(), - new ImplementOffset())), + new ImplementOffset(), + new ImplementLimitWithTies(metadata))), simplifyOptimizer, new UnaliasSymbolReferences(metadata.getFunctionAndTypeManager()), new IterativeOptimizer( diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java index 4659ef7f8683e..b1ecfd235b066 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/QueryPlanner.java @@ -51,6 +51,7 @@ import com.facebook.presto.sql.tree.Cast; import com.facebook.presto.sql.tree.Delete; import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.FieldReference; import com.facebook.presto.sql.tree.FrameBound; import com.facebook.presto.sql.tree.FunctionCall; @@ -146,9 +147,10 @@ public RelationPlan plan(Query query) List outputs = analysis.getOutputExpressions(query); builder = handleSubqueries(builder, query, outputs); builder = project(builder, Iterables.concat(orderBy, outputs)); - builder = sort(builder, query); + Optional orderingScheme = orderingScheme(builder, query.getOrderBy(), analysis.getOrderByExpressions(query)); + builder = sort(builder, orderingScheme); builder = offset(builder, query.getOffset()); - builder = limit(builder, query); + builder = limit(builder, query.getLimit(), orderingScheme); builder = project(builder, analysis.getOutputExpressions(query)); return new RelationPlan(builder.getRoot(), analysis.getScope(query), computeOutputs(builder, analysis.getOutputExpressions(query))); @@ -194,9 +196,10 @@ public RelationPlan plan(QuerySpecification node) builder = project(builder, Iterables.concat(orderBy, outputs)); builder = distinct(builder, node); - builder = sort(builder, node); + Optional orderingScheme = orderingScheme(builder, node.getOrderBy(), analysis.getOrderByExpressions(node)); + builder = sort(builder, orderingScheme); builder = offset(builder, node.getOffset()); - builder = limit(builder, node); + builder = limit(builder, node.getLimit(), orderingScheme); builder = project(builder, outputs); return new RelationPlan(builder.getRoot(), analysis.getScope(node), computeOutputs(builder, outputs)); @@ -887,29 +890,17 @@ private PlanBuilder distinct(PlanBuilder subPlan, QuerySpecification node) return subPlan; } - private PlanBuilder sort(PlanBuilder subPlan, Query node) - { - return sort(subPlan, node.getOrderBy(), analysis.getOrderByExpressions(node)); - } - - private PlanBuilder sort(PlanBuilder subPlan, QuerySpecification node) - { - return sort(subPlan, node.getOrderBy(), analysis.getOrderByExpressions(node)); - } - - private PlanBuilder sort(PlanBuilder subPlan, Optional orderBy, List orderByExpressions) + private Optional orderingScheme(PlanBuilder subPlan, Optional orderBy, List orderByExpressions) { if (!orderBy.isPresent() || (isSkipRedundantSort(session)) && analysis.isOrderByRedundant(orderBy.get())) { - return subPlan; + return Optional.empty(); } - PlanNode planNode; OrderingScheme orderingScheme = toOrderingScheme( orderByExpressions.stream().map(subPlan::translate).collect(toImmutableList()), orderBy.get().getSortItems().stream().map(PlannerUtils::toSortOrder).collect(toImmutableList())); - planNode = new SortNode(idAllocator.getNextId(), subPlan.getRoot(), orderingScheme, false); - return subPlan.withNewRoot(planNode); + return Optional.of(orderingScheme); } private PlanBuilder offset(PlanBuilder subPlan, Optional offset) @@ -925,25 +916,33 @@ private PlanBuilder offset(PlanBuilder subPlan, Optional offset) analysis.getOffset(offset.get()))); } - private PlanBuilder limit(PlanBuilder subPlan, Query node) - { - return limit(subPlan, node.getLimit()); - } - - private PlanBuilder limit(PlanBuilder subPlan, QuerySpecification node) - { - return limit(subPlan, node.getLimit()); - } - - private PlanBuilder limit(PlanBuilder subPlan, Optional limit) + private PlanBuilder sort(PlanBuilder subPlan, Optional orderingScheme) { - if (!limit.isPresent()) { + if (!orderingScheme.isPresent()) { return subPlan; } + return subPlan.withNewRoot( + new SortNode( + idAllocator.getNextId(), + subPlan.getRoot(), + orderingScheme.get(), + false)); + } - if (!limit.get().equalsIgnoreCase("all")) { - long limitValue = Long.parseLong(limit.get()); - subPlan = subPlan.withNewRoot(new LimitNode(idAllocator.getNextId(), subPlan.getRoot(), limitValue, FINAL)); + private PlanBuilder limit(PlanBuilder subPlan, Optional limit, Optional orderingScheme) + { + if (limit.isPresent() && analysis.getLimit(limit.get()).isPresent()) { + Optional tiesResolvingScheme = Optional.empty(); + if (limit.get() instanceof FetchFirst && ((FetchFirst) limit.get()).isWithTies()) { + tiesResolvingScheme = orderingScheme; + } + return subPlan.withNewRoot( + new LimitNode( + idAllocator.getNextId(), + subPlan.getRoot(), + analysis.getLimit(limit.get()).getAsLong(), + tiesResolvingScheme, + FINAL)); } return subPlan; diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementLimitWithTies.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementLimitWithTies.java new file mode 100644 index 0000000000000..8be57e89b1a97 --- /dev/null +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementLimitWithTies.java @@ -0,0 +1,137 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.planner.iterative.rule; + +import com.facebook.presto.matching.Capture; +import com.facebook.presto.matching.Captures; +import com.facebook.presto.matching.Pattern; +import com.facebook.presto.metadata.Metadata; +import com.facebook.presto.spi.function.FunctionHandle; +import com.facebook.presto.spi.plan.FilterNode; +import com.facebook.presto.spi.plan.LimitNode; +import com.facebook.presto.spi.plan.PlanNode; +import com.facebook.presto.spi.plan.ProjectNode; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.iterative.Rule; +import com.facebook.presto.sql.planner.plan.WindowNode; +import com.facebook.presto.sql.tree.ComparisonExpression; +import com.facebook.presto.sql.tree.FrameBound; +import com.facebook.presto.sql.tree.GenericLiteral; +import com.facebook.presto.sql.tree.QualifiedName; +import com.facebook.presto.sql.tree.SymbolReference; +import com.facebook.presto.sql.tree.WindowFrame; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; + +import java.util.Optional; + +import static com.facebook.presto.common.type.BigintType.BIGINT; +import static com.facebook.presto.matching.Capture.newCapture; +import static com.facebook.presto.metadata.FunctionAndTypeManager.qualifyObjectName; +import static com.facebook.presto.sql.planner.optimizations.WindowNodeUtil.toBoundType; +import static com.facebook.presto.sql.planner.optimizations.WindowNodeUtil.toWindowType; +import static com.facebook.presto.sql.planner.plan.AssignmentUtils.identityAssignmentsAsSymbolReferences; +import static com.facebook.presto.sql.planner.plan.Patterns.limit; +import static com.facebook.presto.sql.planner.plan.Patterns.source; +import static com.facebook.presto.sql.relational.Expressions.call; +import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToRowExpression; +import static java.util.Objects.requireNonNull; + +/** + * Transforms: + * 
+ * - Limit (row count = x, tiesResolvingScheme(a,b,c))
+ *    - source
+ *
+ * Into: + * 
+ * - Project (prune rank symbol)
+ *    - Filter (rank <= x)
+ *       - Window (function: rank, order by a,b,c)
+ *          - source
+ *
+ */ +public class ImplementLimitWithTies + implements Rule +{ + private static final Capture CHILD = newCapture(); + private static final Pattern PATTERN = limit() + .matching(LimitNode::isWithTies) + .with(source().capturedAs(CHILD)); + + private final Metadata metadata; + + public ImplementLimitWithTies(Metadata metadata) + { + this.metadata = requireNonNull(metadata, "metadata is null"); + } + + @Override + public Pattern getPattern() + { + return PATTERN; + } + + @Override + public Result apply(LimitNode parent, Captures captures, Context context) + { + PlanNode child = captures.get(CHILD); + VariableReferenceExpression rankSymbol = context.getVariableAllocator().newVariable("rank_num", BIGINT); + + FunctionHandle function = metadata.getFunctionAndTypeManager().resolveFunction( + Optional.of(context.getSession().getSessionFunctions()), + Optional.empty(), + qualifyObjectName(QualifiedName.of("rank")), + ImmutableList.of()); + + WindowNode.Frame frame = new WindowNode.Frame( + toWindowType(WindowFrame.Type.RANGE), + toBoundType(FrameBound.Type.UNBOUNDED_PRECEDING), + Optional.empty(), + toBoundType(FrameBound.Type.CURRENT_ROW), + Optional.empty(), + Optional.empty(), + Optional.empty()); + + WindowNode.Function rankFunction = new WindowNode.Function( + call("rank", function, BIGINT, ImmutableList.of()), + frame, + false); + + WindowNode windowNode = new WindowNode( + context.getIdAllocator().getNextId(), + child, + new WindowNode.Specification(ImmutableList.of(), parent.getTiesResolvingScheme()), + ImmutableMap.of(rankSymbol, rankFunction), + Optional.empty(), + ImmutableSet.of(), + 0); + + FilterNode filterNode = new FilterNode( + context.getIdAllocator().getNextId(), + windowNode, + castToRowExpression(new ComparisonExpression( + ComparisonExpression.Operator.LESS_THAN_OR_EQUAL, + new SymbolReference(rankSymbol.getName()), + new GenericLiteral("BIGINT", Long.toString(parent.getCount()))))); + + ProjectNode projectNode = new ProjectNode( + context.getIdAllocator().getNextId(), + filterNode, + identityAssignmentsAsSymbolReferences(parent.getOutputVariables())); + + return Rule.Result.ofPlanNode(projectNode); + } +} diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithDistinct.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithDistinct.java index ac440d631b3f0..7f2f082e9d35a 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithDistinct.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithDistinct.java @@ -32,6 +32,7 @@ public class MergeLimitWithDistinct private static final Capture CHILD = newCapture(); private static final Pattern PATTERN = limit() + .matching(limit -> !limit.isWithTies()) .with(source().matching(aggregation().capturedAs(CHILD) .matching(MergeLimitWithDistinct::isDistinct))); @@ -41,6 +42,7 @@ public class MergeLimitWithDistinct private static boolean isDistinct(AggregationNode node) { return node.getAggregations().isEmpty() && + !node.getGroupingKeys().isEmpty() && node.getOutputVariables().size() == node.getGroupingKeys().size() && node.getOutputVariables().containsAll(node.getGroupingKeys()); } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithSort.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithSort.java index bc0f4cba1a454..54f91e42af70a 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithSort.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithSort.java @@ -32,6 +32,7 @@ public class MergeLimitWithSort private static final Capture CHILD = newCapture(); private static final Pattern PATTERN = limit() + .matching(limit -> !limit.isWithTies()) .with(source().matching(sort().capturedAs(CHILD))); @Override diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithTopN.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithTopN.java index 531742f0c78b8..a9778273b86a8 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithTopN.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimitWithTopN.java @@ -25,6 +25,30 @@ import static com.facebook.presto.sql.planner.plan.Patterns.source; import static com.facebook.presto.sql.planner.plan.Patterns.topN; +/** + * This rule handles both LimitNode with ties and LimitNode without ties. + * In the case of LimitNode with ties, the LimitNode is removed + * if only its row count equals or exceeds TopNNode's row count: + * 
+ *     - Limit (5, with ties)
+ *        - TopN (3)
+ *
+ * is transformed into: + * 
+ *     - TopN (3)
+ *
+ * In the case of LimitNode without ties, the LimitNode is removed, + * and the TopNNode's row count is updated to minimum row count + * of both nodes: + * 
+ *     - Limit (3)
+ *        - TopN (5)
+ *
+ * is transformed into: + * 
+ *     - TopN (3)
+ *
+ */ public class MergeLimitWithTopN implements Rule { @@ -44,6 +68,15 @@ public Result apply(LimitNode parent, Captures captures, Context context) { TopNNode child = captures.get(CHILD); + if (parent.isWithTies()) { + if (parent.getCount() < child.getCount()) { + return Result.empty(); + } + else { + return Result.ofPlanNode(child); + } + } + return Result.ofPlanNode( new TopNNode( parent.getId(), diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimits.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimits.java index a917cd4af4c1d..f9b072d4a060b 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimits.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/MergeLimits.java @@ -18,17 +18,64 @@ import com.facebook.presto.matching.Pattern; import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.sql.planner.iterative.Rule; +import com.google.common.collect.ImmutableList; import static com.facebook.presto.matching.Capture.newCapture; import static com.facebook.presto.sql.planner.plan.Patterns.limit; import static com.facebook.presto.sql.planner.plan.Patterns.source; +/** + * This rule handles both LimitNode with ties and LimitNode without ties. + * The parent LimitNode is without ties. + *

+ * If the child LimitNode is without ties, both nodes are merged + * into a single LimitNode with row count being the minimum + * of their row counts: + *

+ * 
+ *    - Limit (3)
+ *       - Limit (5)
+ *
+ * is transformed into: + * 
+ *     - Limit (3)
+ *
+ *

+ * If the child LimitNode is with ties, the rule's behavior depends + * on both nodes' row count. + * If parent row count is lower or equal to child row count, + * child node is removed from the plan: + *

+ * 
+ *     - Limit (3)
+ *        - Limit (5, withTies)
+ *
+ * is transformed into: + * 
+ *     - Limit (3)
+ *
+ *

+ * If parent row count is greater than child row count, both nodes + * remain in the plan, but they are rearranged in the way that + * the LimitNode with ties is the root: + *

+ * 
+ *     - Limit (5)
+ *        - Limit (3, withTies)
+ *
+ * is transformed into: + * 
+ *     - Limit (3, withTies)
+ *        - Limit (5)
+ *
+ */ public class MergeLimits implements Rule { private static final Capture CHILD = newCapture(); private static final Pattern PATTERN = limit() + .matching(limit -> !limit.isWithTies()) .with(source().matching(limit().capturedAs(CHILD))); @Override @@ -42,11 +89,27 @@ public Result apply(LimitNode parent, Captures captures, Context context) { LimitNode child = captures.get(CHILD); + // parent and child are without ties + if (!child.isWithTies()) { + return Result.ofPlanNode( + new LimitNode( + parent.getId(), + child.getSource(), + Math.min(parent.getCount(), child.getCount()), + parent.getTiesResolvingScheme(), + parent.getStep())); + } + + // parent is without ties and child is with ties + if (parent.getCount() > child.getCount()) { + return Result.ofPlanNode( + child.replaceChildren(ImmutableList.of( + parent.replaceChildren(ImmutableList.of( + child.getSource()))))); + } + return Result.ofPlanNode( - new LimitNode( - parent.getId(), - child.getSource(), - Math.min(parent.getCount(), child.getCount()), - parent.getStep())); + parent.replaceChildren(ImmutableList.of( + child.getSource()))); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PruneLimitColumns.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PruneLimitColumns.java index d5db49acf38d9..9de63d198ecd9 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PruneLimitColumns.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PruneLimitColumns.java @@ -14,10 +14,13 @@ package com.facebook.presto.sql.planner.iterative.rule; import com.facebook.presto.spi.plan.LimitNode; +import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.PlanNodeIdAllocator; import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.PlanVariableAllocator; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import java.util.Optional; import java.util.Set; @@ -36,6 +39,13 @@ public PruneLimitColumns() @Override protected Optional pushDownProjectOff(PlanNodeIdAllocator idAllocator, PlanVariableAllocator variableAllocator, LimitNode limitNode, Set referencedOutputs) { - return restrictChildOutputs(idAllocator, limitNode, referencedOutputs); + Set prunedLimitInputs = ImmutableSet.builder() + .addAll(referencedOutputs) + .addAll(limitNode.getTiesResolvingScheme() + .map(OrderingScheme::getOrderByVariables) + .orElse(ImmutableList.of())) + .build(); + + return restrictChildOutputs(idAllocator, limitNode, prunedLimitInputs); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughMarkDistinct.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughMarkDistinct.java index 056a9677cd1ef..d9f089612b998 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughMarkDistinct.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughMarkDistinct.java @@ -31,6 +31,7 @@ public class PushLimitThroughMarkDistinct { private static final Capture CHILD = newCapture(); + // Applies to both limit with ties and limit without ties. private static final Pattern PATTERN = limit() .with(source().matching(markDistinct().capturedAs(CHILD))); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughOuterJoin.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughOuterJoin.java index 3856ad6eaaafa..f1c93a149c416 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughOuterJoin.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughOuterJoin.java @@ -52,6 +52,7 @@ * - Limit (present if Join is right or outer) * - right source * + * Applies to LimitNode without ties only to avoid optimizer loop. */ public class PushLimitThroughOuterJoin implements Rule @@ -60,6 +61,7 @@ public class PushLimitThroughOuterJoin private static final Pattern PATTERN = limit() + .matching(limit -> !limit.isWithTies()) .with(source().matching( join() .with(type().matching(type -> type == LEFT || type == RIGHT)) diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughProject.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughProject.java index edebb79e6d14e..73fa6000cc8a9 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughProject.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughProject.java @@ -18,13 +18,20 @@ import com.facebook.presto.matching.Pattern; import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.ProjectNode; +import com.facebook.presto.spi.relation.RowExpression; +import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.iterative.Rule; +import com.facebook.presto.sql.planner.optimizations.SymbolMapper; +import com.facebook.presto.sql.tree.SymbolReference; +import com.google.common.collect.ImmutableList; import static com.facebook.presto.matching.Capture.newCapture; import static com.facebook.presto.sql.planner.iterative.rule.Util.transpose; import static com.facebook.presto.sql.planner.plan.Patterns.limit; import static com.facebook.presto.sql.planner.plan.Patterns.project; import static com.facebook.presto.sql.planner.plan.Patterns.source; +import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToExpression; +import static com.facebook.presto.sql.relational.OriginalExpressionUtils.isExpression; import static com.facebook.presto.sql.relational.ProjectNodeUtils.isIdentity; public class PushLimitThroughProject @@ -48,6 +55,35 @@ public Pattern getPattern() @Override public Result apply(LimitNode parent, Captures captures, Context context) { - return Result.ofPlanNode(transpose(parent, captures.get(CHILD))); + ProjectNode projectNode = captures.get(CHILD); + + // for a LimitNode without ties, simply reorder the nodes + if (!parent.isWithTies()) { + return Result.ofPlanNode(transpose(parent, projectNode)); + } + + // for a LimitNode with ties, the tiesResolvingScheme must be rewritten in terms of symbols before projection + SymbolMapper.Builder symbolMapper = SymbolMapper.builder(); + for (VariableReferenceExpression symbol : parent.getTiesResolvingScheme().get().getOrderByVariables()) { + RowExpression expression = projectNode.getAssignments().get(symbol); + // if a symbol results from some computation, the translation fails + if (!isExpression(expression)) { + return Result.empty(); + } + else if (isExpression(expression) && castToExpression(expression) instanceof SymbolReference) { + VariableReferenceExpression variable = context.getVariableAllocator().toVariableReference(castToExpression(expression)); + symbolMapper.put(symbol, variable); + } + else if (expression instanceof VariableReferenceExpression) { + VariableReferenceExpression variable = context.getVariableAllocator().newVariable(expression); + symbolMapper.put(symbol, variable); + } + else { + return Result.empty(); + } + } + + LimitNode mappedLimitNode = symbolMapper.build().map(parent, projectNode.getSource()); + return Result.ofPlanNode(projectNode.replaceChildren(ImmutableList.of(mappedLimitNode))); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughSemiJoin.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughSemiJoin.java index 3bc94429d0abc..8826458dac473 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughSemiJoin.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughSemiJoin.java @@ -43,6 +43,14 @@ public Pattern getPattern() @Override public Result apply(LimitNode parent, Captures captures, Context context) { - return Result.ofPlanNode(transpose(parent, captures.get(CHILD))); + SemiJoinNode semiJoinNode = captures.get(CHILD); + + if (parent.isWithTies()) { + // do not push down Limit if ties depend on symbol produced by SemiJoin + if (parent.getTiesResolvingScheme().get().getOrderBy().contains(semiJoinNode.getSemiJoinOutput())) { + return Result.empty(); + } + } + return Result.ofPlanNode(transpose(parent, semiJoinNode)); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughUnion.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughUnion.java index ca64d20867a50..bedb9cf08d303 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughUnion.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PushLimitThroughUnion.java @@ -55,6 +55,7 @@ public class PushLimitThroughUnion private static final Capture CHILD = newCapture(); private static final Pattern PATTERN = limit() + .matching(limit -> !limit.isWithTies()) .with(source().matching(union().capturedAs(CHILD))); @Override diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java index a602169c45fe8..eb7ca3440add0 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddExchanges.java @@ -538,6 +538,10 @@ public PlanWithProperties visitSort(SortNode node, PreferredProperties preferred @Override public PlanWithProperties visitLimit(LimitNode node, PreferredProperties preferredProperties) { + if (node.isWithTies()) { + throw new IllegalStateException("Unexpected node: LimitNode with ties"); + } + PlanWithProperties child = planChild(node, PreferredProperties.any()); if (!child.getProperties().isSingleNode()) { diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddLocalExchanges.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddLocalExchanges.java index b82f0a035e517..61f28ca9f3c02 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddLocalExchanges.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/AddLocalExchanges.java @@ -242,6 +242,10 @@ public PlanWithProperties visitTopN(TopNNode node, StreamPreferredProperties par @Override public PlanWithProperties visitLimit(LimitNode node, StreamPreferredProperties parentPreferences) { + if (node.isWithTies()) { + throw new IllegalStateException("Unexpected node: LimitNode with ties"); + } + if (node.isPartial()) { return planAndEnforceChildren( node, diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/LimitPushDown.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/LimitPushDown.java index 6545038581036..94db61631bffb 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/LimitPushDown.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/LimitPushDown.java @@ -114,8 +114,10 @@ public PlanNode visitPlan(PlanNode node, RewriteContext context) public PlanNode visitLimit(LimitNode node, RewriteContext context) { long count = node.getCount(); - if (context.get() != null) { - count = Math.min(count, context.get().getCount()); + + LimitContext limit = context.get(); + if (limit != null) { + count = Math.min(count, limit.getCount()); } // return empty ValuesNode in case of limit 0 @@ -125,8 +127,13 @@ public PlanNode visitLimit(LimitNode node, RewriteContext context) ImmutableList.of()); } + if (!node.isWithTies() || (limit != null && node.getCount() >= limit.getCount())) { + // default visitPlan logic will insert the limit node + return context.rewrite(node.getSource(), new LimitContext(count, FINAL)); + } + // default visitPlan logic will insert the limit node - return context.rewrite(node.getSource(), new LimitContext(count, FINAL)); + return context.rewrite(node.getSource(), context.get()); } @Override diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java index 6d418aad1c022..663d7f30cd3fc 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PruneUnreferencedOutputs.java @@ -25,6 +25,7 @@ import com.facebook.presto.spi.plan.IntersectNode; import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; +import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.PlanNodeIdAllocator; import com.facebook.presto.spi.plan.ProjectNode; @@ -596,7 +597,8 @@ public PlanNode visitOutput(OutputNode node, RewriteContext> context) { ImmutableSet.Builder expectedInputs = ImmutableSet.builder() - .addAll(context.get()); + .addAll(context.get()) + .addAll(node.getTiesResolvingScheme().map(OrderingScheme::getOrderByVariables).orElse(ImmutableList.of())); PlanNode source = context.rewrite(node.getSource(), expectedInputs.build()); return new LimitNode(node.getId(), source, node.getCount(), node.getStep()); } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/QueryCardinalityUtil.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/QueryCardinalityUtil.java index 636aaf0a01c45..45e44a7b2c93d 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/QueryCardinalityUtil.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/QueryCardinalityUtil.java @@ -159,13 +159,28 @@ public Range visitOffset(OffsetNode node, Void context) @Override public Range visitLimit(LimitNode node, Void context) { - Range sourceCardinalityRange = node.getSource().accept(this, null); - long upper = node.getCount(); + if (node.isWithTies()) { + Range sourceCardinalityRange = node.getSource().accept(this, null); + long lower = min(node.getCount(), sourceCardinalityRange.lowerEndpoint()); + if (sourceCardinalityRange.hasUpperBound()) { + return Range.closed(lower, sourceCardinalityRange.upperEndpoint()); + } + else { + return Range.atLeast(lower); + } + } + + return applyLimit(node.getSource(), node.getCount()); + } + + private Range applyLimit(PlanNode node, long limit) + { + Range sourceCardinalityRange = node.accept(this, null); if (sourceCardinalityRange.hasUpperBound()) { - upper = min(sourceCardinalityRange.upperEndpoint(), node.getCount()); + limit = min(sourceCardinalityRange.upperEndpoint(), limit); } - long lower = min(upper, sourceCardinalityRange.lowerEndpoint()); - return Range.closed(lower, upper); + long lower = min(limit, sourceCardinalityRange.lowerEndpoint()); + return Range.closed(lower, limit); } } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/SymbolMapper.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/SymbolMapper.java index 9fb49216339b4..b7091dd50f28c 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/SymbolMapper.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/SymbolMapper.java @@ -18,6 +18,7 @@ import com.facebook.presto.expressions.RowExpressionTreeRewriter; import com.facebook.presto.spi.plan.AggregationNode; import com.facebook.presto.spi.plan.AggregationNode.Aggregation; +import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.Ordering; import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.PlanNode; @@ -131,15 +132,29 @@ public RowExpression rewriteVariableReference(VariableReferenceExpression variab }, value); } + public LimitNode map(LimitNode node, PlanNode source) + { + return new LimitNode( + node.getId(), + source, + node.getCount(), + node.getTiesResolvingScheme().map(this::map), + node.getStep()); + } + public OrderingScheme map(OrderingScheme orderingScheme) { // SymbolMapper inlines symbol with multiple level reference (SymbolInliner only inline single level). ImmutableList.Builder orderBy = ImmutableList.builder(); ImmutableMap.Builder ordering = ImmutableMap.builder(); + Set added = new HashSet<>(orderingScheme.getOrderBy().size()); + for (VariableReferenceExpression variable : orderingScheme.getOrderByVariables()) { VariableReferenceExpression translated = map(variable); - orderBy.add(translated); - ordering.put(translated, orderingScheme.getOrdering(variable)); + if (added.add(translated)) { + orderBy.add(translated); + ordering.put(translated, orderingScheme.getOrdering(variable)); + } } ImmutableMap orderingMap = ordering.build(); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/UnaliasSymbolReferences.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/UnaliasSymbolReferences.java index 9d1fddb7f12af..bfcbee4e99d61 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/UnaliasSymbolReferences.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/UnaliasSymbolReferences.java @@ -372,6 +372,10 @@ public PlanNode visitOffset(OffsetNode node, RewriteContext context) @Override public PlanNode visitLimit(LimitNode node, RewriteContext context) { + if (node.isWithTies()) { + PlanNode source = context.rewrite(node.getSource()); + return new LimitNode(node.getId(), source, node.getCount(), node.getTiesResolvingScheme().map(this::canonicalizeAndDistinct), node.getStep()); + } return context.defaultRewrite(node); } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/WindowFilterPushDown.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/WindowFilterPushDown.java index 5528fa31006e8..8c82f40be063e 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/WindowFilterPushDown.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/WindowFilterPushDown.java @@ -125,6 +125,10 @@ public PlanNode visitWindow(WindowNode node, RewriteContext context) @Override public PlanNode visitLimit(LimitNode node, RewriteContext context) { + if (node.isWithTies()) { + return context.defaultRewrite(node); + } + // Operators can handle MAX_VALUE rows per page, so do not optimize if count is greater than this value if (node.getCount() > Integer.MAX_VALUE) { return context.defaultRewrite(node); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java index e1df9bcd54ba9..7ab183c67a540 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java @@ -548,7 +548,7 @@ public Void visitLimit(LimitNode node, Void context) { addNode(node, format("Limit%s", node.isPartial() ? "Partial" : ""), - format("[%s]", node.getCount())); + format("[%s%s]", node.getCount(), node.isWithTies() ? "+ties" : "")); return processChildren(node, context); } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeInputRewrite.java b/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeInputRewrite.java index b2515c9d001d1..3c56820bffed3 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeInputRewrite.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeInputRewrite.java @@ -26,6 +26,7 @@ import com.facebook.presto.sql.tree.AstVisitor; import com.facebook.presto.sql.tree.DescribeInput; import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.LongLiteral; import com.facebook.presto.sql.tree.Node; import com.facebook.presto.sql.tree.NullLiteral; @@ -113,10 +114,10 @@ protected Node visitDescribeInput(DescribeInput node, Void context) // return the positions and types of all parameters Row[] rows = parameters.stream().map(parameter -> createDescribeInputRow(parameter, analysis)).toArray(Row[]::new); - Optional limit = Optional.empty(); + Optional limit = Optional.empty(); if (rows.length == 0) { rows = new Row[] {row(new NullLiteral(), new NullLiteral())}; - limit = Optional.of("0"); + limit = Optional.of(new Limit("0")); } return simpleQuery( diff --git a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeOutputRewrite.java b/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeOutputRewrite.java index 97a45171aacbc..b913029685c05 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeOutputRewrite.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/rewrite/DescribeOutputRewrite.java @@ -28,6 +28,7 @@ import com.facebook.presto.sql.tree.BooleanLiteral; import com.facebook.presto.sql.tree.DescribeOutput; import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.LongLiteral; import com.facebook.presto.sql.tree.Node; import com.facebook.presto.sql.tree.NullLiteral; @@ -103,12 +104,12 @@ protected Node visitDescribeOutput(DescribeOutput node, Void context) Analyzer analyzer = new Analyzer(session, metadata, parser, accessControl, queryExplainer, parameters, warningCollector); Analysis analysis = analyzer.analyze(statement, true); - Optional limit = Optional.empty(); + Optional limit = Optional.empty(); Row[] rows = analysis.getRootScope().getRelationType().getVisibleFields().stream().map(field -> createDescribeOutputRow(field, analysis)).toArray(Row[]::new); if (rows.length == 0) { NullLiteral nullLiteral = new NullLiteral(); rows = new Row[] {row(nullLiteral, nullLiteral, nullLiteral, nullLiteral, nullLiteral, nullLiteral, nullLiteral)}; - limit = Optional.of("0"); + limit = Optional.of(new Limit("0")); } return simpleQuery( selectList( diff --git a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java index ac0230f32bf8b..567a20fc0a37c 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java @@ -39,7 +39,9 @@ import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_COLUMN_NAME; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_PROPERTY; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_RELATION; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_FETCH_FIRST_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_FUNCTION_NAME; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_LIMIT_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_LITERAL; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_OFFSET_ROW_COUNT; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_ORDINAL; @@ -52,6 +54,7 @@ import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_ATTRIBUTE; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_CATALOG; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_COLUMN; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_ORDER_BY; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_SCHEMA; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MULTIPLE_FIELDS_FROM_SUBQUERY; @@ -341,6 +344,28 @@ public void testOffsetInvalidRowCount() assertFails(INVALID_OFFSET_ROW_COUNT, "SELECT * FROM t1 OFFSET 987654321098765432109876543210 ROWS"); } + @Test + public void testFetchFirstInvalidRowCount() + { + assertFails(INVALID_FETCH_FIRST_ROW_COUNT, "SELECT * FROM t1 FETCH FIRST 987654321098765432109876543210 ROWS ONLY"); + assertFails(INVALID_FETCH_FIRST_ROW_COUNT, "SELECT * FROM t1 FETCH FIRST 0 ROWS ONLY"); + } + + @Test + public void testFetchFirstWithTiesMissingOrderBy() + { + assertFails(MISSING_ORDER_BY, "SELECT * FROM t1 FETCH FIRST 5 ROWS WITH TIES"); + + // ORDER BY clause must be in the same scope as FETCH FIRST WITH TIES + assertFails(MISSING_ORDER_BY, "SELECT * FROM (SELECT * FROM (values 1, 3, 2) t(a) ORDER BY a) FETCH FIRST 5 ROWS WITH TIES"); + } + + @Test + public void testLimitInvalidRowCount() + { + assertFails(INVALID_LIMIT_ROW_COUNT, "SELECT * FROM t1 LIMIT 987654321098765432109876543210"); + } + @Test public void testNestedAggregation() { diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/TestLogicalPlanner.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/TestLogicalPlanner.java index 504482652b9b1..c73b4c7d1888c 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/TestLogicalPlanner.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/TestLogicalPlanner.java @@ -14,6 +14,7 @@ package com.facebook.presto.sql.planner; import com.facebook.presto.Session; +import com.facebook.presto.common.block.SortOrder; import com.facebook.presto.spi.plan.AggregationNode; import com.facebook.presto.spi.plan.DistinctLimitNode; import com.facebook.presto.spi.plan.FilterNode; @@ -1373,4 +1374,59 @@ public void testOffset() tableScan("nation", ImmutableMap.of("name", "name", "regionkey", "regionkey")))))) .withAlias("row_num", new RowNumberSymbolMatcher()))))); } + @Test + public void testOrderByFetch() + { + assertPlan( + "SELECT * FROM nation ORDER BY name FETCH FIRST 2 ROWS ONLY", + anyTree( + topN( + 2, + ImmutableList.of(sort("NAME", ASCENDING, LAST)), + tableScan("nation", ImmutableMap.of( + "NAME", "name"))))); + } + + @Test + public void testFetch() + { + assertPlan( + "SELECT * FROM nation FETCH FIRST 2 ROWS ONLY", + anyTree( + limit( + 2, + any( + tableScan("nation"))))); + } + + @Test + public void testWithTies() + { + assertPlan( + "SELECT name, regionkey FROM nation ORDER BY regionkey FETCH FIRST 6 ROWS WITH TIES", + any( + strictProject( + ImmutableMap.of("name", new ExpressionMatcher("name"), "regionkey", new ExpressionMatcher("regionkey")), + filter( + "rank_num <= BIGINT '6'", + window( + windowMatcherBuilder -> windowMatcherBuilder + .specification(specification( + ImmutableList.of(), + ImmutableList.of("regionkey"), + ImmutableMap.of("regionkey", SortOrder.ASC_NULLS_LAST))) + .addFunction( + "rank_num", + functionCall( + "rank", + Optional.empty(), + ImmutableList.of())), + anyTree( + sort( + ImmutableList.of(sort("regionkey", ASCENDING, LAST)), + any( + tableScan( + "nation", + ImmutableMap.of("NAME", "name", "REGIONKEY", "regionkey")))))))))); + } } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/LimitMatcher.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/LimitMatcher.java index 10ae84e36afb5..17f1bef779a2a 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/LimitMatcher.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/LimitMatcher.java @@ -17,19 +17,29 @@ import com.facebook.presto.cost.StatsProvider; import com.facebook.presto.metadata.Metadata; import com.facebook.presto.spi.plan.LimitNode; +import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.PlanNode; +import com.google.common.collect.ImmutableList; +import java.util.List; + +import static com.facebook.presto.sql.planner.assertions.MatchResult.NO_MATCH; +import static com.facebook.presto.sql.planner.assertions.MatchResult.match; +import static com.facebook.presto.sql.planner.assertions.Util.orderingSchemeMatches; import static com.google.common.base.Preconditions.checkState; +import static java.util.Objects.requireNonNull; public class LimitMatcher implements Matcher { private final long limit; + private final List tiesResolvers; private final boolean partial; - public LimitMatcher(long limit, boolean partial) + public LimitMatcher(long limit, List tiesResolvers, boolean partial) { this.limit = limit; + this.tiesResolvers = ImmutableList.copyOf(requireNonNull(tiesResolvers, "tiesResolvers is null")); this.partial = partial; } @@ -41,13 +51,22 @@ public boolean shapeMatches(PlanNode node) } LimitNode limitNode = (LimitNode) node; - return limitNode.getCount() == limit && limitNode.isPartial() == partial; + return limitNode.getCount() == limit + && limitNode.isWithTies() == !tiesResolvers.isEmpty() + && limitNode.isPartial() == partial; } @Override public MatchResult detailMatches(PlanNode node, StatsProvider stats, Session session, Metadata metadata, SymbolAliases symbolAliases) { checkState(shapeMatches(node)); - return MatchResult.match(); + if (!((LimitNode) node).isWithTies()) { + return match(); + } + OrderingScheme tiesResolvingScheme = ((LimitNode) node).getTiesResolvingScheme().get(); + if (orderingSchemeMatches(tiesResolvers, tiesResolvingScheme, symbolAliases)) { + return match(); + } + return NO_MATCH; } } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java index d3f83535833eb..1ec87ec245457 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/PlanMatchPattern.java @@ -541,12 +541,17 @@ public static PlanMatchPattern offset(long rowCount, PlanMatchPattern source) public static PlanMatchPattern limit(long limit, PlanMatchPattern source) { - return limit(limit, false, source); + return limit(limit, ImmutableList.of(), source); } - public static PlanMatchPattern limit(long limit, boolean partial, PlanMatchPattern source) + public static PlanMatchPattern limit(long limit, List tiesResolvers, PlanMatchPattern source) { - return node(LimitNode.class, source).with(new LimitMatcher(limit, partial)); + return limit(limit, tiesResolvers, false, source); + } + + public static PlanMatchPattern limit(long limit, List tiesResolvers, boolean partial, PlanMatchPattern source) + { + return node(LimitNode.class, source).with(new LimitMatcher(limit, tiesResolvers, partial)); } public static PlanMatchPattern enforceSingleRow(PlanMatchPattern source) diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestImplementLimitWithTies.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestImplementLimitWithTies.java new file mode 100644 index 0000000000000..32f02a764d958 --- /dev/null +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestImplementLimitWithTies.java @@ -0,0 +1,67 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.planner.iterative.rule; + +import com.facebook.presto.common.block.SortOrder; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.assertions.ExpressionMatcher; +import com.facebook.presto.sql.planner.iterative.rule.test.BaseRuleTest; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +import java.util.Optional; + +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.filter; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.functionCall; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.specification; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.strictProject; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.values; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.window; + +public class TestImplementLimitWithTies + extends BaseRuleTest +{ + @Test + public void testReplaceLimitWithTies() + { + tester().assertThat(new ImplementLimitWithTies(tester().getMetadata())) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression b = p.variable("b"); + return p.limit( + 2, + ImmutableList.of(a), + p.values(a, b)); + }) + .matches( + strictProject( + ImmutableMap.of("a", new ExpressionMatcher("a"), "b", new ExpressionMatcher("b")), + filter( + "rank_num <= BIGINT '2'", + window( + windowMatcherBuilder -> windowMatcherBuilder + .specification(specification( + ImmutableList.of(), + ImmutableList.of("a"), + ImmutableMap.of("a", SortOrder.ASC_NULLS_FIRST))) + .addFunction( + "rank_num", + functionCall( + "rank", + Optional.empty(), + ImmutableList.of())), + values("a", "b"))))); + } +} diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimitWithDistinct.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimitWithDistinct.java new file mode 100644 index 0000000000000..78d8da1dda515 --- /dev/null +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimitWithDistinct.java @@ -0,0 +1,83 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.planner.iterative.rule; + +import com.facebook.presto.spi.plan.DistinctLimitNode; +import com.facebook.presto.spi.plan.ValuesNode; +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.iterative.rule.test.BaseRuleTest; +import com.google.common.collect.ImmutableList; +import org.testng.annotations.Test; + +import static com.facebook.presto.common.type.BigintType.BIGINT; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.node; +import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.expression; + +public class TestMergeLimitWithDistinct + extends BaseRuleTest +{ + @Test + public void test() + { + tester().assertThat(new MergeLimitWithDistinct()) + .on(p -> + p.limit( + 1, + p.aggregation(builder -> builder + .singleGroupingSet(p.variable("foo")) + .source(p.values(p.variable("foo")))))) + .matches( + node(DistinctLimitNode.class, + node(ValuesNode.class))); + } + + @Test + public void testDoesNotFire() + { + tester().assertThat(new MergeLimitWithDistinct()) + .on(p -> + p.limit( + 1, + p.aggregation(builder -> builder + .addAggregation(p.variable("c"), expression("count(foo)"), ImmutableList.of(BIGINT)) + .globalGrouping() + .source(p.values(p.variable("foo")))))) + .doesNotFire(); + + tester().assertThat(new MergeLimitWithDistinct()) + .on(p -> + p.limit( + 1, + p.aggregation(builder -> builder + .globalGrouping() + .source(p.values(p.variable("foo")))))) + .doesNotFire(); + } + + @Test + public void testDoNotMergeLimitWithTies() + { + tester().assertThat(new MergeLimitWithDistinct()) + .on(p -> { + VariableReferenceExpression foo = p.variable("foo"); + return p.limit( + 1, + ImmutableList.of(foo), + p.aggregation(builder -> builder + .singleGroupingSet(foo) + .source(p.values(foo)))); + }) + .doesNotFire(); + } +} diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimits.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimits.java new file mode 100644 index 0000000000000..5703c3b62e678 --- /dev/null +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestMergeLimits.java @@ -0,0 +1,101 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.planner.iterative.rule; + +import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.iterative.rule.test.BaseRuleTest; +import com.google.common.collect.ImmutableList; +import org.testng.annotations.Test; + +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.limit; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.sort; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.values; +import static com.facebook.presto.sql.tree.SortItem.NullOrdering.FIRST; +import static com.facebook.presto.sql.tree.SortItem.Ordering.ASCENDING; + +public class TestMergeLimits + extends BaseRuleTest +{ + @Test + public void testMergeLimitsNoTies() + { + tester().assertThat(new MergeLimits()) + .on(p -> p.limit( + 5, + p.limit( + 3, + p.values()))) + .matches( + limit( + 3, + values())); + } + + @Test + public void testMergeLimitsNoTiesTies() + { + tester().assertThat(new MergeLimits()) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + return p.limit( + 3, + p.limit( + 5, + ImmutableList.of(a), + p.values(a))); + }) + .matches( + limit( + 3, + values("a"))); + } + + @Test + public void testRearrangeLimitsNoTiesTies() + { + tester().assertThat(new MergeLimits()) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + return p.limit( + 5, + p.limit( + 3, + ImmutableList.of(a), + p.values(a))); + }) + .matches( + limit( + 3, + ImmutableList.of(sort("a", ASCENDING, FIRST)), + limit( + 5, + values("a")))); + } + + @Test + public void testDoNotFireParentWithTies() + { + tester().assertThat(new MergeLimits()) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + return p.limit( + 5, + ImmutableList.of(a), + p.limit( + 3, + p.values(a))); + }) + .doesNotFire(); + } +} diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPruneLimitColumns.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPruneLimitColumns.java index 744c111dd13c7..f798984ff215c 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPruneLimitColumns.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPruneLimitColumns.java @@ -17,6 +17,7 @@ import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.iterative.rule.test.BaseRuleTest; import com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.testng.annotations.Test; @@ -57,6 +58,20 @@ public void testAllOutputsReferenced() .doesNotFire(); } + @Test + public void doNotPruneLimitWithTies() + { + tester().assertThat(new PruneLimitColumns()) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression b = p.variable("b"); + return p.project( + identityAssignmentsAsSymbolReferences(ImmutableList.of(b)), + p.limit(1, ImmutableList.of(a), p.values(a, b))); + }) + .doesNotFire(); + } + private ProjectNode buildProjectedLimit(PlanBuilder planBuilder, Predicate projectionFilter) { VariableReferenceExpression a = planBuilder.variable("a"); diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughMarkDistinct.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughMarkDistinct.java index 8f504c0c7d3b5..831ba535aea2b 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughMarkDistinct.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughMarkDistinct.java @@ -40,6 +40,22 @@ public void test() node(ValuesNode.class)))); } + @Test + public void testPushLimitWithTies() + { + tester().assertThat(new PushLimitThroughMarkDistinct()) + .on(p -> + p.limit( + 1, + ImmutableList.of(p.variable("foo")), + p.markDistinct( + p.variable("foo"), ImmutableList.of(p.variable("bar")), p.values()))) + .matches( + node(MarkDistinctNode.class, + node(LimitNode.class, + node(ValuesNode.class)))); + } + @Test public void testDoesNotFire() { diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughOuterJoin.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughOuterJoin.java index f8b884cdab1cf..fb927eac6797f 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughOuterJoin.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughOuterJoin.java @@ -48,7 +48,7 @@ public void testPushLimitThroughLeftJoin() join( LEFT, ImmutableList.of(equiJoinClause("leftKey", "rightKey")), - limit(1, true, values("leftKey")), + limit(1, ImmutableList.of(), true, values("leftKey")), values("rightKey")))); } @@ -85,4 +85,23 @@ public void testDoNotPushWhenAlreadyLimited() }) .doesNotFire(); } + + @Test + public void testDoNotPushLimitWithTies() + { + tester().assertThat(new PushLimitThroughOuterJoin()) + .on(p -> { + VariableReferenceExpression leftKey = p.variable("leftKey"); + VariableReferenceExpression rightKey = p.variable("rightKey"); + return p.limit( + 1, + ImmutableList.of(leftKey), + p.join( + LEFT, + p.values(5, leftKey), + p.values(5, rightKey), + new EquiJoinClause(leftKey, rightKey))); + }) + .doesNotFire(); + } } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughProject.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughProject.java index 19583c3fb4847..56ed739ecfe30 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughProject.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughProject.java @@ -13,18 +13,30 @@ */ package com.facebook.presto.sql.planner.iterative.rule; +import com.facebook.presto.spi.plan.Assignments; import com.facebook.presto.spi.relation.VariableReferenceExpression; +import com.facebook.presto.sql.planner.assertions.ExpressionMatcher; import com.facebook.presto.sql.planner.iterative.rule.test.BaseRuleTest; +import com.facebook.presto.sql.relational.OriginalExpressionUtils; +import com.facebook.presto.sql.tree.ArithmeticBinaryExpression; +import com.facebook.presto.sql.tree.SymbolReference; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.testng.annotations.Test; import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.expression; import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.limit; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.project; +import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.sort; import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.strictProject; import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.values; import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.assignment; +import static com.facebook.presto.sql.planner.iterative.rule.test.PlanBuilder.castToRowExpression; import static com.facebook.presto.sql.relational.OriginalExpressionUtils.asSymbolReference; +import static com.facebook.presto.sql.tree.ArithmeticBinaryExpression.Operator.ADD; import static com.facebook.presto.sql.tree.BooleanLiteral.TRUE_LITERAL; +import static com.facebook.presto.sql.tree.SortItem.NullOrdering.FIRST; +import static com.facebook.presto.sql.tree.SortItem.Ordering.ASCENDING; public class TestPushLimitThroughProject extends BaseRuleTest @@ -46,6 +58,72 @@ public void testPushdownLimitNonIdentityProjection() limit(1, values()))); } + @Test + public void testPushdownLimitWithTiesNNonIdentityProjection() + { + tester().assertThat(new PushLimitThroughProject()) + .on(p -> { + VariableReferenceExpression projectedA = p.variable("projectedA"); + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression projectedB = p.variable("projectedB"); + VariableReferenceExpression b = p.variable("b"); + return p.limit( + 1, + ImmutableList.of(projectedA), + p.project( + Assignments.of(projectedA, castToRowExpression("a"), projectedB, castToRowExpression("b")), + p.values(a, b))); + }) + .matches( + project( + limit(1, ImmutableList.of(sort("a", ASCENDING, FIRST)), values("a", "b")))); + } + + @Test + public void testPushdownLimitWithTiesThroughProjectionWithExpression() + { + tester().assertThat(new PushLimitThroughProject()) + .on(p -> { + VariableReferenceExpression projectedA = p.variable("projectedA"); + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression projectedC = p.variable("projectedC"); + VariableReferenceExpression b = p.variable("b"); + return p.limit( + 1, + ImmutableList.of(projectedA), + p.project( + Assignments.of( + projectedA, castToRowExpression("a"), + projectedC, OriginalExpressionUtils.castToRowExpression(new ArithmeticBinaryExpression(ADD, new SymbolReference("a"), new SymbolReference("b")))), + p.values(a, b))); + }) + .matches( + project( + ImmutableMap.of("projectedA", new ExpressionMatcher("a"), "projectedC", new ExpressionMatcher("a + b")), + limit(1, ImmutableList.of(sort("a", ASCENDING, FIRST)), values("a", "b")))); + } + + @Test + public void testDoNotPushdownLimitWithTiesThroughProjectionWithExpression() + { + tester().assertThat(new PushLimitThroughProject()) + .on(p -> { + VariableReferenceExpression projectedA = p.variable("projectedA"); + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression projectedC = p.variable("projectedC"); + VariableReferenceExpression b = p.variable("b"); + return p.limit( + 1, + ImmutableList.of(projectedC), + p.project( + Assignments.of( + projectedA, castToRowExpression("a"), + projectedC, OriginalExpressionUtils.castToRowExpression(new ArithmeticBinaryExpression(ADD, new SymbolReference("a"), new SymbolReference("b")))), + p.values(a, b))); + }) + .doesNotFire(); + } + @Test public void testDoesntPushdownLimitThroughIdentityProjection() { diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughUnion.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughUnion.java index a2a72678b8031..49abcfd8933dd 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughUnion.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestPushLimitThroughUnion.java @@ -47,8 +47,8 @@ public void testPushLimitThroughUnion() .matches( limit(1, union( - limit(1, true, values("a")), - limit(1, true, values("b"))))); + limit(1, ImmutableList.of(), true, values("a")), + limit(1, ImmutableList.of(), true, values("b"))))); } @Test @@ -87,4 +87,27 @@ public void doesNotFire() }) .doesNotFire(); } + + @Test + public void testDoNotPushLimitWithTies() + { + tester().assertThat(new PushLimitThroughUnion()) + .on(p -> { + VariableReferenceExpression a = p.variable("a"); + VariableReferenceExpression b = p.variable("b"); + VariableReferenceExpression c = p.variable("c"); + return p.limit( + 1, + ImmutableList.of(c), + p.union( + ImmutableListMultimap.builder() + .put(c, a) + .put(c, b) + .build(), + ImmutableList.of( + p.values(10, a), + p.values(10, b)))); + }) + .doesNotFire(); + } } diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java index 543938708060a..ba66adb21c7e1 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/test/PlanBuilder.java @@ -255,7 +255,23 @@ public OffsetNode offset(long rowCount, PlanNode source) public LimitNode limit(long limit, PlanNode source) { - return new LimitNode(idAllocator.getNextId(), source, limit, FINAL); + return limit(limit, ImmutableList.of(), source); + } + + public LimitNode limit(long limit, List tiesResolvers, PlanNode source) + { + Optional tiesResolvingScheme = Optional.empty(); + if (!tiesResolvers.isEmpty()) { + tiesResolvingScheme = Optional.of( + new OrderingScheme( + tiesResolvers.stream().map(variable -> new Ordering(variable, SortOrder.ASC_NULLS_FIRST)).collect(toImmutableList()))); + } + return new LimitNode( + idAllocator.getNextId(), + source, + limit, + tiesResolvingScheme, + FINAL); } public TopNNode topN(long count, List orderBy, PlanNode source) diff --git a/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 b/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 index db3da79a01dd4..5f7958c22aafa 100644 --- a/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 +++ b/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 @@ -218,7 +218,7 @@ queryNoWith: queryTerm (ORDER BY sortItem (',' sortItem)*)? (OFFSET offset=INTEGER_VALUE (ROW | ROWS)?)? - (LIMIT limit=(INTEGER_VALUE | ALL))? + ((LIMIT limit=(INTEGER_VALUE | ALL)) | (FETCH (FIRST | NEXT) (fetchFirst=INTEGER_VALUE)? (ROW | ROWS) (ONLY | WITH TIES)))? ; queryTerm @@ -560,20 +560,20 @@ nonReserved | CALL | CALLED | CASCADE | CATALOGS | COLUMN | COLUMNS | COMMENT | COMMIT | COMMITTED | CURRENT | CURRENT_ROLE | DATA | DATE | DAY | DEFINER | DESC | DETERMINISTIC | DISTRIBUTED | EXCLUDING | EXPLAIN | EXTERNAL - | FILTER | FIRST | FOLLOWING | FORMAT | FUNCTION | FUNCTIONS + | FETCH | FILTER | FIRST | FOLLOWING | FORMAT | FUNCTION | FUNCTIONS | GRANT | GRANTED | GRANTS | GRAPHVIZ | HOUR | IF | IGNORE | INCLUDING | INPUT | INTERVAL | INVOKER | IO | ISOLATION | JSON | LANGUAGE | LAST | LATERAL | LEVEL | LIMIT | LOGICAL | MAP | MATERIALIZED | MINUTE | MONTH - | NAME | NFC | NFD | NFKC | NFKD | NO | NONE | NULLIF | NULLS + | NAME | NEXT | NFC | NFD | NFKC | NFKD | NO | NONE | NULLIF | NULLS | OFFSET | ONLY | OPTION | ORDINALITY | OUTPUT | OVER | PARTITION | PARTITIONS | POSITION | PRECEDING | PRIVILEGES | PROPERTIES | RANGE | READ | REFRESH | RENAME | REPEATABLE | REPLACE | RESET | RESPECT | RESTRICT | RETURN | RETURNS | REVOKE | ROLE | ROLES | ROLLBACK | ROW | ROWS | SCHEMA | SCHEMAS | SECOND | SECURITY | SERIALIZABLE | SESSION | SET | SETS | SQL | SHOW | SOME | START | STATS | SUBSTRING | SYSTEM - | TABLES | TABLESAMPLE | TEMPORARY | TEXT | TIME | TIMESTAMP | TO | TRANSACTION | TRY_CAST | TYPE + | TABLES | TABLESAMPLE | TEMPORARY | TEXT | TIES | TIME | TIMESTAMP | TO | TRANSACTION | TRY_CAST | TYPE | UNBOUNDED | UNCOMMITTED | USE | USER | VALIDATE | VERBOSE | VIEW | WORK | WRITE @@ -639,6 +639,7 @@ EXPLAIN: 'EXPLAIN'; EXTRACT: 'EXTRACT'; EXTERNAL: 'EXTERNAL'; FALSE: 'FALSE'; +FETCH: 'FETCH'; FILTER: 'FILTER'; FIRST: 'FIRST'; FOLLOWING: 'FOLLOWING'; @@ -688,6 +689,7 @@ MINUTE: 'MINUTE'; MONTH: 'MONTH'; NAME: 'NAME'; NATURAL: 'NATURAL'; +NEXT: 'NEXT'; NFC : 'NFC'; NFD : 'NFD'; NFKC : 'NFKC'; @@ -758,6 +760,7 @@ TABLESAMPLE: 'TABLESAMPLE'; TEMPORARY: 'TEMPORARY'; TEXT: 'TEXT'; THEN: 'THEN'; +TIES: 'TIES'; TIME: 'TIME'; TIMESTAMP: 'TIMESTAMP'; TO: 'TO'; diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/QueryUtil.java b/presto-parser/src/main/java/com/facebook/presto/sql/QueryUtil.java index 126c2b10331e0..e471975ca1192 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/QueryUtil.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/QueryUtil.java @@ -22,6 +22,7 @@ import com.facebook.presto.sql.tree.GroupBy; import com.facebook.presto.sql.tree.Identifier; import com.facebook.presto.sql.tree.LogicalBinaryExpression; +import com.facebook.presto.sql.tree.Node; import com.facebook.presto.sql.tree.Offset; import com.facebook.presto.sql.tree.OrderBy; import com.facebook.presto.sql.tree.QualifiedName; @@ -206,7 +207,7 @@ public static Query simpleQuery(Select select, Relation from, Optional where, Optional groupBy, Optional having, Optional orderBy, Optional offset, Optional limit) + public static Query simpleQuery(Select select, Relation from, Optional where, Optional groupBy, Optional having, Optional orderBy, Optional offset, Optional limit) { return query(new QuerySpecification( select, diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java index d677404ba132a..a74458558e484 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java @@ -50,6 +50,7 @@ import com.facebook.presto.sql.tree.ExplainType; import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.ExternalBodyReference; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.Grant; import com.facebook.presto.sql.tree.GrantRoles; import com.facebook.presto.sql.tree.GrantorSpecification; @@ -63,6 +64,7 @@ import com.facebook.presto.sql.tree.JoinUsing; import com.facebook.presto.sql.tree.Lateral; import com.facebook.presto.sql.tree.LikeClause; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.NaturalJoin; import com.facebook.presto.sql.tree.Node; import com.facebook.presto.sql.tree.Offset; @@ -281,8 +283,7 @@ protected Void visitQuery(Query node, Integer indent) } if (node.getLimit().isPresent()) { - append(indent, "LIMIT " + node.getLimit().get()) - .append('\n'); + process(node.getLimit().get(), indent); } return null; @@ -325,8 +326,7 @@ protected Void visitQuerySpecification(QuerySpecification node, Integer indent) } if (node.getLimit().isPresent()) { - append(indent, "LIMIT " + node.getLimit().get()) - .append('\n'); + process(node.getLimit().get(), indent); } return null; } @@ -347,6 +347,23 @@ protected Void visitOrderBy(OrderBy node, Integer indent) return null; } + @Override + protected Void visitFetchFirst(FetchFirst node, Integer indent) + { + append(indent, "FETCH FIRST " + node.getRowCount().map(c -> c + " ROWS ").orElse("ROW ")) + .append(node.isWithTies() ? "WITH TIES" : "ONLY") + .append('\n'); + return null; + } + + @Override + protected Void visitLimit(Limit node, Integer indent) + { + append(indent, "LIMIT " + node.getLimit()) + .append('\n'); + return null; + } + @Override protected Void visitSelect(Select node, Integer indent) { diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java b/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java index d291e1a4d91b9..e3b73ffe5aafe 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java @@ -69,6 +69,7 @@ import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.ExternalBodyReference; import com.facebook.presto.sql.tree.Extract; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.FrameBound; import com.facebook.presto.sql.tree.FunctionCall; import com.facebook.presto.sql.tree.GenericLiteral; @@ -98,6 +99,7 @@ import com.facebook.presto.sql.tree.Lateral; import com.facebook.presto.sql.tree.LikeClause; import com.facebook.presto.sql.tree.LikePredicate; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.LogicalBinaryExpression; import com.facebook.presto.sql.tree.LongLiteral; import com.facebook.presto.sql.tree.NaturalJoin; @@ -705,10 +707,19 @@ public Node visitQueryNoWith(SqlBaseParser.QueryNoWithContext context) offset = Optional.of(new Offset(Optional.of(getLocation(context.OFFSET())), getTextIfPresent(context.offset).orElseThrow(() -> new IllegalStateException("Missing OFFSET row count")))); } + Optional limit = Optional.empty(); + if (context.FETCH() != null) { + limit = Optional.of(new FetchFirst(Optional.of(getLocation(context.FETCH())), getTextIfPresent(context.fetchFirst), context.TIES() != null)); + } + else if (context.LIMIT() != null) { + limit = Optional.of(new Limit(Optional.of(getLocation(context.LIMIT())), getTextIfPresent(context.limit).orElseThrow(() -> new IllegalStateException("Missing LIMIT value")))); + } + if (term instanceof QuerySpecification) { // When we have a simple query specification - // followed by order by, offset, limit, fold the order by and limit - // clauses into the query specification (analyzer/planner + // followed by order by, offset, limit or fetch, + // fold the order by, limit, offset or fetch clauses + // into the query specification (analyzer/planner // expects this structure to resolve references with respect // to columns defined in the query specification) QuerySpecification query = (QuerySpecification) term; @@ -725,7 +736,7 @@ public Node visitQueryNoWith(SqlBaseParser.QueryNoWithContext context) query.getHaving(), orderBy, offset, - getTextIfPresent(context.limit)), + limit), Optional.empty(), Optional.empty(), Optional.empty()); @@ -737,7 +748,7 @@ public Node visitQueryNoWith(SqlBaseParser.QueryNoWithContext context) term, orderBy, offset, - getTextIfPresent(context.limit)); + limit); } @Override diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java index d1d70891b7fd2..e260b6d2450ce 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java @@ -232,6 +232,16 @@ protected R visitOffset(Offset node, C context) return visitNode(node, context); } + protected R visitFetchFirst(FetchFirst node, C context) + { + return visitNode(node, context); + } + + protected R visitLimit(Limit node, C context) + { + return visitNode(node, context); + } + protected R visitQuerySpecification(QuerySpecification node, C context) { return visitQueryBody(node, context); diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/FetchFirst.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/FetchFirst.java new file mode 100644 index 0000000000000..2f927c01892c5 --- /dev/null +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/FetchFirst.java @@ -0,0 +1,108 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.tree; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import static com.google.common.base.MoreObjects.toStringHelper; + +public class FetchFirst + extends Node +{ + private final Optional rowCount; + private final boolean withTies; + + public FetchFirst(String rowCount) + { + this(Optional.empty(), Optional.of(rowCount), false); + } + + public FetchFirst(String rowCount, boolean withTies) + { + this(Optional.empty(), Optional.of(rowCount), withTies); + } + + public FetchFirst(Optional rowCount) + { + this(Optional.empty(), rowCount, false); + } + + public FetchFirst(Optional rowCount, boolean withTies) + { + this(Optional.empty(), rowCount, withTies); + } + + public FetchFirst(Optional location, Optional rowCount, boolean withTies) + { + super(location); + this.rowCount = rowCount; + this.withTies = withTies; + } + + public Optional getRowCount() + { + return rowCount; + } + + public boolean isWithTies() + { + return withTies; + } + + @Override + public R accept(AstVisitor visitor, C context) + { + return visitor.visitFetchFirst(this, context); + } + + @Override + public List getChildren() + { + return ImmutableList.of(); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if ((o == null) || (getClass() != o.getClass())) { + return false; + } + FetchFirst that = (FetchFirst) o; + return withTies == that.withTies && + Objects.equals(rowCount, that.rowCount); + } + + @Override + public int hashCode() + { + return Objects.hash(rowCount, withTies); + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("rowCount", rowCount.orElse(null)) + .add("withTies", withTies) + .omitNullValues() + .toString(); + } +} diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/Limit.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Limit.java new file mode 100644 index 0000000000000..9f4d2f0f2123c --- /dev/null +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Limit.java @@ -0,0 +1,88 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.facebook.presto.sql.tree; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import static com.google.common.base.MoreObjects.toStringHelper; + +public class Limit + extends Node +{ + private final String limit; + + public Limit(String limit) + { + this(Optional.empty(), limit); + } + + public Limit(NodeLocation location, String limit) + { + this(Optional.of(location), limit); + } + + public Limit(Optional location, String limit) + { + super(location); + this.limit = limit; + } + + public String getLimit() + { + return limit; + } + + @Override + public R accept(AstVisitor visitor, C context) + { + return visitor.visitLimit(this, context); + } + + @Override + public List getChildren() + { + return ImmutableList.of(); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) { + return true; + } + if ((obj == null) || (getClass() != obj.getClass())) { + return false; + } + Limit o = (Limit) obj; + return Objects.equals(limit, o.limit); + } + + @Override + public int hashCode() + { + return Objects.hash(limit); + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("limit", limit) + .toString(); + } +} diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/Query.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Query.java index 662f3b73f05b7..4f8a7d455964c 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/Query.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Query.java @@ -20,6 +20,7 @@ import java.util.Optional; import static com.google.common.base.MoreObjects.toStringHelper; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; public class Query @@ -28,15 +29,15 @@ public class Query private final Optional with; private final QueryBody queryBody; private final Optional orderBy; - private final Optional limit; private final Optional offset; + private final Optional limit; public Query( Optional with, QueryBody queryBody, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { this(Optional.empty(), with, queryBody, orderBy, offset, limit); } @@ -47,7 +48,7 @@ public Query( QueryBody queryBody, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { this(Optional.of(location), with, queryBody, orderBy, offset, limit); } @@ -58,7 +59,7 @@ private Query( QueryBody queryBody, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { super(location); requireNonNull(with, "with is null"); @@ -66,6 +67,11 @@ private Query( requireNonNull(orderBy, "orderBy is null"); requireNonNull(offset, "offset is null"); requireNonNull(limit, "limit is null"); + checkArgument( + !limit.isPresent() || + limit.get() instanceof FetchFirst || + limit.get() instanceof Limit, + "limit must be optional of either FetchFirst or Limit type"); this.with = with; this.queryBody = queryBody; @@ -89,7 +95,7 @@ public Optional getOrderBy() return orderBy; } - public Optional getLimit() + public Optional getLimit() { return limit; } @@ -113,6 +119,7 @@ public List getChildren() nodes.add(queryBody); orderBy.ifPresent(nodes::add); offset.ifPresent(nodes::add); + limit.ifPresent(nodes::add); return nodes.build(); } diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/QuerySpecification.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/QuerySpecification.java index cfee64646eb09..5a5bdb8b81880 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/QuerySpecification.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/QuerySpecification.java @@ -20,6 +20,7 @@ import java.util.Optional; import static com.google.common.base.MoreObjects.toStringHelper; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; public class QuerySpecification @@ -32,7 +33,7 @@ public class QuerySpecification private final Optional having; private final Optional orderBy; private final Optional offset; - private final Optional limit; + private final Optional limit; public QuerySpecification( Select select, @@ -42,7 +43,7 @@ public QuerySpecification( Optional having, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { this(Optional.empty(), select, from, where, groupBy, having, orderBy, offset, limit); } @@ -56,7 +57,7 @@ public QuerySpecification( Optional having, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { this(Optional.of(location), select, from, where, groupBy, having, orderBy, offset, limit); } @@ -70,7 +71,7 @@ private QuerySpecification( Optional having, Optional orderBy, Optional offset, - Optional limit) + Optional limit) { super(location); requireNonNull(select, "select is null"); @@ -81,6 +82,11 @@ private QuerySpecification( requireNonNull(orderBy, "orderBy is null"); requireNonNull(offset, "offset is null"); requireNonNull(limit, "limit is null"); + checkArgument( + !limit.isPresent() + || limit.get() instanceof FetchFirst + || limit.get() instanceof Limit, + "limit must be optional of either FetchFirst or Limit type"); this.select = select; this.from = from; @@ -127,7 +133,7 @@ public Optional getOffset() return offset; } - public Optional getLimit() + public Optional getLimit() { return limit; } @@ -149,6 +155,7 @@ public List getChildren() having.ifPresent(nodes::add); orderBy.ifPresent(nodes::add); offset.ifPresent(nodes::add); + limit.ifPresent(nodes::add); return nodes.build(); } diff --git a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java index ef659818d3ed5..0cc2734d15acb 100644 --- a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java +++ b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java @@ -62,6 +62,7 @@ import com.facebook.presto.sql.tree.ExplainFormat; import com.facebook.presto.sql.tree.ExplainType; import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.FunctionCall; import com.facebook.presto.sql.tree.GenericLiteral; import com.facebook.presto.sql.tree.Grant; @@ -84,6 +85,7 @@ import com.facebook.presto.sql.tree.LambdaExpression; import com.facebook.presto.sql.tree.Lateral; import com.facebook.presto.sql.tree.LikeClause; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.LogicalBinaryExpression; import com.facebook.presto.sql.tree.LongLiteral; import com.facebook.presto.sql.tree.NaturalJoin; @@ -535,8 +537,40 @@ public void testBetween() } @Test - public void testLimitAll() + public void testSelectWithLimit() { + assertStatement("SELECT * FROM table1 LIMIT 2", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new Limit("2"))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + + assertStatement("SELECT * FROM table1 LIMIT ALL", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new Limit("ALL"))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + Query valuesQuery = query(values( row(new LongLiteral("1"), new StringLiteral("1")), row(new LongLiteral("2"), new StringLiteral("2")))); @@ -549,7 +583,7 @@ public void testLimitAll() Optional.empty(), Optional.empty(), Optional.empty(), - Optional.of("ALL"))); + Optional.of(new Limit("ALL")))); } @Test @@ -952,7 +986,7 @@ public void testSelectWithOffset() Optional.empty(), Optional.of(new OrderBy(ImmutableList.of(new SortItem(new Identifier("x"), ASCENDING, UNDEFINED)))), Optional.of(new Offset("2")), - Optional.of("10")), + Optional.of(new Limit("10"))), Optional.empty(), Optional.empty(), Optional.empty())); @@ -982,6 +1016,104 @@ public void testSelectWithOffset() Optional.empty())); } + @Test + public void testSelectWithFetch() + { + assertStatement("SELECT * FROM table1 FETCH FIRST 2 ROWS ONLY", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst("2"))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + + assertStatement("SELECT * FROM table1 FETCH NEXT ROW ONLY", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst(Optional.empty()))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + + Query valuesQuery = query(values( + row(new LongLiteral("1"), new StringLiteral("1")), + row(new LongLiteral("2"), new StringLiteral("2")))); + + assertStatement("SELECT * FROM (VALUES (1, '1'), (2, '2')) FETCH FIRST ROW ONLY", + simpleQuery(selectList(new AllColumns()), + subquery(valuesQuery), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst(Optional.empty())))); + + assertStatement("SELECT * FROM (VALUES (1, '1'), (2, '2')) FETCH FIRST ROW WITH TIES", + simpleQuery(selectList(new AllColumns()), + subquery(valuesQuery), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst(Optional.empty(), true)))); + + assertStatement("SELECT * FROM table1 FETCH FIRST 2 ROWS WITH TIES", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst("2", true))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + + assertStatement("SELECT * FROM table1 FETCH NEXT ROW WITH TIES", + new Query( + Optional.empty(), + new QuerySpecification( + selectList(new AllColumns()), + Optional.of(new Table(QualifiedName.of("table1"))), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new FetchFirst(Optional.empty(), true))), + Optional.empty(), + Optional.empty(), + Optional.empty())); + } + + @Test + public void testSelectFetchFirstException() + { + assertInvalidStatement("SELECT * FROM t1 FETCH FIRST -1 ROWS ONLY", + "mismatched input '-'. Expecting: 'ROW', 'ROWS', "); + } @Test public void testSelectWithGroupBy() { diff --git a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParserErrorHandling.java b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParserErrorHandling.java index dd4ede017c703..fbc3dc123b52c 100644 --- a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParserErrorHandling.java +++ b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParserErrorHandling.java @@ -49,7 +49,7 @@ public Object[][] getStatements() {"select * from 'oops", "line 1:15: mismatched input '''. Expecting: '(', 'LATERAL', 'UNNEST', "}, {"select *\nfrom x\nfrom", - "line 3:1: mismatched input 'from'. Expecting: ',', '.', 'AS', 'CROSS', 'EXCEPT', 'FULL', 'GROUP', 'HAVING', 'INNER', 'INTERSECT', 'JOIN', 'LEFT', 'LIMIT', 'NATURAL', 'OFFSET', " + + "line 3:1: mismatched input 'from'. Expecting: ',', '.', 'AS', 'CROSS', 'EXCEPT', 'FETCH', 'FULL', 'GROUP', 'HAVING', 'INNER', 'INTERSECT', 'JOIN', 'LEFT', 'LIMIT', 'NATURAL', 'OFFSET', " + "'ORDER', 'RIGHT', 'TABLESAMPLE', 'UNION', 'WHERE', , "}, {"select *\nfrom x\nwhere from", "line 3:7: mismatched input 'from'. Expecting: "}, @@ -112,7 +112,7 @@ public Object[][] getStatements() {"SELECT foo(*) filter (", "line 1:23: mismatched input ''. Expecting: 'WHERE'"}, {"SELECT * FROM t t x", - "line 1:19: mismatched input 'x'. Expecting: '(', ',', 'CROSS', 'EXCEPT', 'FULL', 'GROUP', 'HAVING', 'INNER', 'INTERSECT', 'JOIN', 'LEFT', 'LIMIT', 'NATURAL', 'OFFSET', 'ORDER', " + + "line 1:19: mismatched input 'x'. Expecting: '(', ',', 'CROSS', 'EXCEPT', 'FETCH', 'FULL', 'GROUP', 'HAVING', 'INNER', 'INTERSECT', 'JOIN', 'LEFT', 'LIMIT', 'NATURAL', 'OFFSET', 'ORDER', " + "'RIGHT', 'TABLESAMPLE', 'UNION', 'WHERE', "}, {"SELECT * FROM t WHERE EXISTS (", "line 1:31: mismatched input ''. Expecting: "}}; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/plan/LimitNode.java b/presto-spi/src/main/java/com/facebook/presto/spi/plan/LimitNode.java index 77cfe476b3b97..d4c7ea7e2f2bd 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/plan/LimitNode.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/plan/LimitNode.java @@ -22,6 +22,7 @@ import javax.annotation.concurrent.Immutable; import java.util.List; +import java.util.Optional; import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR; import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; @@ -46,20 +47,29 @@ public enum Step private final PlanNode source; private final long count; + private final Optional tiesResolvingScheme; private final Step step; + public LimitNode(PlanNodeId id, PlanNode source, long count, Step partial) + { + this(id, source, count, Optional.empty(), partial); + } + @JsonCreator public LimitNode( @JsonProperty("id") PlanNodeId id, @JsonProperty("source") PlanNode source, @JsonProperty("count") long count, + @JsonProperty("tiesResolvingScheme") Optional tiesResolvingScheme, @JsonProperty("step") Step step) { super(id); checkCondition(count >= 0, INVALID_FUNCTION_ARGUMENT, "count must be greater than or equal to zero"); + requireNonNull(tiesResolvingScheme, "tiesResolvingScheme is null"); this.source = requireNonNull(source, "source is null"); this.count = count; + this.tiesResolvingScheme = requireNonNull(tiesResolvingScheme, "tiesResolvingScheme is null"); this.step = requireNonNull(step, "step is null"); } @@ -87,6 +97,17 @@ public long getCount() return count; } + public boolean isWithTies() + { + return tiesResolvingScheme.isPresent(); + } + + @JsonProperty + public Optional getTiesResolvingScheme() + { + return tiesResolvingScheme; + } + @JsonProperty public Step getStep() { @@ -114,7 +135,7 @@ public R accept(PlanVisitor visitor, C context) public PlanNode replaceChildren(List newChildren) { checkCondition(newChildren != null && newChildren.size() == 1, GENERIC_INTERNAL_ERROR, "Expect exactly 1 child PlanNode"); - return new LimitNode(getId(), newChildren.get(0), count, getStep()); + return new LimitNode(getId(), newChildren.get(0), count, tiesResolvingScheme, getStep()); } private static void checkCondition(boolean condition, ErrorCodeSupplier errorCode, String message) diff --git a/presto-tests/src/test/java/com/facebook/presto/tests/AbstractTestEngineOnlyQueries.java b/presto-tests/src/test/java/com/facebook/presto/tests/AbstractTestEngineOnlyQueries.java index c2020d8bac6a2..746ae424336ba 100644 --- a/presto-tests/src/test/java/com/facebook/presto/tests/AbstractTestEngineOnlyQueries.java +++ b/presto-tests/src/test/java/com/facebook/presto/tests/AbstractTestEngineOnlyQueries.java @@ -92,4 +92,25 @@ public void testLocallyUnrepresentableTimeLiterals() assertEquals(computeScalar(sql), localTimeThatDidNotOccurOn19700101); // this tests Presto and the QueryRunner assertQuery(sql); // this tests H2QueryRunner } + + @Test + public void testFetchFirstWithTies() + { + String values = "(VALUES 1, 1, 1, 0, 0, 0, 2, 2, 2) AS t(x)"; + + assertQuery("SELECT x FROM " + values + " ORDER BY x FETCH FIRST 4 ROWS WITH TIES", "VALUES 0, 0, 0, 1, 1, 1"); + assertQuery("SELECT x FROM " + values + " ORDER BY x FETCH FIRST ROW WITH TIES", "VALUES 0, 0, 0"); + assertQuery("SELECT x FROM " + values + " ORDER BY x FETCH FIRST 20 ROWS WITH TIES", "VALUES 0, 0, 0, 1, 1, 1, 2, 2, 2"); + + assertQueryFails("SELECT x FROM " + values + " FETCH FIRST 4 ROWS WITH TIES", "line 1:58: FETCH FIRST WITH TIES clause requires ORDER BY"); + assertQueryFails( + "SELECT x FROM (SELECT a FROM (VALUES 3, 2, 1, 1, 0) t(a) ORDER BY a) t1(x) FETCH FIRST 2 ROWS WITH TIES", + "line 1:76: FETCH FIRST WITH TIES clause requires ORDER BY"); + + String valuesMultiColumn = "(VALUES ('b', 0), ('b', 0), ('a', 1), ('a', 0), ('b', 1)) AS t(x, y)"; + + // if ORDER BY uses multiple symbols, then TIES are resolved basing on multiple symbols too + assertQuery("SELECT x, y FROM " + valuesMultiColumn + " ORDER BY x, y FETCH FIRST 3 ROWS WITH TIES", "VALUES ('a', 0), ('a', 1), ('b', 0), ('b', 0)"); + assertQuery("SELECT x, y FROM " + valuesMultiColumn + " ORDER BY x DESC, y FETCH FIRST ROW WITH TIES", "VALUES ('b', 0), ('b', 0)"); + } } diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/LimitQueryDeterminismAnalyzer.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/LimitQueryDeterminismAnalyzer.java index 6b3637e893af0..09fbe694cd6c6 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/LimitQueryDeterminismAnalyzer.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/LimitQueryDeterminismAnalyzer.java @@ -14,12 +14,17 @@ package com.facebook.presto.verifier.framework; import com.facebook.presto.jdbc.QueryStats; +import com.facebook.presto.sql.analyzer.SemanticException; +import com.facebook.presto.sql.tree.AstVisitor; import com.facebook.presto.sql.tree.CreateTableAsSelect; import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.FetchFirst; import com.facebook.presto.sql.tree.FunctionCall; import com.facebook.presto.sql.tree.Identifier; import com.facebook.presto.sql.tree.Insert; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.LongLiteral; +import com.facebook.presto.sql.tree.Node; import com.facebook.presto.sql.tree.OrderBy; import com.facebook.presto.sql.tree.QualifiedName; import com.facebook.presto.sql.tree.Query; @@ -44,6 +49,8 @@ import java.util.Set; import static com.facebook.presto.sql.QueryUtil.simpleQuery; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_FETCH_FIRST_ROW_COUNT; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_LIMIT_ROW_COUNT; import static com.facebook.presto.verifier.framework.LimitQueryDeterminismAnalysis.DETERMINISTIC; import static com.facebook.presto.verifier.framework.LimitQueryDeterminismAnalysis.FAILED_DATA_CHANGED; import static com.facebook.presto.verifier.framework.LimitQueryDeterminismAnalysis.FAILED_QUERY_FAILURE; @@ -149,18 +156,67 @@ private LimitQueryDeterminismAnalysis analyzeQuery(Query query) if (query.getOrderBy().isPresent() || !query.getLimit().isPresent()) { return NOT_RUN; } - if (isLimitAll(query.getLimit().get())) { + String queryLimit = analyzeLimit(query.getLimit().get()); + if (isLimitAll(queryLimit)) { return NOT_RUN; } - long limit = parseLong(query.getLimit().get()); + long limit = parseLong(queryLimit); if (rowCount < limit) { return DETERMINISTIC; } - Optional newLimit = Optional.of(Long.toString(limit + 1)); + Optional newLimit = Optional.of(new Limit(Long.toString(limit + 1))); Query newLimitQuery = new Query(query.getWith(), query.getQueryBody(), Optional.empty(), query.getOffset(), newLimit); return analyzeLimitNoOrderBy(newLimitQuery, limit); } + private String analyzeLimit(Node node) + { + checkState( + node instanceof FetchFirst || node instanceof Limit, + "Invalid limit node type. Expected: FetchFirst or Limit. Actual: %s", node.getClass().getName()); + return new AstVisitor() + { + @Override + protected String visitFetchFirst(FetchFirst node, Void context) + { + if (!node.getRowCount().isPresent()) { + return Long.toString(1); + } + else { + long rowCount; + try { + rowCount = Long.parseLong(node.getRowCount().get()); + } + catch (NumberFormatException e) { + throw new SemanticException(INVALID_FETCH_FIRST_ROW_COUNT, node, "Invalid FETCH FIRST row count: %s", node.getRowCount().get()); + } + if (rowCount <= 0) { + throw new SemanticException(INVALID_FETCH_FIRST_ROW_COUNT, node, "FETCH FIRST row count must be positive (actual value: %s)", rowCount); + } + return Long.toString(rowCount); + } + } + + @Override + protected String visitLimit(Limit node, Void context) + { + if (node.getLimit().equalsIgnoreCase("all")) { + return Long.toString(0); + } + else { + long rowCount; + try { + rowCount = Long.parseLong(node.getLimit()); + } + catch (NumberFormatException e) { + throw new SemanticException(INVALID_LIMIT_ROW_COUNT, node, "Invalid LIMIT row count: %s", node.getLimit()); + } + return Long.toString(rowCount); + } + } + }.process(node, null); + } + /** * To check whether all ORDER BY columns are matching between the n-th and the (n+1)-th row, we * may need to project additional columns. Takes in the list of SelectItems of the original query @@ -210,14 +266,16 @@ private LimitQueryDeterminismAnalysis analyzeQuerySpecification(Optional w if (!querySpecification.getLimit().isPresent()) { return NOT_RUN; } - if (isLimitAll(querySpecification.getLimit().get())) { + String queryLimit = analyzeLimit(querySpecification.getLimit().get()); + if (isLimitAll(queryLimit)) { return NOT_RUN; } - long limit = parseLong(querySpecification.getLimit().get()); + + long limit = parseLong(queryLimit); if (rowCount < limit) { return DETERMINISTIC; } - Optional newLimit = Optional.of(Long.toString(limit + 1)); + Optional newLimit = Optional.of(new Limit(Long.toString(limit + 1))); Optional orderBy = querySpecification.getOrderBy(); if (orderBy.isPresent()) { @@ -308,7 +366,7 @@ private LimitQueryDeterminismAnalysis analyzeLimitOrderBy(Query tieInspectorQuer private static boolean isLimitAll(String limitClause) { - return limitClause.toLowerCase(ENGLISH).equals("all"); + return limitClause.toLowerCase(ENGLISH).equals("0"); } private static class ColumnNameOrIndex diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java index 00c91700975d9..d4a48e2bef48c 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java @@ -33,6 +33,7 @@ import com.facebook.presto.sql.tree.Identifier; import com.facebook.presto.sql.tree.Insert; import com.facebook.presto.sql.tree.LikeClause; +import com.facebook.presto.sql.tree.Limit; import com.facebook.presto.sql.tree.Property; import com.facebook.presto.sql.tree.QualifiedName; import com.facebook.presto.sql.tree.Query; @@ -273,13 +274,13 @@ private ResultSetMetaData getResultMetadata(Query query) querySpecification.getHaving(), querySpecification.getOrderBy(), querySpecification.getOffset(), - Optional.of("0")), + Optional.of(new Limit("0"))), Optional.empty(), Optional.empty(), Optional.empty()); } else { - zeroRowQuery = new Query(query.getWith(), query.getQueryBody(), Optional.empty(), Optional.empty(), Optional.of("0")); + zeroRowQuery = new Query(query.getWith(), query.getQueryBody(), Optional.empty(), Optional.empty(), Optional.of(new Limit("0"))); } return prestoAction.execute(zeroRowQuery, REWRITE, ResultSetConverter.DEFAULT).getMetadata(); }