phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Taylor (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PHOENIX-2722) support mysql "limit,offset" clauses
Date Thu, 17 Mar 2016 00:12:33 GMT

    [ https://issues.apache.org/jira/browse/PHOENIX-2722?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15198464#comment-15198464
] 

James Taylor commented on PHOENIX-2722:
---------------------------------------

I don't understand the need for [ FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } ONLY ] .
I thought this was about adding support for OFFSET?

Regarding the white space changes, we gave you a free pass before with the agreement that
it'd get better next time. This patch is 3649 lines with a lot of them appearing to just be
white space related or pure formatting. I think it's fine if a line is changing that the formatting
can change (presuming it follows our conventions[1]), but I don't think the entire file should
be reformatted. If you could make a pass through the patch and copy/paste the original code
where you've made purely white space/formatting changes, that would be much appreciated.

Here's one example:
{code}
@@ -194,26 +183,25 @@ public class SubqueryRewriter extends ParseNodeRewriter {
         if (isTopNode) {
             topNode = null;
         }
-        
+
         ParseNode secondChild = l.get(1);
-        if (!(secondChild instanceof SubqueryParseNode)) {
-            return super.visitLeave(node, l);
-        }
-        
-        SubqueryParseNode subqueryNode = (SubqueryParseNode) secondChild;
+        if (!(secondChild instanceof SubqueryParseNode)) { return super.visitLeave(node,
l); }
+
+        SubqueryParseNode subqueryNode = (SubqueryParseNode)secondChild;
         SelectStatement subquery = fixSubqueryStatement(subqueryNode.getSelectNode());
         String rhsTableAlias = ParseNodeFactory.createTempAlias();
-        JoinConditionExtractor conditionExtractor = new JoinConditionExtractor(subquery,
resolver, connection, rhsTableAlias);
+        JoinConditionExtractor conditionExtractor = new JoinConditionExtractor(subquery,
resolver, connection,
+                rhsTableAlias);
         ParseNode where = subquery.getWhere() == null ? null : subquery.getWhere().accept(conditionExtractor);
         if (where == subquery.getWhere()) { // non-correlated comparison subquery, add LIMIT
2, expectSingleRow = true
-            subquery = NODE_FACTORY.select(subquery, NODE_FACTORY.limit(NODE_FACTORY.literal(2)));
+            subquery = NODE_FACTORY.select(subquery, NODE_FACTORY.limit(NODE_FACTORY.literal(2)),
null);
             subqueryNode = NODE_FACTORY.subquery(subquery, true);
             l = Lists.newArrayList(l.get(0), subqueryNode);
             node = NODE_FACTORY.comparison(node.getFilterOp(), l.get(0), l.get(1));
             return super.visitLeave(node, l);
         }
-        
-        ParseNode rhsNode = null; 
+
+        ParseNode rhsNode = null;
         boolean isGroupby = !subquery.getGroupBy().isEmpty();
         boolean isAggregate = subquery.isAggregate();
         List<AliasedNode> aliasedNodes = subquery.getSelect();
@@ -226,52 +214,52 @@ public class SubqueryRewriter extends ParseNodeRewriter {
             }
             rhsNode = NODE_FACTORY.rowValueConstructor(nodes);
         }
-        
+
         List<AliasedNode> additionalSelectNodes = conditionExtractor.getAdditionalSelectNodes();
-        List<AliasedNode> selectNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size()
+ 1);        
+        List<AliasedNode> selectNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size()
+ 1);
         selectNodes.add(NODE_FACTORY.aliasedNode(ParseNodeFactory.createTempAlias(), rhsNode));
         selectNodes.addAll(additionalSelectNodes);
-        
+
         if (!isAggregate) {
-            subquery = NODE_FACTORY.select(subquery, subquery.isDistinct(), selectNodes,
where);            
+            subquery = NODE_FACTORY.select(subquery, subquery.isDistinct(), selectNodes,
where);
         } else {
-            List<ParseNode> groupbyNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size()
+ subquery.getGroupBy().size());
+            List<ParseNode> groupbyNodes = Lists
+                    .newArrayListWithExpectedSize(additionalSelectNodes.size() + subquery.getGroupBy().size());
             for (AliasedNode aliasedNode : additionalSelectNodes) {
                 groupbyNodes.add(aliasedNode.getNode());
             }
             groupbyNodes.addAll(subquery.getGroupBy());
             subquery = NODE_FACTORY.select(subquery, subquery.isDistinct(), selectNodes,
where, groupbyNodes, true);
         }
-        
+
         ParseNode onNode = conditionExtractor.getJoinCondition();
         TableNode rhsTable = NODE_FACTORY.derivedTable(rhsTableAlias, subquery);
         JoinType joinType = isTopNode ? JoinType.Inner : JoinType.Left;
-        ParseNode ret = NODE_FACTORY.comparison(node.getFilterOp(), l.get(0), NODE_FACTORY.column(NODE_FACTORY.table(null,
rhsTableAlias), selectNodes.get(0).getAlias(), null));
+        ParseNode ret = NODE_FACTORY.comparison(node.getFilterOp(), l.get(0),
+                NODE_FACTORY.column(NODE_FACTORY.table(null, rhsTableAlias), selectNodes.get(0).getAlias(),
null));
         tableNode = NODE_FACTORY.join(joinType, tableNode, rhsTable, onNode, !isAggregate
|| isGroupby);
-        
+
         return ret;
     }
 
     @Override
     public ParseNode visitLeave(ArrayAnyComparisonNode node, List<ParseNode> l) throws
SQLException {
         List<ParseNode> children = leaveArrayComparisonNode(node, l);
-        if (children == l)
-            return super.visitLeave(node, l);
-        
-        node = NODE_FACTORY.arrayAny(children.get(0), (ComparisonParseNode) children.get(1));
+        if (children == l) return super.visitLeave(node, l);
+
+        node = NODE_FACTORY.arrayAny(children.get(0), (ComparisonParseNode)children.get(1));
         return node;
     }
 
     @Override
     public ParseNode visitLeave(ArrayAllComparisonNode node, List<ParseNode> l) throws
SQLException {
         List<ParseNode> children = leaveArrayComparisonNode(node, l);
-        if (children == l)
-            return super.visitLeave(node, l);
-        
-        node = NODE_FACTORY.arrayAll(children.get(0), (ComparisonParseNode) children.get(1));
+        if (children == l) return super.visitLeave(node, l);
+
+        node = NODE_FACTORY.arrayAll(children.get(0), (ComparisonParseNode)children.get(1));
         return node;
     }
-    
+
     protected List<ParseNode> leaveArrayComparisonNode(ParseNode node, List<ParseNode>
l) throws SQLException {
         boolean isTopNode = topNode == node;
         if (isTopNode) {
@@ -279,20 +267,19 @@ public class SubqueryRewriter extends ParseNodeRewriter {
         }
 
         ParseNode firstChild = l.get(0);
-        if (!(firstChild instanceof SubqueryParseNode)) {
-            return l;
-        }
-        
-        SubqueryParseNode subqueryNode = (SubqueryParseNode) firstChild;
+        if (!(firstChild instanceof SubqueryParseNode)) { return l; }
+
+        SubqueryParseNode subqueryNode = (SubqueryParseNode)firstChild;
         SelectStatement subquery = fixSubqueryStatement(subqueryNode.getSelectNode());
         String rhsTableAlias = ParseNodeFactory.createTempAlias();
-        JoinConditionExtractor conditionExtractor = new JoinConditionExtractor(subquery,
resolver, connection, rhsTableAlias);
+        JoinConditionExtractor conditionExtractor = new JoinConditionExtractor(subquery,
resolver, connection,
+                rhsTableAlias);
         ParseNode where = subquery.getWhere() == null ? null : subquery.getWhere().accept(conditionExtractor);
         if (where == subquery.getWhere()) { // non-correlated any/all comparison subquery
             return l;
         }
-        
-        ParseNode rhsNode = null; 
+
+        ParseNode rhsNode = null;
         boolean isNonGroupByAggregate = subquery.getGroupBy().isEmpty() && subquery.isAggregate();
         List<AliasedNode> aliasedNodes = subquery.getSelect();
         String derivedTableAlias = null;
@@ -300,50 +287,61 @@ public class SubqueryRewriter extends ParseNodeRewriter {
             derivedTableAlias = ParseNodeFactory.createTempAlias();
             aliasedNodes = fixAliasedNodes(aliasedNodes, false);
         }
-        
+
         if (aliasedNodes.size() == 1) {
-            rhsNode = derivedTableAlias == null ? aliasedNodes.get(0).getNode() : NODE_FACTORY.column(NODE_FACTORY.table(null,
derivedTableAlias), aliasedNodes.get(0).getAlias(), null);
+            rhsNode = derivedTableAlias == null ? aliasedNodes.get(0).getNode()
+                    : NODE_FACTORY.column(NODE_FACTORY.table(null, derivedTableAlias), aliasedNodes.get(0).getAlias(),
+                            null);
         } else {
             List<ParseNode> nodes = Lists.<ParseNode> newArrayListWithExpectedSize(aliasedNodes.size());
             for (AliasedNode aliasedNode : aliasedNodes) {
-                nodes.add(derivedTableAlias == null ? aliasedNode.getNode() : NODE_FACTORY.column(NODE_FACTORY.table(null,
derivedTableAlias), aliasedNode.getAlias(), null));
+                nodes.add(derivedTableAlias == null ? aliasedNode.getNode()
+                        : NODE_FACTORY.column(NODE_FACTORY.table(null, derivedTableAlias),
aliasedNode.getAlias(),
+                                null));
             }
             rhsNode = NODE_FACTORY.rowValueConstructor(nodes);
         }
-        
+
         if (!isNonGroupByAggregate) {
             rhsNode = NODE_FACTORY.function(DistinctValueAggregateFunction.NAME, Collections.singletonList(rhsNode));
         }
-        
+
         List<AliasedNode> additionalSelectNodes = conditionExtractor.getAdditionalSelectNodes();
-        List<AliasedNode> selectNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size()
+ 1);        
+        List<AliasedNode> selectNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size()
+ 1);
         selectNodes.add(NODE_FACTORY.aliasedNode(ParseNodeFactory.createTempAlias(), rhsNode));
         selectNodes.addAll(additionalSelectNodes);
         List<ParseNode> groupbyNodes = Lists.newArrayListWithExpectedSize(additionalSelectNodes.size());
         for (AliasedNode aliasedNode : additionalSelectNodes) {
             groupbyNodes.add(aliasedNode.getNode());
         }
-        
+
         if (derivedTableAlias == null) {
             subquery = NODE_FACTORY.select(subquery, false, selectNodes, where, groupbyNodes,
true);
         } else {
-            List<ParseNode> derivedTableGroupBy = Lists.newArrayListWithExpectedSize(subquery.getGroupBy().size()
+ groupbyNodes.size());
+            List<ParseNode> derivedTableGroupBy = Lists
+                    .newArrayListWithExpectedSize(subquery.getGroupBy().size() + groupbyNodes.size());
             derivedTableGroupBy.addAll(groupbyNodes);
             derivedTableGroupBy.addAll(subquery.getGroupBy());
-            List<AliasedNode> derivedTableSelect = Lists.newArrayListWithExpectedSize(aliasedNodes.size()
+ selectNodes.size() - 1);
+            List<AliasedNode> derivedTableSelect = Lists
+                    .newArrayListWithExpectedSize(aliasedNodes.size() + selectNodes.size()
- 1);
             derivedTableSelect.addAll(aliasedNodes);
             for (int i = 1; i < selectNodes.size(); i++) {
                 AliasedNode aliasedNode = selectNodes.get(i);
                 String alias = ParseNodeFactory.createTempAlias();
                 derivedTableSelect.add(NODE_FACTORY.aliasedNode(alias, aliasedNode.getNode()));
-                aliasedNode = NODE_FACTORY.aliasedNode(aliasedNode.getAlias(), NODE_FACTORY.column(NODE_FACTORY.table(null,
derivedTableAlias), alias, null));
+                aliasedNode = NODE_FACTORY.aliasedNode(aliasedNode.getAlias(),
+                        NODE_FACTORY.column(NODE_FACTORY.table(null, derivedTableAlias),
alias, null));
                 selectNodes.set(i, aliasedNode);
                 groupbyNodes.set(i - 1, aliasedNode.getNode());
             }
-            SelectStatement derivedTableStmt = NODE_FACTORY.select(subquery, subquery.isDistinct(),
derivedTableSelect, where, derivedTableGroupBy, true);
-            subquery = NODE_FACTORY.select(NODE_FACTORY.derivedTable(derivedTableAlias, derivedTableStmt),
subquery.getHint(), false, selectNodes, null, groupbyNodes, null, Collections.<OrderByNode>
emptyList(), null, subquery.getBindCount(), true, false, Collections.<SelectStatement>emptyList(),
subquery.getUdfParseNodes());
+            SelectStatement derivedTableStmt = NODE_FACTORY.select(subquery, subquery.isDistinct(),
derivedTableSelect,
+                    where, derivedTableGroupBy, true);
+            subquery = NODE_FACTORY.select(NODE_FACTORY.derivedTable(derivedTableAlias, derivedTableStmt),
+                    subquery.getHint(), false, selectNodes, null, groupbyNodes, null,
+                    Collections.<OrderByNode> emptyList(), null, null, subquery.getBindCount(),
true, false,
+                    Collections.<SelectStatement> emptyList(), subquery.getUdfParseNodes());
         }
-        
+
         ParseNode onNode = conditionExtractor.getJoinCondition();
         TableNode rhsTable = NODE_FACTORY.derivedTable(rhsTableAlias, subquery);
         JoinType joinType = isTopNode ? JoinType.Inner : JoinType.Left;
@@ -351,46 +349,51 @@ public class SubqueryRewriter extends ParseNodeRewriter {
 
         firstChild = NODE_FACTORY.column(NODE_FACTORY.table(null, rhsTableAlias), selectNodes.get(0).getAlias(),
null);
         if (isNonGroupByAggregate) {
-            firstChild = NODE_FACTORY.upsertStmtArrayNode(Collections.singletonList(firstChild));

+            firstChild = NODE_FACTORY.upsertStmtArrayNode(Collections.singletonList(firstChild));
         }
-        ComparisonParseNode secondChild = (ComparisonParseNode) l.get(1);
-        secondChild = NODE_FACTORY.comparison(secondChild.getFilterOp(), secondChild.getLHS(),
NODE_FACTORY.elementRef(Lists.newArrayList(firstChild, NODE_FACTORY.literal(1))));
-        
+        ComparisonParseNode secondChild = (ComparisonParseNode)l.get(1);
+        secondChild = NODE_FACTORY.comparison(secondChild.getFilterOp(), secondChild.getLHS(),
+                NODE_FACTORY.elementRef(Lists.newArrayList(firstChild, NODE_FACTORY.literal(1))));
+
         return Lists.newArrayList(firstChild, secondChild);
     }
-    
+
     private SelectStatement fixSubqueryStatement(SelectStatement select) {
-        if (!select.isUnion())
-            return select;
-        
+        if (!select.isUnion()) return select;
+
         // Wrap as a derived table.
-        return NODE_FACTORY.select(NODE_FACTORY.derivedTable(ParseNodeFactory.createTempAlias(),
select), HintNode.EMPTY_HINT_NODE, false, select.getSelect(), null, null, null, null, null,
select.getBindCount(), false, false, Collections.<SelectStatement> emptyList(), select.getUdfParseNodes());
+        return NODE_FACTORY.select(NODE_FACTORY.derivedTable(ParseNodeFactory.createTempAlias(),
select),
+                HintNode.EMPTY_HINT_NODE, false, select.getSelect(), null, null, null, null,
null, null,
+                select.getBindCount(), false, false, Collections.<SelectStatement>
emptyList(),
+                select.getUdfParseNodes());
     }
-    
+
{code}

> support mysql "limit,offset" clauses 
> -------------------------------------
>
>                 Key: PHOENIX-2722
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-2722
>             Project: Phoenix
>          Issue Type: New Feature
>            Reporter: Ankit Singhal
>            Assignee: Ankit Singhal
>            Priority: Minor
>         Attachments: PHOENIX-2722.patch
>
>
> For serial query(query with “serial" hint or  “limit" without "order by”), we can
limit each scan(using page filter) to “limit+offset” instead of limit earlier.
> And then, for all queries, we can forward the relevant client iterators to the offset
provided and then return the result.
> syntax
> {code}
> [ LIMIT { count } ]
>     [ OFFSET start [ ROW | ROWS ] ]
>     [ FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } ONLY ]
> {code}
> Some new keywords(OFFSET,FETCH,ROW, ROWS,ONLY) are getting introduced so users might
need to see that they are not using them as column name or something.
> WDYT, [~jamestaylor]



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message