You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by tu...@apache.org on 2023/04/17 10:26:04 UTC

[shardingsphere] branch master updated: Revise pr 25189 for code style (#25198)

This is an automated email from the ASF dual-hosted git repository.

tuichenchuxin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git


The following commit(s) were added to refs/heads/master by this push:
     new 81787a002e0 Revise pr 25189 for code style (#25198)
81787a002e0 is described below

commit 81787a002e0676d1c40d1178c4c074092d291b8c
Author: Zhengqiang Duan <du...@apache.org>
AuthorDate: Mon Apr 17 18:25:56 2023 +0800

    Revise pr 25189 for code style (#25198)
---
 .../segment/expression/impl/FunctionConverter.java |  5 ++-
 .../impl/LiteralExpressionConverter.java           | 30 ++++++-------
 ...onConverter.java => TrimFunctionConverter.java} | 25 +++++------
 .../statement/impl/MySQLStatementSQLVisitor.java   |  6 +--
 .../test/it/optimize/SQLNodeConverterEngineIT.java |  8 ++--
 .../resources/case/dml/select-special-function.xml | 51 ++++++++++++++++++++++
 .../sql/supported/dml/select-special-function.xml  |  9 ++--
 7 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java
index 8b95ac8f52d..b49aab8139c 100644
--- a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java
+++ b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java
@@ -40,7 +40,7 @@ import java.util.Optional;
 /**
  * Function converter.
  */
-public final class FunctionConverter implements SQLSegmentConverter<FunctionSegment, SqlNode> {
+public class FunctionConverter implements SQLSegmentConverter<FunctionSegment, SqlNode> {
     
     @Override
     public Optional<SqlNode> convert(final FunctionSegment segment) {
@@ -49,6 +49,9 @@ public final class FunctionConverter implements SQLSegmentConverter<FunctionSegm
         if ("CURRENT_USER".equalsIgnoreCase(functionName.getSimple())) {
             return Optional.of(functionName);
         }
+        if ("TRIM".equalsIgnoreCase(functionName.getSimple())) {
+            return new TrimFunctionConverter().convert(segment);
+        }
         List<SqlOperator> functions = new LinkedList<>();
         SqlStdOperatorTable.instance().lookupOperatorOverloads(functionName, null, SqlSyntax.FUNCTION, functions, SqlNameMatchers.withCaseSensitive(false));
         return Optional.of(functions.isEmpty()
diff --git a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/LiteralExpressionConverter.java b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/LiteralExpressionConverter.java
index 624adaaa0c9..83feed36a80 100644
--- a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/LiteralExpressionConverter.java
+++ b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/LiteralExpressionConverter.java
@@ -19,11 +19,13 @@ package org.apache.shardingsphere.sqlfederation.optimizer.converter.segment.expr
 
 import org.apache.calcite.sql.SqlLiteral;
 import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.fun.SqlTrimFunction.Flag;
 import org.apache.calcite.sql.parser.SqlParserPos;
-import org.apache.calcite.sql.fun.SqlTrimFunction;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.simple.LiteralExpressionSegment;
 import org.apache.shardingsphere.sqlfederation.optimizer.converter.segment.SQLSegmentConverter;
 
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Optional;
 
 /**
@@ -31,6 +33,14 @@ import java.util.Optional;
  */
 public final class LiteralExpressionConverter implements SQLSegmentConverter<LiteralExpressionSegment, SqlNode> {
     
+    private static final Collection<String> TRIM_FUNCTION_FLAGS = new HashSet<>(3, 1);
+    
+    static {
+        TRIM_FUNCTION_FLAGS.add("BOTH");
+        TRIM_FUNCTION_FLAGS.add("LEADING");
+        TRIM_FUNCTION_FLAGS.add("TRAILING");
+    }
+    
     @Override
     public Optional<SqlNode> convert(final LiteralExpressionSegment segment) {
         if (null == segment.getLiterals()) {
@@ -39,22 +49,8 @@ public final class LiteralExpressionConverter implements SQLSegmentConverter<Lit
         if (segment.getLiterals() instanceof Integer) {
             return Optional.of(SqlLiteral.createExactNumeric(String.valueOf(segment.getLiterals()), SqlParserPos.ZERO));
         }
-        if (segment.getLiterals().equals("BOTH") || segment.getLiterals().equals("LEADING") || segment.getLiterals().equals("TRAILING")) {
-            SqlTrimFunction.Flag flag;
-            switch (segment.getLiterals().toString()) {
-                case "BOTH":
-                    flag = SqlTrimFunction.Flag.BOTH;
-                    break;
-                case "LEADING":
-                    flag = SqlTrimFunction.Flag.LEADING;
-                    break;
-                case "TRAILING":
-                    flag = SqlTrimFunction.Flag.TRAILING;
-                    break;
-                default:
-                    throw new IllegalArgumentException("Invalid literal for flag: " + segment.getLiterals());
-            }
-            return Optional.of(SqlLiteral.createSymbol(flag, SqlParserPos.ZERO));
+        if (TRIM_FUNCTION_FLAGS.contains(String.valueOf(segment.getLiterals()))) {
+            return Optional.of(SqlLiteral.createSymbol(Flag.valueOf(String.valueOf(segment.getLiterals())), SqlParserPos.ZERO));
         }
         if (segment.getLiterals() instanceof String) {
             return Optional.of(SqlLiteral.createCharString(String.valueOf(segment.getLiterals()), SqlParserPos.ZERO));
diff --git a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/TrimFunctionConverter.java
similarity index 72%
copy from kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java
copy to kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/TrimFunctionConverter.java
index 8b95ac8f52d..3867d29db9a 100644
--- a/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/FunctionConverter.java
+++ b/kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/TrimFunctionConverter.java
@@ -18,18 +18,17 @@
 package org.apache.shardingsphere.sqlfederation.optimizer.converter.segment.expression.impl;
 
 import org.apache.calcite.sql.SqlBasicCall;
-import org.apache.calcite.sql.SqlFunctionCategory;
 import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlLiteral;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlSyntax;
-import org.apache.calcite.sql.SqlUnresolvedFunction;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.fun.SqlTrimFunction.Flag;
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.validate.SqlNameMatchers;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.ExpressionSegment;
 import org.apache.shardingsphere.sql.parser.sql.common.segment.dml.expr.FunctionSegment;
-import org.apache.shardingsphere.sqlfederation.optimizer.converter.segment.SQLSegmentConverter;
 import org.apache.shardingsphere.sqlfederation.optimizer.converter.segment.expression.ExpressionConverter;
 
 import java.util.Collection;
@@ -38,27 +37,27 @@ import java.util.List;
 import java.util.Optional;
 
 /**
- * Function converter.
+ * Trim function converter.
  */
-public final class FunctionConverter implements SQLSegmentConverter<FunctionSegment, SqlNode> {
+public final class TrimFunctionConverter extends FunctionConverter {
     
     @Override
     public Optional<SqlNode> convert(final FunctionSegment segment) {
         SqlIdentifier functionName = new SqlIdentifier(segment.getFunctionName(), SqlParserPos.ZERO);
-        // TODO optimize sql parse logic for select current_user.
-        if ("CURRENT_USER".equalsIgnoreCase(functionName.getSimple())) {
-            return Optional.of(functionName);
-        }
         List<SqlOperator> functions = new LinkedList<>();
         SqlStdOperatorTable.instance().lookupOperatorOverloads(functionName, null, SqlSyntax.FUNCTION, functions, SqlNameMatchers.withCaseSensitive(false));
-        return Optional.of(functions.isEmpty()
-                ? new SqlBasicCall(
-                        new SqlUnresolvedFunction(functionName, null, null, null, null, SqlFunctionCategory.USER_DEFINED_FUNCTION), getFunctionParameters(segment.getParameters()), SqlParserPos.ZERO)
-                : new SqlBasicCall(functions.iterator().next(), getFunctionParameters(segment.getParameters()), SqlParserPos.ZERO));
+        return Optional.of(new SqlBasicCall(functions.iterator().next(), getFunctionParameters(segment.getParameters()), SqlParserPos.ZERO));
     }
     
     private List<SqlNode> getFunctionParameters(final Collection<ExpressionSegment> sqlSegments) {
         List<SqlNode> result = new LinkedList<>();
+        if (1 == sqlSegments.size()) {
+            result.add(Flag.BOTH.symbol(SqlParserPos.ZERO));
+            result.add(SqlLiteral.createCharString(" ", SqlParserPos.ZERO));
+        }
+        if (2 == sqlSegments.size()) {
+            result.add(Flag.BOTH.symbol(SqlParserPos.ZERO));
+        }
         for (ExpressionSegment each : sqlSegments) {
             new ExpressionConverter().convert(each).ifPresent(result::add);
         }
diff --git a/sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java b/sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java
index f71f31196aa..95e399d57f2 100644
--- a/sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java
+++ b/sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java
@@ -961,10 +961,9 @@ public abstract class MySQLStatementSQLVisitor extends MySQLStatementBaseVisitor
         }
         return result;
     }
-
+    
     @Override
     public final ASTNode visitTrimFunction(final TrimFunctionContext ctx) {
-        calculateParameterCount(ctx.expr());
         FunctionSegment result = new FunctionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.TRIM().getText(), getOriginalText(ctx));
         if (null != ctx.BOTH()) {
             result.getParameters().add(new LiteralExpressionSegment(ctx.BOTH().getSymbol().getStartIndex(), ctx.BOTH().getSymbol().getStopIndex(),
@@ -979,8 +978,7 @@ public abstract class MySQLStatementSQLVisitor extends MySQLStatementBaseVisitor
                     new OtherLiteralValue(ctx.LEADING().getSymbol().getText()).getValue()));
         }
         for (ExprContext each : ctx.expr()) {
-            ASTNode expr = visit(each);
-            result.getParameters().add((ExpressionSegment) expr);
+            result.getParameters().add((ExpressionSegment) visit(each));
         }
         return result;
     }
diff --git a/test/it/optimizer/src/test/java/org/apache/shardingsphere/test/it/optimize/SQLNodeConverterEngineIT.java b/test/it/optimizer/src/test/java/org/apache/shardingsphere/test/it/optimize/SQLNodeConverterEngineIT.java
index 09ec95ab03b..fc29e59cbcf 100644
--- a/test/it/optimizer/src/test/java/org/apache/shardingsphere/test/it/optimize/SQLNodeConverterEngineIT.java
+++ b/test/it/optimizer/src/test/java/org/apache/shardingsphere/test/it/optimize/SQLNodeConverterEngineIT.java
@@ -52,7 +52,6 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.Properties;
-import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -103,8 +102,8 @@ class SQLNodeConverterEngineIT {
         
         // TODO remove the method when all SQL statement support convert to SQL node
         // CHECKSTYLE:OFF
-        private Set<String> getSupportedSQLCaseIDs() {
-            Set<String> result = new HashSet<>();
+        private Collection<String> getSupportedSQLCaseIDs() {
+            Collection<String> result = new HashSet<>();
             result.add("select_with_join_table_subquery");
             result.add("select_with_projection_subquery");
             result.add("select_with_in_subquery_condition");
@@ -194,8 +193,11 @@ class SQLNodeConverterEngineIT {
             result.add("select_char");
             result.add("select_weight_string");
             result.add("select_trim");
+            result.add("select_trim_with_both");
             result.add("select_with_trim_expr");
+            result.add("select_with_trim_expr_and_both");
             result.add("select_with_trim_expr_from_expr");
+            result.add("select_with_trim_expr_from_expr_and_both");
             return result;
         }
         // CHECKSTYLE:ON
diff --git a/test/it/parser/src/main/resources/case/dml/select-special-function.xml b/test/it/parser/src/main/resources/case/dml/select-special-function.xml
index 7a1a1f06477..53ab83f1d11 100644
--- a/test/it/parser/src/main/resources/case/dml/select-special-function.xml
+++ b/test/it/parser/src/main/resources/case/dml/select-special-function.xml
@@ -150,6 +150,19 @@
         </projections>
     </select>
     <select sql-case-id="select_trim">
+        <projections start-index="7" stop-index="22">
+            <expression-projection text="TRIM('  bar   ')" start-index="7" stop-index="22">
+                <expr>
+                    <function function-name="TRIM" start-index="7" stop-index="22" text="TRIM('  bar   ')">
+                        <parameter>
+                            <literal-expression value="  bar   " start-index="12" stop-index="21" />
+                        </parameter>
+                    </function>
+                </expr>
+            </expression-projection>
+        </projections>
+    </select>
+    <select sql-case-id="select_trim_with_both">
         <projections start-index="7" stop-index="33">
             <expression-projection text="TRIM(BOTH ' ' from ' bar ')" start-index="7" stop-index="33">
                 <expr>
@@ -169,6 +182,25 @@
         </projections>
     </select>
     <select sql-case-id="select_with_trim_expr">
+        <projections start-index="7" stop-index="27">
+            <expression-projection text="TRIM('#' FROM `name`)" start-index="7" stop-index="27">
+                <expr>
+                    <function function-name="TRIM" start-index="7" stop-index="27" text="TRIM('#' FROM `name`)" >
+                        <parameter>
+                            <literal-expression value="#" start-index="12" stop-index="14" />
+                        </parameter>
+                        <parameter>
+                            <column name="name" start-delimiter="`" end-delimiter="`" start-index="21" stop-index="26" />
+                        </parameter>
+                    </function>
+                </expr>
+            </expression-projection>
+        </projections>
+        <from>
+            <simple-table name="t_order" start-index="34" stop-index="40" />
+        </from>
+    </select>
+    <select sql-case-id="select_with_trim_expr_and_both">
         <projections start-index="7" stop-index="32">
             <expression-projection text="TRIM(BOTH '#' FROM `name`)" start-index="7" stop-index="32">
                 <expr>
@@ -191,6 +223,25 @@
         </from>
     </select>
     <select sql-case-id="select_with_trim_expr_from_expr">
+        <projections start-index="7" stop-index="33">
+            <expression-projection text="TRIM(remove_name FROM name)" start-index="7" stop-index="33">
+                <expr>
+                    <function function-name="TRIM" start-index="7" stop-index="33" text="TRIM(remove_name FROM name)" >
+                        <parameter>
+                            <column name="remove_name" start-index="12" stop-index="22" />
+                        </parameter>
+                        <parameter>
+                            <column name="name" start-index="29" stop-index="32" />
+                        </parameter>
+                    </function>
+                </expr>
+            </expression-projection>
+        </projections>
+        <from>
+            <simple-table name="t_order" start-index="40" stop-index="46" />
+        </from>
+    </select>
+    <select sql-case-id="select_with_trim_expr_from_expr_and_both">
         <projections start-index="7" stop-index="42">
             <expression-projection text="TRIM(BOTH `remove_name` FROM `name`)" start-index="7" stop-index="42">
                 <expr>
diff --git a/test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml b/test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml
index b01cbc076ed..99f8b1e8581 100644
--- a/test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml
+++ b/test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml
@@ -26,9 +26,12 @@
     <sql-case id="select_substring" value="SELECT SUBSTRING('foobarbar' from 4)" db-types="MySQL" />
     <sql-case id="select_extract" value="SELECT EXTRACT(YEAR FROM '2019-07-02')" db-types="MySQL" />
     <sql-case id="select_char" value="SELECT CHAR(77,121,83,81,'76')" db-types="MySQL" />
-    <sql-case id="select_trim" value="SELECT TRIM(BOTH ' ' from ' bar ')" db-types="MySQL" />
-    <sql-case id="select_with_trim_expr" value="SELECT TRIM(BOTH '#' FROM `name`) FROM `t_order`" db-types="MySQL" />
-    <sql-case id="select_with_trim_expr_from_expr" value="SELECT TRIM(BOTH `remove_name` FROM `name`) FROM `t_order`" db-types="MySQL" />
+    <sql-case id="select_trim" value="SELECT TRIM('  bar   ')" db-types="MySQL" />
+    <sql-case id="select_trim_with_both" value="SELECT TRIM(BOTH ' ' from ' bar ')" db-types="MySQL" />
+    <sql-case id="select_with_trim_expr" value="SELECT TRIM('#' FROM `name`) FROM t_order" db-types="MySQL" />
+    <sql-case id="select_with_trim_expr_and_both" value="SELECT TRIM(BOTH '#' FROM `name`) FROM `t_order`" db-types="MySQL" />
+    <sql-case id="select_with_trim_expr_from_expr" value="SELECT TRIM(remove_name FROM name) FROM t_order" db-types="MySQL" />
+    <sql-case id="select_with_trim_expr_from_expr_and_both" value="SELECT TRIM(BOTH `remove_name` FROM `name`) FROM `t_order`" db-types="MySQL" />
     <sql-case id="select_weight_string" value="SELECT WEIGHT_STRING('bar')" db-types="MySQL" />
     <sql-case id="select_values" value="SELECT VALUES(order_id) FROM t_order" db-types="MySQL" />
     <sql-case id="select_current_user_brackets" value="SELECT CURRENT_USER()" db-types="MySQL" />