You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/02/10 12:09:30 UTC

[GitHub] [incubator-shardingsphere] jingshanglu opened a new pull request #4228: add visitor for setVariable of mysql

jingshanglu opened a new pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228
 
 
   Fixes #3914 .
   
   Changes proposed in this pull request:
   - add visitor for setVariable
   - add visitor for showOther
   - add test for showColumus, setVariable,showCreateTable
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377102058
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/impl/SetVariableStatementAssert.java
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.integrate.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal.SetVariableStatementTestCase;
+import org.apache.shardingsphere.sql.parser.sql.statement.dal.dialect.mysql.SetStatement;
+
+/**
+ * set variable statement assert.
+ *
+ * @author lujingshang
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SetVariableStatementAssert {
+    
+    /**
+     * Assert describe statement is correct with expected parser result.
+     * 
+     * @param assertContext assert context
+     * @param actual actual describe statement
+     * @param expected expected describe statement test case
 
 Review comment:
   Incorrect java doc for `describe statement`, should be `set statement`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377101138
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/DALStatementAssert.java
 ##########
 @@ -83,6 +86,8 @@ public static void assertIs(final SQLCaseAssertContext assertContext, final DALS
             ShowTableStatusStatementAssert.assertIs(assertContext, (ShowTableStatusStatement) actual, (ShowTableStatusStatementTestCase) expected);
         } else if (actual instanceof ShowIndexStatement) {
             ShowIndexStatementAssert.assertIs(assertContext, (ShowIndexStatement) actual, (ShowIndexStatementTestCase) expected);
+        } else if (actual instanceof SetStatement) {
 
 Review comment:
   `SetStatement` is a new type with `ShowXXXStatement`, it should not between `ShowIndexStatement` and `ShowStatement`, please move `SetStatement` behind `ShowStatement`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377106499
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/resources/sql/dal/show.xml
 ##########
 @@ -50,6 +50,15 @@
             <owner name="sharding_db" start-delimiter="`" end-delimiter="`" start-index="15" stop-index="27" />
         </table>
     </show-index>
+
 
 Review comment:
   Please only keep single blank line.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377105798
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
 ##########
 @@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set.ExpectedVariableExpr;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * set variable statement test case.
+ * 
+ * @author lujingshang
+ */
+@Getter
+@Setter
+public final class SetVariableStatementTestCase extends SQLParserTestCase {
+
+    @XmlElement(name = "variable-expr")
+    private final List<ExpectedVariableExpr> variableExprs = new LinkedList<>();
+
+
 
 Review comment:
   Please remove unnecessary blank lines.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu merged pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584103954
 
 
   ## Pull Request Test Coverage Report for [Build 9429](https://coveralls.io/builds/28630060)
   
   * **0** of **15**   **(0.0%)**  changed or added relevant lines in **3** files are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage decreased (**-0.04%**) to **64.429%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/SetStatement.java](https://coveralls.io/builds/28630060/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdal%2Fdialect%2Fmysql%2FSetStatement.java#L22) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/FromTableSegment.java](https://coveralls.io/builds/28630060/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FFromTableSegment.java#L39) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/MySQLDALVisitor.java](https://coveralls.io/builds/28630060/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2FMySQLDALVisitor.java#L130) | 0 | 13 | 0.0%
   <!-- | **Total:** | **0** | **15** | **0.0%** | -->
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/28630060/badge)](https://coveralls.io/builds/28630060) |
   | :-- | --: |
   | Change from base [Build 9428](https://coveralls.io/builds/28629529): |  -0.04% |
   | Covered Lines: | 10960 |
   | Relevant Lines: | 17011 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377105583
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
 ##########
 @@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set.ExpectedVariableExpr;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * set variable statement test case.
+ * 
+ * @author lujingshang
+ */
+@Getter
+@Setter
+public final class SetVariableStatementTestCase extends SQLParserTestCase {
+
+    @XmlElement(name = "variable-expr")
 
 Review comment:
   Please do not use abbreviation, please rename `variable-expr` to `variable-expression`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377099355
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableExprSegment.java
 ##########
 @@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+@RequiredArgsConstructor
+@Getter
+public class VariableExprSegment implements SQLSegment {
+
+    private final int startIndex;
+
+    private final int stopIndex;
+
+    private final String variable;
+
+    private final String expr;
+
 
 Review comment:
   Unnecessary blank lines 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377101955
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/impl/SetVariableStatementAssert.java
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.integrate.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal.SetVariableStatementTestCase;
+import org.apache.shardingsphere.sql.parser.sql.statement.dal.dialect.mysql.SetStatement;
+
+/**
+ * set variable statement assert.
+ *
+ * @author lujingshang
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SetVariableStatementAssert {
+    
+    /**
+     * Assert describe statement is correct with expected parser result.
+     * 
+     * @param assertContext assert context
+     * @param actual actual describe statement
 
 Review comment:
   Incorrect java doc for `describe statement`, should be `set statement`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584103954
 
 
   ## Pull Request Test Coverage Report for [Build 9435](https://coveralls.io/builds/28647977)
   
   * **0** of **15**   **(0.0%)**  changed or added relevant lines in **3** files are covered.
   * **302** unchanged lines in **29** files lost coverage.
   * Overall coverage increased (+**0.2%**) to **64.657%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/SetStatement.java](https://coveralls.io/builds/28647977/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdal%2Fdialect%2Fmysql%2FSetStatement.java#L27) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/FromTableSegment.java](https://coveralls.io/builds/28647977/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FFromTableSegment.java#L39) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/MySQLDALVisitor.java](https://coveralls.io/builds/28647977/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2FMySQLDALVisitor.java#L130) | 0 | 13 | 0.0%
   <!-- | **Total:** | **0** | **15** | **0.0%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/AuthenticationChangedListener.java](https://coveralls.io/builds/28647977/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FAuthenticationChangedListener.java#L36) | 1 | 75.0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/PropertiesChangedListener.java](https://coveralls.io/builds/28647977/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FPropertiesChangedListener.java#L34) | 1 | 75.0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/SchemaChangedListener.java](https://coveralls.io/builds/28647977/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FSchemaChangedListener.java#L113) | 1 | 97.5% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/state/listener/DataSourceStateChangedListener.java](https://coveralls.io/builds/28647977/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fstate%2Flistener%2FDataSourceStateChangedListener.java#L36) | 1 | 85.71% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/state/listener/InstanceStateChangedListener.java](https://coveralls.io/builds/28647977/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fstate%2Flistener%2FInstanceStateChangedListener.java#L35) | 1 | 75.0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dml/column/ColumnSegment.java](https://coveralls.io/builds/28647977/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdml%2Fcolumn%2FColumnSegment.java#L65) | 1 | 50.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringEncryptDataSource.java](https://coveralls.io/builds/28647977/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringEncryptDataSource.java#L37) | 1 | 50.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringMasterSlaveDataSource.java](https://coveralls.io/builds/28647977/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringMasterSlaveDataSource.java#L38) | 1 | 50.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringShardingDataSource.java](https://coveralls.io/builds/28647977/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringShardingDataSource.java#L38) | 1 | 50.0% |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/extractor/impl/common/schema/SchemaExtractor.java](https://coveralls.io/builds/28647977/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fcore%2Fextractor%2Fimpl%2Fcommon%2Fschema%2FSchemaExtractor.java#L41) | 2 | 66.67% |
   <!-- | **Total:** | **302** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/28647977/badge)](https://coveralls.io/builds/28647977) |
   | :-- | --: |
   | Change from base [Build 9428](https://coveralls.io/builds/28629529): |  0.2% |
   | Covered Lines: | 11015 |
   | Relevant Lines: | 17036 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377099079
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableExprSegment.java
 ##########
 @@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+@RequiredArgsConstructor
+@Getter
+public class VariableExprSegment implements SQLSegment {
 
 Review comment:
   1. Missing java doc
   2. Can the class be final?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377104832
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/segment/impl/set/ExpectedVariableExpr.java
 ##########
 @@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set;
+
+import lombok.Getter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlElement;
+
+/**
+ * Expected variable expr.
 
 Review comment:
   Please do not use abbreviation, please rename `expr` to `expression`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377104042
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/SQLParserTestCases.java
 ##########
 @@ -203,6 +205,9 @@
 
     @XmlElement(name = "show-index")
     private final List<ShowIndexStatementTestCase> showIndexTestCases = new LinkedList<>();
+
+    @XmlElement(name = "set-variable")
+    private final List<SetVariableStatementTestCase> setVariableStatementTestCases = new LinkedList<>();
 
 Review comment:
   Please do not interrupt `showXXXTestCases`, please move `setVariableStatementTestCases` behind `showTestCases`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377098631
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/SetStatement.java
 ##########
 @@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.sql.statement.dal.dialect.mysql;
+
+import org.apache.shardingsphere.sql.parser.sql.statement.dal.DALStatement;
+
+public final class SetStatement extends DALStatement {
 
 Review comment:
   Missing java doc.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero edited a comment on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584154881
 
 
   @terrymanu Hi, i guess you are reviewing this PR, so is it necessary for me to give it a look?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584466148
 
 
   > > The comment `Fixes #3914` will close the #3914, but the issue is not finished yet.
   > 
   > @terrymanu What should be written？
   
   Maybe `For #3914` or just `#3914`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] codecov-io edited a comment on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584101450
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@ec7dde4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4228   +/-   ##
   =========================================
     Coverage          ?   60.76%           
     Complexity        ?      352           
   =========================================
     Files             ?     1013           
     Lines             ?    17036           
     Branches          ?     3005           
   =========================================
     Hits              ?    10352           
     Misses            ?     6031           
     Partials          ?      653
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=footer). Last update [ec7dde4...9586ecb](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377105096
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/segment/impl/set/ExpectedVariableExpr.java
 ##########
 @@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set;
+
+import lombok.Getter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlElement;
+
+/**
+ * Expected variable expr.
+ * 
+ * @author lujingshang
+ */
+@Getter
+public final class ExpectedVariableExpr extends AbstractExpectedSQLSegment {
+    
+    @XmlElement(name = "variable")
+    private String variable;
+
+    @XmlElement(name = "expr")
+    private String expr;
 
 Review comment:
   Please do not use abbreviation, please rename `expr` to `expression`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] codecov-io commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584101450
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=h1) Report
   > :exclamation: No coverage uploaded for pull request base (`master@ec7dde4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #4228   +/-   ##
   =========================================
     Coverage          ?   60.53%           
     Complexity        ?      352           
   =========================================
     Files             ?     1016           
     Lines             ?    17011           
     Branches          ?     3002           
   =========================================
     Hits              ?    10298           
     Misses            ?     6061           
     Partials          ?      652
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=footer). Last update [ec7dde4...1690f20](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4228?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] tristaZero commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584154881
 
 
   @terrymanu Hi, i guess you are reviewing this PR, so do i need to give it a look?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377100027
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableExprSegment.java
 ##########
 @@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+@RequiredArgsConstructor
+@Getter
+public class VariableExprSegment implements SQLSegment {
 
 Review comment:
   We prefer do not use abbreviation.
   Please rename `expr` to `expression` 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584481054
 
 
   Please pay attention for #4233

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377105269
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
 ##########
 @@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set.ExpectedVariableExpr;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * set variable statement test case.
 
 Review comment:
   First letter of java doc should be upper case.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] jingshanglu commented on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584448817
 
 
   > The comment `Fixes #3914` will close the #3914, but the issue is not finished yet.
   
   @terrymanu What should be written？

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377103958
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/SQLParserTestCases.java
 ##########
 @@ -258,6 +263,7 @@
         putAll(showCreateTableTestCases, result);
         putAll(showTableStatusTestCases, result);
         putAll(showIndexTestCases, result);
+        putAll(setVariableStatementTestCases, result);
 
 Review comment:
   Please do not interrupt `showXXXTestCases`, please move `setVariableStatementTestCases` behind `showTestCases`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377101348
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/impl/SetVariableStatementAssert.java
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.integrate.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal.SetVariableStatementTestCase;
+import org.apache.shardingsphere.sql.parser.sql.statement.dal.dialect.mysql.SetStatement;
+
+/**
+ * set variable statement assert.
 
 Review comment:
   First letter of java doc should be upper case.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377104688
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/segment/impl/set/ExpectedVariableExpr.java
 ##########
 @@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.set;
+
+import lombok.Getter;
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlElement;
+
+/**
+ * Expected variable expr.
+ * 
+ * @author lujingshang
+ */
+@Getter
+public final class ExpectedVariableExpr extends AbstractExpectedSQLSegment {
 
 Review comment:
   Please do not use abbreviation, please rename `ExpectedVariableExpr` to `ExpectedVariableExpression`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377103089
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/SQLParserTestCases.java
 ##########
 @@ -203,6 +205,9 @@
 
     @XmlElement(name = "show-index")
     private final List<ShowIndexStatementTestCase> showIndexTestCases = new LinkedList<>();
+
+    @XmlElement(name = "set-variable")
+    private final List<SetVariableStatementTestCase> setVariableStatementTestCases = new LinkedList<>();
 
 Review comment:
   The variable name `setVariableStatementTestCases` should keep consist with others, it is better to  rename as `setVariableTestCases`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377100102
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableExprSegment.java
 ##########
 @@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.shardingsphere.sql.parser.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+@RequiredArgsConstructor
+@Getter
+public class VariableExprSegment implements SQLSegment {
+
+    private final int startIndex;
+
+    private final int stopIndex;
+
+    private final String variable;
+
+    private final String expr;
 
 Review comment:
   We prefer do not use abbreviation.
   Please rename `expr` to `expression` 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] coveralls edited a comment on issue #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#issuecomment-584103954
 
 
   ## Pull Request Test Coverage Report for [Build 1673](https://coveralls.io/builds/28633553)
   
   * **0** of **15**   **(0.0%)**  changed or added relevant lines in **3** files are covered.
   * **94** unchanged lines in **18** files lost coverage.
   * Overall coverage increased (+**0.3%**) to **64.746%**
   
   ---
   
   |  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
   | :-----|--------------|--------|---: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/SetStatement.java](https://coveralls.io/builds/28633553/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdal%2Fdialect%2Fmysql%2FSetStatement.java#L22) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/FromTableSegment.java](https://coveralls.io/builds/28633553/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FFromTableSegment.java#L39) | 0 | 1 | 0.0%
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/MySQLDALVisitor.java](https://coveralls.io/builds/28633553/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2FMySQLDALVisitor.java#L130) | 0 | 13 | 0.0%
   <!-- | **Total:** | **0** | **15** | **0.0%** | -->
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/AuthenticationChangedListener.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FAuthenticationChangedListener.java#L36) | 1 | 75.0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/PropertiesChangedListener.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FPropertiesChangedListener.java#L34) | 1 | 75.0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/config/listener/SchemaChangedListener.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fconfig%2Flistener%2FSchemaChangedListener.java#L113) | 1 | 97.5% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/state/listener/DataSourceStateChangedListener.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fstate%2Flistener%2FDataSourceStateChangedListener.java#L36) | 1 | 85.71% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/registry/state/listener/InstanceStateChangedListener.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Fregistry%2Fstate%2Flistener%2FInstanceStateChangedListener.java#L35) | 1 | 75.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringEncryptDataSource.java](https://coveralls.io/builds/28633553/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringEncryptDataSource.java#L37) | 1 | 50.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringMasterSlaveDataSource.java](https://coveralls.io/builds/28633553/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringMasterSlaveDataSource.java#L38) | 1 | 50.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-namespace/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/datasource/OrchestrationSpringShardingDataSource.java](https://coveralls.io/builds/28633553/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-namespace%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fdatasource%2FOrchestrationSpringShardingDataSource.java#L38) | 1 | 50.0% |
   | [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/28633553/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L74) | 2 | 80.0% |
   | [sharding-spring/sharding-jdbc-orchestration-spring/sharding-jdbc-orchestration-spring-boot-starter/src/main/java/org/apache/shardingsphere/shardingjdbc/orchestration/spring/boot/OrchestrationSpringBootConfiguration.java](https://coveralls.io/builds/28633553/source?filename=sharding-spring%2Fsharding-jdbc-orchestration-spring%2Fsharding-jdbc-orchestration-spring-boot-starter%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingjdbc%2Forchestration%2Fspring%2Fboot%2FOrchestrationSpringBootConfiguration.java#L193) | 2 | 95.45% |
   <!-- | **Total:** | **94** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/28633553/badge)](https://coveralls.io/builds/28633553) |
   | :-- | --: |
   | Change from base [Build 9428](https://coveralls.io/builds/28629529): |  0.3% |
   | Covered Lines: | 11034 |
   | Relevant Lines: | 17042 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-shardingsphere] terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql

Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4228: add visitor for setVariable of mysql
URL: https://github.com/apache/incubator-shardingsphere/pull/4228#discussion_r377106212
 
 

 ##########
 File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/ShowColumnsStatementTestCase.java
 ##########
 @@ -17,12 +17,20 @@
 
 package org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.dal;
 
+import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.segment.impl.table.ExpectedTable;
 import org.apache.shardingsphere.sql.parser.integrate.jaxb.domain.statement.SQLParserTestCase;
 
+import javax.xml.bind.annotation.XmlElement;
+
 /**
  * Show columns statement test case.
  * 
  * @author zhangliang 
  */
 public final class ShowColumnsStatementTestCase extends SQLParserTestCase {
+
+    @XmlElement
+    private ExpectedTable table;
+
+
 
 Review comment:
   Please remove unnecessary blank lines.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services