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/25 19:54:16 UTC
[GitHub] [incubator-shardingsphere] beijing-penguin opened a new pull
request #4477: Parse mysql variable value for SET statement
beijing-penguin opened a new pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477
Fixes #4453.
Changes proposed in this pull request:
- DALStatement.g4 update
- visitor logic code write
- modify test case set.xml
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386195263
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
##########
@@ -30,6 +30,9 @@
@Setter
public final class SetVariableStatementTestCase extends SQLParserTestCase {
- @XmlAttribute(name = "variable")
- private String variable;
+ @XmlAttribute(name = "variableName")
Review comment:
Please reference other name of xml attribute, we prefer using `-` to delimiter tow words.
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386090557
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,7 +84,11 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET (variable? (TO | EQ_) variableValue COMMA_?)*
+ ;
+
+variableValue
+ : (NUMBER_ | identifier | STRING_ | DEFAULT)
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384354126
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,7 +84,11 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET (variable? (TO | EQ_) variableValue COMMA_?)*
+ ;
+
+variableValue
+ : (NUMBER_ | identifier | STRING_ | DEFAULT)
Review comment:
What is the sequence of `NUMBER_ | identifier | STRING_ | DEFAULT`?
How about adjust sequence to `identifier | NUMBER_ | STRING_ | DEFAULT`?
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193604
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
+@AllArgsConstructor
+public class VariableExpr {
+
+ private VariableSegment variable;
+
+ private VariableValueSegment variableValue;
+
+ private String scopeType;
+
Review comment:
Please remove useless 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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386195277
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
##########
@@ -30,6 +30,9 @@
@Setter
public final class SetVariableStatementTestCase extends SQLParserTestCase {
- @XmlAttribute(name = "variable")
- private String variable;
+ @XmlAttribute(name = "variableName")
+ private String variableName;
+
+ @XmlAttribute(name = "variableValue")
Review comment:
Please reference other name of xml attribute, we prefer using `-` to delimiter tow words.
----------------------------------------------------------------
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] nancyzrh removed a comment on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
nancyzrh removed a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-593061871
/run tests
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386310999
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
+ VariableExprContext variableExprContext = ctx.variableExpr();
+ VariableSegment variableSegment = null;
+ VariableValueSegment variableValueSegment = null;
+ String scopeType = variableExprContext.scope() == null ? null : variableExprContext.scope().getText();
Review comment:
`scopeType` is inconsistent with rule name `scope`
----------------------------------------------------------------
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 #4477:
Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-599358880
@beijing-penguin It is difficult to merge now, can I close this issue if no more update?
----------------------------------------------------------------
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] beijing-penguin commented on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-599360715
yes
----------------------------------------------------------------
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 #4477:
Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
tristaZero commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-592930431
/ci
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088253
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
Review comment:
re-design
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386273265
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/TimeZoneSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+/**
+ * Variable segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class TimeZoneSegment implements SQLSegment {
+
+ private final int startIndex;
+
+ private final int stopIndex;
+
+ private final String variable;
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386194460
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,9 +84,17 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET variableExpr (COMMA_ variableExpr)*
;
+variableExpr
+ : variable EQ_ variableValue
Review comment:
Please consider about the relationship of rule name of `variable` and `variableValue`
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386309830
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/antlr4/imports/postgresql/BaseRule.g4
##########
@@ -328,3 +328,23 @@ ignoredIdentifier_
ignoredIdentifiers_
: ignoredIdentifier_ (COMMA_ ignoredIdentifier_)*
;
+
+variableExpr
+ : ((scope? variable (TO | EQ_) variableValue) | (scope? timeZone variableValue))
Review comment:
There are unnecessary brackets
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384352494
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
Review comment:
Please avoid using ArrayList without init size
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384357428
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
Review comment:
Is it correct for setVariableValue to variableValueSegmentList.get(0)?
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193740
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/SetStatement.java
##########
@@ -28,5 +31,6 @@
@Setter
public final class SetStatement extends DALStatement {
- private VariableSegment variable;
+ private List<VariableExpr> variableExprList;
+
Review comment:
Please remove useless 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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386307550
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -144,15 +151,31 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
@Override
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+
Review comment:
Please remove useless 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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384357941
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
##########
@@ -32,4 +32,7 @@
@XmlAttribute(name = "variable")
private String variable;
+
+ @XmlAttribute(name = "variableValue")
+ private String variableValue;
Review comment:
Please consider about the name `variable` and `variableValue`
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088460
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * VariableValue segment.
+ * @author duanchao
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193567
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
+@AllArgsConstructor
+public class VariableExpr {
+
+ private VariableSegment variable;
+
+ private VariableValueSegment variableValue;
Review comment:
What is the relationship of `variable` and `variableValue`? Are they for same level or contain relation?
----------------------------------------------------------------
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] beijing-penguin closed pull request
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin closed pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386305026
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * Variable value segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class VariableValueSegment implements ValueASTNode<String> {
+
+ private final String value;
+
+ private final QuoteCharacter quoteCharacter;
Review comment:
The quote character is used for identifier only, `identifier` and `literal` are different rule type.
So we can not use quote character for literal. `String` is a type of literal.
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386308299
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * Variable value segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class VariableValueSegment implements ValueASTNode<String> {
Review comment:
If the name of `VariableValueSegment` contains `Segment`, why this class do not implement for `SQLSegment`?
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384356717
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
+
Review comment:
Please remove unnecessary 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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193437
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
+@AllArgsConstructor
+public class VariableExpr {
Review comment:
Missing final here
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088504
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/SetStatement.java
##########
@@ -29,4 +31,6 @@
public final class SetStatement extends DALStatement {
private VariableSegment variable;
Review comment:
re-code re-design
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386268186
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
##########
@@ -30,6 +30,9 @@
@Setter
public final class SetVariableStatementTestCase extends SQLParserTestCase {
- @XmlAttribute(name = "variable")
- private String variable;
+ @XmlAttribute(name = "variableName")
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388385338
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/antlr4/imports/postgresql/BaseRule.g4
##########
@@ -328,3 +328,23 @@ ignoredIdentifier_
ignoredIdentifiers_
: ignoredIdentifier_ (COMMA_ ignoredIdentifier_)*
;
+
+variableExpr
+ : ((scope? variable (TO | EQ_) variableValue) | (scope? timeZone variableValue))
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384357405
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
+
+ List<VariableValueContext> variableValueContextList = ctx.variableValue();
+ List<VariableValueSegment> variableValueSegmentList = new ArrayList<VariableValueSegment>();
+ for (VariableValueContext variableValueContext : variableValueContextList) {
+ variableValueSegmentList.add((VariableValueSegment) visit(variableValueContext));
+ }
+ result.setVariableValue(variableValueSegmentList.get(0));
Review comment:
Is it correct for setVariableValue to variableValueSegmentList.get(0)?
----------------------------------------------------------------
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 #4477:
Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9802](https://coveralls.io/builds/28956991)
* **0** of **19** **(0.0%)** changed or added relevant lines in **2** files are covered.
* **3** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.06%**) to **58.963%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/28956991/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L39) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/28956991/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L153) | 0 | 15 | 0.0%
<!-- | **Total:** | **0** | **19** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/28956991/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
<!-- | **Total:** | **3** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/28956991/badge)](https://coveralls.io/builds/28956991) |
| :-- | --: |
| Change from base [Build 933](https://coveralls.io/builds/28944931): | -0.06% |
| Covered Lines: | 10614 |
| Relevant Lines: | 18001 |
---
##### 💛 - [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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384356068
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,7 +84,11 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET (variable? (TO | EQ_) variableValue COMMA_?)*
Review comment:
The rule is `SET variable = expr [, variable = expr] ...`
This rule is not accurate, for example, if only one variable and variableValue, the COMMA_ should never exist.
Where is the definition of keyword `TO` ?
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386273958
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,9 +84,17 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET variableExpr (COMMA_ variableExpr)*
;
+variableExpr
+ : variable EQ_ variableValue
Review comment:
variable is abstract object,include some property
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386090417
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
+
+ List<VariableValueContext> variableValueContextList = ctx.variableValue();
+ List<VariableValueSegment> variableValueSegmentList = new ArrayList<VariableValueSegment>();
+ for (VariableValueContext variableValueContext : variableValueContextList) {
+ variableValueSegmentList.add((VariableValueSegment) visit(variableValueContext));
+ }
+ result.setVariableValue(variableValueSegmentList.get(0));
Review comment:
re-design
----------------------------------------------------------------
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] nancyzrh commented on issue #4477: Parse
mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
nancyzrh commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-593061871
/run tests
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193203
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
+@AllArgsConstructor
+public class VariableExpr {
Review comment:
Missing javadoc
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386310749
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
Review comment:
Use `Collections.singletionList(xxx)` is better
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193132
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * Variable value segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class VariableValueSegment implements ValueASTNode<String> {
+
+ private final String value;
+
+ private final QuoteCharacter quoteCharacter;
Review comment:
The variable value should not contain quoteCharacter, is it?
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388387527
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Variable expr.
+ */
+@Setter
+@Getter
+@AllArgsConstructor
+public class VariableExpr {
+
+ private VariableSegment variable;
+
+ private VariableValueSegment variableValue;
+
+ private String scopeType;
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388374788
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/impl/SetVariableStatementAssert.java
##########
@@ -42,11 +42,12 @@
* @param expected expected variable statement test case
*/
public static void assertIs(final SQLCaseAssertContext assertContext, final SetStatement actual, final SetVariableStatementTestCase expected) {
- if (null != expected.getVariable()) {
- assertNotNull(assertContext.getText("Actual variable expression should exist."), actual.getVariable());
- assertThat(assertContext.getText("variable expression assertion error: "), actual.getVariable().getVariable(), is(expected.getVariable()));
+ if (null != expected.getVariableName()) {
+ assertNotNull(assertContext.getText("Actual variable expression should exist."), actual.getVariableExprList());
+ assertThat(assertContext.getText("variable expression assertion error: "), actual.getVariableExprList().get(0).getVariable().getVariableName(), is(expected.getVariableName()));
Review comment:
yes,i know ,but this time code have commit too much file,I want to optimize multivariable test on next time....there code will can check a single variable and a single variable value..
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386306844
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Variable expr.
+ */
+@Setter
+@Getter
+@AllArgsConstructor
+public class VariableExpr {
Review comment:
`VariableExpr` maybe a bad name, we prefer do not use abbreviation for class name.
`VariableExpression` is a better name.
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386194675
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
Review comment:
If the size of ArrayList is 1, why we need create a list?
How about return the element directly?
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388371978
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
+ VariableExprContext variableExprContext = ctx.variableExpr();
+ VariableSegment variableSegment = null;
+ VariableValueSegment variableValueSegment = null;
+ String scopeType = variableExprContext.scope() == null ? null : variableExprContext.scope().getText();
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088464
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * VariableValue segment.
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386267808
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/jaxb/domain/statement/dal/SetVariableStatementTestCase.java
##########
@@ -30,6 +30,9 @@
@Setter
public final class SetVariableStatementTestCase extends SQLParserTestCase {
- @XmlAttribute(name = "variable")
- private String variable;
+ @XmlAttribute(name = "variableName")
+ private String variableName;
+
+ @XmlAttribute(name = "variableValue")
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9956](https://coveralls.io/builds/29066694)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **678** unchanged lines in **47** files lost coverage.
* Overall coverage decreased (**-0.8%**) to **58.311%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L155) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 18 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dal/EncryptDALResultDecorator.java](https://coveralls.io/builds/29066694/source?filename=encrypt-core%2Fencrypt-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fmerge%2Fdal%2FEncryptDALResultDecorator.java#L50) | 1 | 0% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java](https://coveralls.io/builds/29066694/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java#L58) | 1 | 95.83% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertValueParameterRewriter.java](https://coveralls.io/builds/29066694/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertValueParameterRewriter.java#L77) | 1 | 94.87% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptAssignmentTokenGenerator.java](https://coveralls.io/builds/29066694/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptAssignmentTokenGenerator.java#L85) | 1 | 98.31% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java](https://coveralls.io/builds/29066694/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptProjectionTokenGenerator.java#L58) | 1 | 94.74% |
| [sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/sharding/rewrite/token/generator/impl/keygen/GeneratedKeyInsertValuesTokenGenerator.java](https://coveralls.io/builds/29066694/source?filename=sharding-core%2Fsharding-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2Fkeygen%2FGeneratedKeyInsertValuesTokenGenerator.java#L73) | 1 | 95.0% |
| [sharding-core/sharding-core-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/ShardingRouteEngineFactory.java](https://coveralls.io/builds/29066694/source?filename=sharding-core%2Fsharding-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Froute%2Fengine%2Ftype%2FShardingRouteEngineFactory.java#L101) | 1 | 97.37% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/DescribeStatement.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdal%2Fdialect%2Fmysql%2FDescribeStatement.java#L30) | 1 | 0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/dialect/mysql/ShowCreateTableStatement.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdal%2Fdialect%2Fmysql%2FShowCreateTableStatement.java#L30) | 1 | 0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dcl/DenyUserStatement.java](https://coveralls.io/builds/29066694/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdcl%2FDenyUserStatement.java#L29) | 1 | 0% |
<!-- | **Total:** | **678** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29066694/badge)](https://coveralls.io/builds/29066694) |
| :-- | --: |
| Change from base [Build 951](https://coveralls.io/builds/29023442): | -0.8% |
| Covered Lines: | 10528 |
| Relevant Lines: | 18055 |
---
##### 💛 - [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] coveralls edited a comment on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9903](https://coveralls.io/builds/29053958)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **264** unchanged lines in **24** files lost coverage.
* Overall coverage decreased (**-1.04%**) to **58.097%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29053958/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29053958/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L155) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29053958/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 18 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dal/EncryptDALResultDecorator.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fmerge%2Fdal%2FEncryptDALResultDecorator.java#L50) | 1 | 0% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java#L58) | 1 | 95.65% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertValueParameterRewriter.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertValueParameterRewriter.java#L77) | 1 | 94.87% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptAssignmentTokenGenerator.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptAssignmentTokenGenerator.java#L84) | 1 | 98.31% |
| [sharding-core/sharding-core-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/ShardingRouteEngineFactory.java](https://coveralls.io/builds/29053958/source?filename=sharding-core%2Fsharding-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Froute%2Fengine%2Ftype%2FShardingRouteEngineFactory.java#L101) | 1 | 97.37% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptInsertOnUpdateTokenGenerator.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptInsertOnUpdateTokenGenerator.java#L56) | 2 | 95.56% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptProjectionTokenGenerator.java#L53) | 2 | 90.0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-relation/src/main/java/org/apache/shardingsphere/sql/parser/relation/segment/table/TablesContext.java](https://coveralls.io/builds/29053958/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-relation%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Frelation%2Fsegment%2Ftable%2FTablesContext.java#L63) | 2 | 96.15% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptForUseDefaultInsertColumnsTokenGenerator.java](https://coveralls.io/builds/29053958/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptForUseDefaultInsertColumnsTokenGenerator.java#L55) | 3 | 80.43% |
| [shadow-core/shadow-core-rewrite/src/main/java/org/apache/shardingsphere/shadow/rewrite/judgement/impl/SimpleJudgementEngine.java](https://coveralls.io/builds/29053958/source?filename=shadow-core%2Fshadow-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshadow%2Frewrite%2Fjudgement%2Fimpl%2FSimpleJudgementEngine.java#L58) | 3 | 81.82% |
<!-- | **Total:** | **264** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29053958/badge)](https://coveralls.io/builds/29053958) |
| :-- | --: |
| Change from base [Build 951](https://coveralls.io/builds/29023442): | -1.04% |
| Covered Lines: | 10544 |
| Relevant Lines: | 18149 |
---
##### 💛 - [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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386273165
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9962](https://coveralls.io/builds/29068566)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.09%**) to **58.3%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29068566/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29068566/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L155) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29068566/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 18 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29068566/badge)](https://coveralls.io/builds/29068566) |
| :-- | --: |
| Change from base [Build 9958](https://coveralls.io/builds/29067117): | -0.09% |
| Covered Lines: | 10522 |
| Relevant Lines: | 18048 |
---
##### 💛 - [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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388379395
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Variable expr.
+ */
+@Setter
+@Getter
+@AllArgsConstructor
+public class VariableExpr {
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388376111
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
+ VariableExprContext variableExprContext = ctx.variableExpr();
+ VariableSegment variableSegment = null;
+ VariableValueSegment variableValueSegment = null;
+ String scopeType = variableExprContext.scope() == null ? null : variableExprContext.scope().getText();
Review comment:
use scope .done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384358167
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/resources/sql/dal/set.xml
##########
@@ -17,7 +17,7 @@
-->
<sql-parser-test-cases>
- <set-variable sql-case-id="set_parameter_equal" variable="configuration_parameter" />
+ <set-variable sql-case-id="set_parameter_equal" variable="configuration_parameter" variableValue="value"/>
Review comment:
Please add a space before `/>`
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386273113
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * Variable value segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class VariableValueSegment implements ValueASTNode<String> {
+
+ private final String value;
+
+ private final QuoteCharacter quoteCharacter;
Review comment:
Is needed for judge int or String
----------------------------------------------------------------
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 #4477:
Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591076568
# [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=h1) Report
> Merging [#4477](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/dcd6fe75cd332254938796007ee2cac61198e53c?src=pr&el=desc) will **decrease** coverage by `0.05%`.
> The diff coverage is `0%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4477 +/- ##
============================================
- Coverage 55.36% 55.31% -0.06%
Complexity 330 330
============================================
Files 952 953 +1
Lines 17984 18001 +17
Branches 3384 3386 +2
============================================
Hits 9957 9957
- Misses 7382 7399 +17
Partials 645 645
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ere/sql/parser/sql/statement/dal/SetStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RhbC9TZXRTdGF0ZW1lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...phere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW15c3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9NeVNRTERBTFZpc2l0b3IuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...l/parser/sql/segment/dal/VariableValueSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kYWwvVmFyaWFibGVWYWx1ZVNlZ21lbnQuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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/4477?src=pr&el=footer). Last update [dcd6fe7...134006c](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384350820
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * VariableValue segment.
+ * @author duanchao
Review comment:
Please remove author
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9900](https://coveralls.io/builds/29053798)
* **0** of **37** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **264** unchanged lines in **24** files lost coverage.
* Overall coverage decreased (**-1.05%**) to **58.09%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29053798/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29053798/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L155) | 0 | 14 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29053798/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 19 | 0.0%
<!-- | **Total:** | **0** | **37** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dal/EncryptDALResultDecorator.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fmerge%2Fdal%2FEncryptDALResultDecorator.java#L50) | 1 | 0% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java#L58) | 1 | 95.65% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertValueParameterRewriter.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertValueParameterRewriter.java#L77) | 1 | 94.87% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptAssignmentTokenGenerator.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptAssignmentTokenGenerator.java#L84) | 1 | 98.31% |
| [sharding-core/sharding-core-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/ShardingRouteEngineFactory.java](https://coveralls.io/builds/29053798/source?filename=sharding-core%2Fsharding-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Froute%2Fengine%2Ftype%2FShardingRouteEngineFactory.java#L101) | 1 | 97.37% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptInsertOnUpdateTokenGenerator.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptInsertOnUpdateTokenGenerator.java#L56) | 2 | 95.56% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptProjectionTokenGenerator.java#L53) | 2 | 90.0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-relation/src/main/java/org/apache/shardingsphere/sql/parser/relation/segment/table/TablesContext.java](https://coveralls.io/builds/29053798/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-relation%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Frelation%2Fsegment%2Ftable%2FTablesContext.java#L63) | 2 | 96.15% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptForUseDefaultInsertColumnsTokenGenerator.java](https://coveralls.io/builds/29053798/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptForUseDefaultInsertColumnsTokenGenerator.java#L55) | 3 | 80.43% |
| [shadow-core/shadow-core-rewrite/src/main/java/org/apache/shardingsphere/shadow/rewrite/judgement/impl/SimpleJudgementEngine.java](https://coveralls.io/builds/29053798/source?filename=shadow-core%2Fshadow-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshadow%2Frewrite%2Fjudgement%2Fimpl%2FSimpleJudgementEngine.java#L58) | 3 | 81.82% |
<!-- | **Total:** | **264** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29053798/badge)](https://coveralls.io/builds/29053798) |
| :-- | --: |
| Change from base [Build 951](https://coveralls.io/builds/29023442): | -1.05% |
| Covered Lines: | 10544 |
| Relevant Lines: | 18151 |
---
##### 💛 - [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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386090554
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
+
+ List<VariableValueContext> variableValueContextList = ctx.variableValue();
+ List<VariableValueSegment> variableValueSegmentList = new ArrayList<VariableValueSegment>();
+ for (VariableValueContext variableValueContext : variableValueContextList) {
+ variableValueSegmentList.add((VariableValueSegment) visit(variableValueContext));
+ }
+ result.setVariableValue(variableValueSegmentList.get(0));
}
return result;
}
+ @Override
+ public ASTNode visitVariableValue(final VariableValueContext ctx) {
+ VariableValueSegment result = new VariableValueSegment(ctx.getText());
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 2110](https://coveralls.io/builds/29375455)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.08%**) to **59.047%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29375455/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L42) | 0 | 6 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29375455/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L294) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29375455/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L52) | 0 | 16 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29375455/badge)](https://coveralls.io/builds/29375455) |
| :-- | --: |
| Change from base [Build 998](https://coveralls.io/builds/29154645): | -0.08% |
| Covered Lines: | 12117 |
| Relevant Lines: | 20521 |
---
##### 💛 - [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] codecov-io edited a comment on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591076568
# [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=h1) Report
> Merging [#4477](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-shardingsphere/commit/0ce016e13fd5ba42b5f6beba595d68e955f8dc1e?src=pr&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `0%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4477 +/- ##
============================================
- Coverage 55.52% 55.44% -0.08%
Complexity 421 421
============================================
Files 1130 1131 +1
Lines 20492 20521 +29
Branches 3792 3796 +4
============================================
Hits 11378 11378
- Misses 8396 8425 +29
Partials 718 718
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ere/sql/parser/sql/statement/dal/SetStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RhbC9TZXRTdGF0ZW1lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [.../sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXBvc3RncmVzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NxbC9wYXJzZXIvdmlzaXRvci9pbXBsL1Bvc3RncmVTUUxEQUxWaXNpdG9yLmphdmE=) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...phere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLW15c3FsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3Zpc2l0b3IvaW1wbC9NeVNRTERBTFZpc2l0b3IuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...l/parser/sql/segment/dal/VariableValueSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kYWwvVmFyaWFibGVWYWx1ZVNlZ21lbnQuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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/4477?src=pr&el=footer). Last update [0ce016e...e594341](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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] coveralls edited a comment on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 23](https://coveralls.io/builds/29053204)
* **0** of **41** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **261** unchanged lines in **23** files lost coverage.
* Overall coverage decreased (**-1.05%**) to **58.089%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29053204/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29053204/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L154) | 0 | 17 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29053204/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 20 | 0.0%
<!-- | **Total:** | **0** | **41** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [encrypt-core/encrypt-core-merge/src/main/java/org/apache/shardingsphere/encrypt/merge/dal/EncryptDALResultDecorator.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-merge%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Fmerge%2Fdal%2FEncryptDALResultDecorator.java#L50) | 1 | 0% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertOnDuplicateKeyUpdateValueParameterRewriter.java#L58) | 1 | 95.65% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/impl/EncryptInsertValueParameterRewriter.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Fparameter%2Fimpl%2FEncryptInsertValueParameterRewriter.java#L77) | 1 | 94.87% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptAssignmentTokenGenerator.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptAssignmentTokenGenerator.java#L84) | 1 | 98.31% |
| [sharding-core/sharding-core-route/src/main/java/org/apache/shardingsphere/sharding/route/engine/type/ShardingRouteEngineFactory.java](https://coveralls.io/builds/29053204/source?filename=sharding-core%2Fsharding-core-route%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsharding%2Froute%2Fengine%2Ftype%2FShardingRouteEngineFactory.java#L101) | 1 | 97.37% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptInsertOnUpdateTokenGenerator.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptInsertOnUpdateTokenGenerator.java#L56) | 2 | 95.56% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptProjectionTokenGenerator.java#L53) | 2 | 90.0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-relation/src/main/java/org/apache/shardingsphere/sql/parser/relation/segment/table/TablesContext.java](https://coveralls.io/builds/29053204/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-relation%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Frelation%2Fsegment%2Ftable%2FTablesContext.java#L63) | 2 | 96.15% |
| [encrypt-core/encrypt-core-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptForUseDefaultInsertColumnsTokenGenerator.java](https://coveralls.io/builds/29053204/source?filename=encrypt-core%2Fencrypt-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fencrypt%2Frewrite%2Ftoken%2Fgenerator%2Fimpl%2FEncryptForUseDefaultInsertColumnsTokenGenerator.java#L55) | 3 | 80.43% |
| [shadow-core/shadow-core-rewrite/src/main/java/org/apache/shardingsphere/shadow/rewrite/judgement/impl/SimpleJudgementEngine.java](https://coveralls.io/builds/29053204/source?filename=shadow-core%2Fshadow-core-rewrite%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshadow%2Frewrite%2Fjudgement%2Fimpl%2FSimpleJudgementEngine.java#L58) | 3 | 81.82% |
<!-- | **Total:** | **261** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29053204/badge)](https://coveralls.io/builds/29053204) |
| :-- | --: |
| Change from base [Build 951](https://coveralls.io/builds/29023442): | -1.05% |
| Covered Lines: | 10546 |
| Relevant Lines: | 18155 |
---
##### 💛 - [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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386194344
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/BaseRule.g4
##########
@@ -110,9 +110,13 @@ unreservedWord
;
variable
- : (AT_? AT_)? (GLOBAL | PERSIST | PERSIST_ONLY | SESSION)? DOT_? identifier
+ : (AT_? AT_)? scopeKeyword? DOT_? identifier
Review comment:
Please consider about how to name to rule, other rules never contain like appendix of `keyWord`
----------------------------------------------------------------
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] beijing-penguin commented on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-592921816
/ci
----------------------------------------------------------------
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] tuohai666 commented on issue #4477:
Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-593365029
/run ci
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384353327
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dal/SetStatement.java
##########
@@ -29,4 +31,6 @@
public final class SetStatement extends DALStatement {
private VariableSegment variable;
Review comment:
Can you consider about rename VariableSegment? We can not understand the relationship with VariableSegment and VariableValueSegment
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388383074
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * Variable value segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class VariableValueSegment implements ValueASTNode<String> {
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388385774
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/DALStatement.g4
##########
@@ -84,7 +84,11 @@ showProfileType
;
setVariable
- : SET variable?
+ : SET (variable? (TO | EQ_) variableValue COMMA_?)*
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088467
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386271357
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/antlr4/imports/mysql/BaseRule.g4
##########
@@ -110,9 +110,13 @@ unreservedWord
;
variable
- : (AT_? AT_)? (GLOBAL | PERSIST | PERSIST_ONLY | SESSION)? DOT_? identifier
+ : (AT_? AT_)? scopeKeyword? DOT_? identifier
Review comment:
done
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386274896
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
Review comment:
For future support pgsql some variable's condition and pgsql and mysql unity in code
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388384337
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/antlr4/imports/postgresql/BaseRule.g4
##########
@@ -328,3 +328,23 @@ ignoredIdentifier_
ignoredIdentifiers_
: ignoredIdentifier_ (COMMA_ ignoredIdentifier_)*
;
+
+variableExpr
+ : ((scope? variable (TO | EQ_) variableValue) | (scope? timeZone variableValue))
+ ;
+
+variableValue
+ : (identifier | NUMBER_ | (MINUS_ NUMBER_) | STRING_ | DEFAULT | LOCAL)
+ ;
+
+timeZone
+ : TIME ZONE
+ ;
+
+variable
+ : identifier
+ ;
+
+scope
+ : (SESSION | LOCAL)
+ ;
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 49](https://coveralls.io/builds/29071555)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **2** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.08%**) to **58.311%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29071555/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29071555/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L155) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29071555/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 18 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29071555/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L72) | 2 | 80.0% |
<!-- | **Total:** | **2** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29071555/badge)](https://coveralls.io/builds/29071555) |
| :-- | --: |
| Change from base [Build 9958](https://coveralls.io/builds/29067117): | -0.08% |
| Covered Lines: | 10524 |
| Relevant Lines: | 18048 |
---
##### 💛 - [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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386305872
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Variable expr.
+ */
+@Setter
+@Getter
+@AllArgsConstructor
+public class VariableExpr {
+
+ private VariableSegment variable;
+
+ private VariableValueSegment variableValue;
+
+ private String scopeType;
Review comment:
It may better if using `scope` to instead of `scopeType`. Make consist with rule of g4 is better.
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386309301
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -144,15 +151,31 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
@Override
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+
+ List<VariableExprContext> variableExprContextList = ctx.variableExpr();
+ if (null != variableExprContextList) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(variableExprContextList.size());
+ for (int i = 0; i < variableExprContextList.size(); i++) {
+ VariableExprContext variableExprContext = variableExprContextList.get(i);
+ VariableSegment variableSegment = (VariableSegment) visitVariable(variableExprContext.variable());
+ VariableValueSegment variableValueSegment = (VariableValueSegment) visitVariableValue(variableExprContext.variableValue());
+ String scopeType = variableExprContext.variable().scope() == null ? null : variableExprContext.variable().scope().getText();
Review comment:
`scopeType` is inconsistent with rule name `scope`
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386310205
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/antlr4/imports/postgresql/BaseRule.g4
##########
@@ -328,3 +328,23 @@ ignoredIdentifier_
ignoredIdentifiers_
: ignoredIdentifier_ (COMMA_ ignoredIdentifier_)*
;
+
+variableExpr
+ : ((scope? variable (TO | EQ_) variableValue) | (scope? timeZone variableValue))
+ ;
+
+variableValue
+ : (identifier | NUMBER_ | (MINUS_ NUMBER_) | STRING_ | DEFAULT | LOCAL)
+ ;
+
+timeZone
+ : TIME ZONE
+ ;
+
+variable
+ : identifier
+ ;
+
+scope
+ : (SESSION | LOCAL)
+ ;
Review comment:
Please keep one empty line on the end of file
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388384728
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -144,15 +151,31 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
@Override
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+
+ List<VariableExprContext> variableExprContextList = ctx.variableExpr();
+ if (null != variableExprContextList) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(variableExprContextList.size());
+ for (int i = 0; i < variableExprContextList.size(); i++) {
+ VariableExprContext variableExprContext = variableExprContextList.get(i);
+ VariableSegment variableSegment = (VariableSegment) visitVariable(variableExprContext.variable());
+ VariableValueSegment variableValueSegment = (VariableValueSegment) visitVariableValue(variableExprContext.variableValue());
+ String scopeType = variableExprContext.variable().scope() == null ? null : variableExprContext.variable().scope().getText();
Review comment:
done.use scope
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388380023
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -144,15 +151,31 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
@Override
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193676
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
+@AllArgsConstructor
Review comment:
Please use @RequiredArgsConstructor
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386192831
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/TimeZoneSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+/**
+ * Variable segment.
Review comment:
The class is `TimeZoneSegment`, but the java doc is `Variable segment`
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591076568
# [Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@4da49ce`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `0.47%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/graphs/tree.svg?width=650&token=ZvlXpWa7so&height=150&src=pr)](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4477 +/- ##
=========================================
Coverage ? 55.06%
Complexity ? 331
=========================================
Files ? 955
Lines ? 17995
Branches ? 3402
=========================================
Hits ? 9909
Misses ? 7453
Partials ? 633
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...t/ddl/column/alter/AddColumnDefinitionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kZGwvY29sdW1uL2FsdGVyL0FkZENvbHVtbkRlZmluaXRpb25TZWdtZW50LmphdmE=) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [...dl/column/alter/ModifyColumnDefinitionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kZGwvY29sdW1uL2FsdGVyL01vZGlmeUNvbHVtbkRlZmluaXRpb25TZWdtZW50LmphdmE=) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [.../sql/parser/sql/statement/dml/DeleteStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RtbC9EZWxldGVTdGF0ZW1lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [...ngjdbc/jdbc/adapter/AbstractConnectionAdapter.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmctamRiYy9zaGFyZGluZy1qZGJjLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5namRiYy9qZGJjL2FkYXB0ZXIvQWJzdHJhY3RDb25uZWN0aW9uQWRhcHRlci5qYXZh) | `73.52% <ø> (ø)` | `6 <0> (?)` | |
| [...l/segment/dml/item/ShorthandProjectionSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9kbWwvaXRlbS9TaG9ydGhhbmRQcm9qZWN0aW9uU2VnbWVudC5qYXZh) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [...ere/sql/parser/sql/statement/dal/SetStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RhbC9TZXRTdGF0ZW1lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [...l/parser/relation/segment/table/TablesContext.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXJlbGF0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcWwvcGFyc2VyL3JlbGF0aW9uL3NlZ21lbnQvdGFibGUvVGFibGVzQ29udGV4dC5qYXZh) | `85.91% <ø> (ø)` | `0 <0> (?)` | |
| [...rite/token/generator/impl/TableTokenGenerator.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLXJld3JpdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYXJkaW5nL3Jld3JpdGUvdG9rZW4vZ2VuZXJhdG9yL2ltcGwvVGFibGVUb2tlbkdlbmVyYXRvci5qYXZh) | `80% <ø> (ø)` | `1 <0> (?)` | |
| [.../sql/parser/sql/statement/dml/UpdateStatement.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc3RhdGVtZW50L2RtbC9VcGRhdGVTdGF0ZW1lbnQuamF2YQ==) | `0% <ø> (ø)` | `0 <0> (?)` | |
| [...e/sql/parser/sql/segment/generic/TableSegment.java](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvc2VnbWVudC9nZW5lcmljL1RhYmxlU2VnbWVudC5qYXZh) | `33.33% <ø> (ø)` | `0 <0> (?)` | |
| ... and [38 more](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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/4477?src=pr&el=footer). Last update [4da49ce...9a4342b](https://codecov.io/gh/apache/incubator-shardingsphere/pull/4477?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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384351185
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.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.sql.segment.dal;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+import org.apache.shardingsphere.sql.parser.core.constant.QuoteCharacter;
+import org.apache.shardingsphere.sql.parser.sql.value.ValueASTNode;
+import org.apache.shardingsphere.sql.parser.util.SQLUtil;
+
+/**
+ * VariableValue segment.
Review comment:
`VariableValue` are 2 words
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386312039
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/java/org/apache/shardingsphere/sql/parser/integrate/asserts/statement/dal/impl/SetVariableStatementAssert.java
##########
@@ -42,11 +42,12 @@
* @param expected expected variable statement test case
*/
public static void assertIs(final SQLCaseAssertContext assertContext, final SetStatement actual, final SetVariableStatementTestCase expected) {
- if (null != expected.getVariable()) {
- assertNotNull(assertContext.getText("Actual variable expression should exist."), actual.getVariable());
- assertThat(assertContext.getText("variable expression assertion error: "), actual.getVariable().getVariable(), is(expected.getVariable()));
+ if (null != expected.getVariableName()) {
+ assertNotNull(assertContext.getText("Actual variable expression should exist."), actual.getVariableExprList());
+ assertThat(assertContext.getText("variable expression assertion error: "), actual.getVariableExprList().get(0).getVariable().getVariableName(), is(expected.getVariableName()));
Review comment:
Why judge the first variable here?
FIY: `actual.getVariableExprList().get(0)`
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193405
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/VariableExpr.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableSegment;
+import org.apache.shardingsphere.sql.parser.sql.segment.dal.VariableValueSegment;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+
+@Data
Review comment:
Why need @Data here, It seems like other similar object do not have the annotation.
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r388378712
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 9885](https://coveralls.io/builds/29045467)
* **0** of **41** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **83** unchanged lines in **6** files lost coverage.
* Overall coverage decreased (**-0.5%**) to **58.644%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L38) | 0 | 4 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L154) | 0 | 17 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L54) | 0 | 20 | 0.0%
<!-- | **Total:** | **0** | **41** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-relation/src/main/java/org/apache/shardingsphere/sql/parser/relation/segment/table/TablesContext.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-relation%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Frelation%2Fsegment%2Ftable%2FTablesContext.java#L75) | 2 | 95.77% |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29045467/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-relation/src/main/java/org/apache/shardingsphere/sql/parser/relation/segment/select/projection/engine/ProjectionsContextEngine.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-relation%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Frelation%2Fsegment%2Fselect%2Fprojection%2Fengine%2FProjectionsContextEngine.java#L184) | 6 | 78.89% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/DeleteStatement.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdml%2FDeleteStatement.java#L39) | 14 | 0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/UpdateStatement.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdml%2FUpdateStatement.java#L40) | 14 | 0% |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/statement/dml/SelectStatement.java](https://coveralls.io/builds/29045467/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fstatement%2Fdml%2FSelectStatement.java#L74) | 44 | 4.35% |
<!-- | **Total:** | **83** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29045467/badge)](https://coveralls.io/builds/29045467) |
| :-- | --: |
| Change from base [Build 951](https://coveralls.io/builds/29023442): | -0.5% |
| Covered Lines: | 10553 |
| Relevant Lines: | 17995 |
---
##### 💛 - [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] beijing-penguin commented on issue
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-592921794
/run ci
----------------------------------------------------------------
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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386088101
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-test/src/test/resources/sql/dal/set.xml
##########
@@ -17,7 +17,7 @@
-->
<sql-parser-test-cases>
- <set-variable sql-case-id="set_parameter_equal" variable="configuration_parameter" />
+ <set-variable sql-case-id="set_parameter_equal" variable="configuration_parameter" variableValue="value"/>
Review comment:
done
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386311311
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java
##########
@@ -33,26 +42,50 @@
* DAL visitor for PostgreSQL.
*/
public final class PostgreSQLDALVisitor extends PostgreSQLVisitor implements DALVisitor {
-
+
@Override
public ASTNode visitShow(final ShowContext ctx) {
return new ShowStatement();
}
-
+
@Override
- public ASTNode visitSet(final SetContext ctx) {
+ public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
- if (null != ctx.configurationParameterClause()) {
- result.setVariable((VariableSegment) visit(ctx.configurationParameterClause()));
+ if (null != ctx.variableExpr()) {
+ List<VariableExpr> variableExprList = new ArrayList<VariableExpr>(1);
+ VariableExprContext variableExprContext = ctx.variableExpr();
+ VariableSegment variableSegment = null;
+ VariableValueSegment variableValueSegment = null;
+ String scopeType = variableExprContext.scope() == null ? null : variableExprContext.scope().getText();
Review comment:
We prefer to use `null == xxx` to instead of `xxx == null`
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386193039
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/TimeZoneSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+/**
+ * Variable segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class TimeZoneSegment implements SQLSegment {
+
+ private final int startIndex;
+
+ private final int stopIndex;
+
+ private final String variable;
Review comment:
What is the relationship of `TimeZoneSegment.variable` and `VariableSegment. variableName`?
Do we need to consider about make the name consist?
----------------------------------------------------------------
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 #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r384357105
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java
##########
@@ -145,14 +150,32 @@ public ASTNode visitShowOther(final ShowOtherContext ctx) {
public ASTNode visitSetVariable(final SetVariableContext ctx) {
SetStatement result = new SetStatement();
if (null != ctx.variable()) {
- result.setVariable((VariableSegment) visit(ctx.variable()));
+ List<VariableContext> variableContextList = ctx.variable();
+ List<VariableSegment> variableSegmentList = new ArrayList<VariableSegment>();
+ for (VariableContext variableContext : variableContextList) {
+ variableSegmentList.add((VariableSegment) visit(variableContext));
+ }
+ result.setVariable(variableSegmentList.get(0));
+
+ List<VariableValueContext> variableValueContextList = ctx.variableValue();
+ List<VariableValueSegment> variableValueSegmentList = new ArrayList<VariableValueSegment>();
+ for (VariableValueContext variableValueContext : variableValueContextList) {
+ variableValueSegmentList.add((VariableValueSegment) visit(variableValueContext));
+ }
+ result.setVariableValue(variableValueSegmentList.get(0));
}
return result;
}
+ @Override
+ public ASTNode visitVariableValue(final VariableValueContext ctx) {
+ VariableValueSegment result = new VariableValueSegment(ctx.getText());
Review comment:
Please inline the return 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] beijing-penguin commented on a change in
pull request #4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
beijing-penguin commented on a change in pull request #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#discussion_r386273341
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/TimeZoneSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+/**
+ * Variable segment.
Review comment:
done
----------------------------------------------------------------
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
#4477: Parse mysql variable value for SET statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #4477: Parse mysql variable value for SET statement
URL: https://github.com/apache/incubator-shardingsphere/pull/4477#issuecomment-591078581
## Pull Request Test Coverage Report for [Build 10061](https://coveralls.io/builds/29157267)
* **0** of **35** **(0.0%)** changed or added relevant lines in **3** files are covered.
* **11** unchanged lines in **3** files lost coverage.
* Overall coverage decreased (**-0.1%**) to **59.003%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dal/VariableValueSegment.java](https://coveralls.io/builds/29157267/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-engine%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fsql%2Fsegment%2Fdal%2FVariableValueSegment.java#L42) | 0 | 6 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/MySQLDALVisitor.java](https://coveralls.io/builds/29157267/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-mysql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FMySQLDALVisitor.java#L294) | 0 | 13 | 0.0%
| [shardingsphere-sql-parser/shardingsphere-sql-parser-postgresql/src/main/java/org/apache/shardingsphere/sql/parser/visitor/impl/PostgreSQLDALVisitor.java](https://coveralls.io/builds/29157267/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-postgresql%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fvisitor%2Fimpl%2FPostgreSQLDALVisitor.java#L52) | 0 | 16 | 0.0%
<!-- | **Total:** | **0** | **35** | **0.0%** | -->
| Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/execute/engine/DefaultSyncTaskExecuteEngine.java](https://coveralls.io/builds/29157267/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexecute%2Fengine%2FDefaultSyncTaskExecuteEngine.java#L94) | 2 | 84.85% |
| [sharding-orchestration/sharding-orchestration-core/src/main/java/org/apache/shardingsphere/orchestration/internal/util/IpUtils.java](https://coveralls.io/builds/29157267/source?filename=sharding-orchestration%2Fsharding-orchestration-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Forchestration%2Finternal%2Futil%2FIpUtils.java#L63) | 3 | 76.0% |
| [sharding-scaling/sharding-scaling-core/src/main/java/org/apache/shardingsphere/shardingscaling/core/execute/executor/channel/MemoryChannel.java](https://coveralls.io/builds/29157267/source?filename=sharding-scaling%2Fsharding-scaling-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fshardingscaling%2Fcore%2Fexecute%2Fexecutor%2Fchannel%2FMemoryChannel.java#L61) | 6 | 59.26% |
<!-- | **Total:** | **11** | | -->
| Totals | [![Coverage Status](https://coveralls.io/builds/29157267/badge)](https://coveralls.io/builds/29157267) |
| :-- | --: |
| Change from base [Build 998](https://coveralls.io/builds/29154645): | -0.1% |
| Covered Lines: | 12108 |
| Relevant Lines: | 20521 |
---
##### 💛 - [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