fixed #323

RELEASE-NOTES.md

@@ -11,6 +11,7 @@
 1. [ISSUE #313](https://github.com/dangdangdotcom/sharding-jdbc/issues/313) 支持<>操作符
 1. [ISSUE #317](https://github.com/dangdangdotcom/sharding-jdbc/issues/317) 分页Limit参数问题
 1. [ISSUE #320](https://github.com/dangdangdotcom/sharding-jdbc/issues/320) GROUP BY + LIMIT的SQL改写错误
+1. [ISSUE #323](https://github.com/dangdangdotcom/sharding-jdbc/issues/323) 解析ORDER BY + 聚合表达式错误
 
 ## 1.5.0
 

sharding-jdbc-core/src/main/java/com/dangdang/ddframe/rdb/sharding/parsing/parser/SQLParser.java

@@ -88,22 +88,24 @@ public class SQLParser extends AbstractParser {
     // TODO 完善Expression解析的各种场景
     public final SQLExpression parseExpression() {
         String literals = getLexer().getCurrentToken().getLiterals();
+        final int beginPosition = getLexer().getCurrentToken().getEndPosition() - literals.length();
         final SQLExpression expression = getExpression(literals);
         if (skipIfEqual(Literals.IDENTIFIER)) {
             if (skipIfEqual(Symbol.DOT)) {
                 String property = getLexer().getCurrentToken().getLiterals();
                 getLexer().nextToken();
-                return skipIfCompositeExpression() ? new SQLIgnoreExpression() : new SQLPropertyExpression(new SQLIdentifierExpression(literals), property);
+                return skipIfCompositeExpression() ? new SQLIgnoreExpression(getLexer().getInput().substring(beginPosition, getLexer().getCurrentToken().getEndPosition()))
+                        : new SQLPropertyExpression(new SQLIdentifierExpression(literals), property);
             }
             if (equalAny(Symbol.LEFT_PAREN)) {
                 skipParentheses();
                 skipRestCompositeExpression();
-                return new SQLIgnoreExpression();
+                return new SQLIgnoreExpression(getLexer().getInput().substring(beginPosition, getLexer().getCurrentToken().getEndPosition()));
             }
-            return skipIfCompositeExpression() ? new SQLIgnoreExpression() : expression;
+            return skipIfCompositeExpression() ? new SQLIgnoreExpression(getLexer().getInput().substring(beginPosition, getLexer().getCurrentToken().getEndPosition())) : expression;
         }
         getLexer().nextToken();
-        return skipIfCompositeExpression() ? new SQLIgnoreExpression() : expression;
+        return skipIfCompositeExpression() ? new SQLIgnoreExpression(getLexer().getInput().substring(beginPosition, getLexer().getCurrentToken().getEndPosition())) : expression;
     }
     
     private SQLExpression getExpression(final String literals) {
@@ -126,7 +128,7 @@ public class SQLParser extends AbstractParser {
         if (equalAny(Literals.IDENTIFIER)) {
             return new SQLIdentifierExpression(SQLUtil.getExactlyValue(literals));
         }
-        return new SQLIgnoreExpression();
+        return new SQLIgnoreExpression(literals);
     }
     
     private boolean skipIfCompositeExpression() {

sharding-jdbc-core/src/main/java/com/dangdang/ddframe/rdb/sharding/parsing/parser/dialect/mysql/MySQLInsertParser.java

@@ -68,7 +68,7 @@ public final class MySQLInsertParser extends AbstractInsertParser {
             } else if (getSqlParser().equalAny(Literals.CHARS)) {
                 sqlExpression = new SQLTextExpression(getSqlParser().getLexer().getCurrentToken().getLiterals());
             } else if (getSqlParser().equalAny(DefaultKeyword.NULL)) {
-                sqlExpression = new SQLIgnoreExpression();
+                sqlExpression = new SQLIgnoreExpression(DefaultKeyword.NULL.name());
             } else if (getSqlParser().equalAny(Symbol.QUESTION)) {
                 sqlExpression = new SQLPlaceholderExpression(getSqlParser().getParametersIndex());
                 getSqlParser().increaseParametersIndex();

sharding-jdbc-core/src/main/java/com/dangdang/ddframe/rdb/sharding/parsing/parser/dialect/oracle/OracleSelectParser.java

@@ -282,8 +282,8 @@ public class OracleSelectParser extends AbstractSelectParser {
     }
     
     @Override
-    protected Optional<OrderItem> parseSelectOrderByItem() {
-        Optional<OrderItem> result = super.parseSelectOrderByItem();
+    protected OrderItem parseSelectOrderByItem() {
+        OrderItem result = super.parseSelectOrderByItem();
         skipAfterOrderByItem();
         return result;
     }

sharding-jdbc-core/src/main/java/com/dangdang/ddframe/rdb/sharding/parsing/parser/expression/SQLIgnoreExpression.java

@@ -17,10 +17,17 @@
 
 package com.dangdang.ddframe.rdb.sharding.parsing.parser.expression;
 
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
 /**
  * 分片中无需关注的SQL表达式.
  *
  * @author zhangliang
  */
-public class SQLIgnoreExpression implements SQLExpression {
+@RequiredArgsConstructor
+@Getter
+public final class SQLIgnoreExpression implements SQLExpression {
+    
+    private final String expression;
 }

sharding-jdbc-core/src/main/java/com/dangdang/ddframe/rdb/sharding/parsing/parser/statement/dql/select/AbstractSelectParser.java

@@ -31,9 +31,11 @@ import com.dangdang.ddframe.rdb.sharding.parsing.parser.context.selectitem.Aggre
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.context.selectitem.CommonSelectItem;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.context.selectitem.SelectItem;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.context.table.Table;
+import com.dangdang.ddframe.rdb.sharding.parsing.parser.exception.SQLParsingException;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.exception.SQLParsingUnsupportedException;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.expression.SQLExpression;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.expression.SQLIdentifierExpression;
+import com.dangdang.ddframe.rdb.sharding.parsing.parser.expression.SQLIgnoreExpression;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.expression.SQLNumberExpression;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.expression.SQLPropertyExpression;
 import com.dangdang.ddframe.rdb.sharding.parsing.parser.statement.SQLStatementParser;
@@ -221,16 +223,16 @@ public abstract class AbstractSelectParser implements SQLStatementParser {
         sqlParser.skipIfEqual(DefaultKeyword.SIBLINGS);
         sqlParser.accept(DefaultKeyword.BY);
         do {
-            Optional<OrderItem> orderItem = parseSelectOrderByItem();
-            if (orderItem.isPresent() && !isInSubQuery) {
-                result.add(orderItem.get());
+            OrderItem orderItem = parseSelectOrderByItem();
+            if (!isInSubQuery) {
+                result.add(orderItem);
             }
         }
         while (sqlParser.skipIfEqual(Symbol.COMMA));
         selectStatement.getOrderByItems().addAll(result);
     }
     
-    protected Optional<OrderItem> parseSelectOrderByItem() {
+    protected OrderItem parseSelectOrderByItem() {
         SQLExpression sqlExpression = sqlParser.parseExpression(selectStatement);
         OrderType orderByType = OrderType.ASC;
         if (sqlParser.skipIfEqual(DefaultKeyword.ASC)) {
@@ -238,20 +240,20 @@ public abstract class AbstractSelectParser implements SQLStatementParser {
         } else if (sqlParser.skipIfEqual(DefaultKeyword.DESC)) {
             orderByType = OrderType.DESC;
         }
-        OrderItem result;
         if (sqlExpression instanceof SQLNumberExpression) {
-            result = new OrderItem(((SQLNumberExpression) sqlExpression).getNumber().intValue(), orderByType);
+            return new OrderItem(((SQLNumberExpression) sqlExpression).getNumber().intValue(), orderByType);
         } else if (sqlExpression instanceof SQLIdentifierExpression) {
-            result = new OrderItem(
+            return new OrderItem(
                     SQLUtil.getExactlyValue(((SQLIdentifierExpression) sqlExpression).getName()), orderByType, getAlias(SQLUtil.getExactlyValue(((SQLIdentifierExpression) sqlExpression).getName())));
         } else if (sqlExpression instanceof SQLPropertyExpression) {
             SQLPropertyExpression sqlPropertyExpression = (SQLPropertyExpression) sqlExpression;
-            result = new OrderItem(SQLUtil.getExactlyValue(sqlPropertyExpression.getOwner().getName()), SQLUtil.getExactlyValue(sqlPropertyExpression.getName()), orderByType, 
+            return new OrderItem(SQLUtil.getExactlyValue(sqlPropertyExpression.getOwner().getName()), SQLUtil.getExactlyValue(sqlPropertyExpression.getName()), orderByType, 
                     getAlias(SQLUtil.getExactlyValue(sqlPropertyExpression.getOwner().getName()) + "." + SQLUtil.getExactlyValue(sqlPropertyExpression.getName())));
-        } else {
-            return Optional.absent();
+        } else if (sqlExpression instanceof SQLIgnoreExpression) {
+            SQLIgnoreExpression sqlIgnoreExpression = (SQLIgnoreExpression) sqlExpression;
+            return new OrderItem(sqlIgnoreExpression.getExpression(), orderByType, getAlias(sqlIgnoreExpression.getExpression()));
         }
-        return Optional.of(result);
+        throw new SQLParsingException(sqlParser.getLexer());
     }
     
     protected void parseGroupBy() {

sharding-jdbc-core/src/test/resources/parser/select_group_by.xml

@@ -149,19 +149,19 @@
         </order-by-columns>
         <limit row-count-index="0" row-count="5" />
     </assert>
-    <!--<assert id="assertSelectWithGroupByAndOrderByAndLimit" parameters="5">-->
-        <!--<tables>-->
-            <!--<table name="t_order" />-->
-        <!--</tables>-->
-        <!--<aggregation-select-items>-->
-            <!--<aggregation-select-item inner-expression="(order_id)" aggregation-type="SUM"/>-->
-        <!--</aggregation-select-items>-->
-        <!--<group-by-columns>-->
-            <!--<group-by-column name="user_id" order-by-type="ASC" />-->
-        <!--</group-by-columns>-->
-        <!--<order-by-columns>-->
-            <!--<order-by-column name="order_id" order-by-type="ASC" />-->
-        <!--</order-by-columns>-->
-        <!--<limit row-count-index="0" row-count="5" />-->
-    <!--</assert>-->
+    <assert id="assertSelectWithGroupByAndOrderByAndLimit" parameters="5">
+        <tables>
+            <table name="t_order" />
+        </tables>
+        <aggregation-select-items>
+            <aggregation-select-item inner-expression="(order_id)" aggregation-type="SUM"/>
+        </aggregation-select-items>
+        <group-by-columns>
+            <group-by-column name="user_id" order-by-type="ASC" />
+        </group-by-columns>
+        <order-by-columns>
+            <order-by-column name="SUM(order_id)" order-by-type="ASC" />
+        </order-by-columns>
+        <limit row-count-index="0" row-count="5" />
+    </assert>
 </asserts>