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 2021/11/01 13:43:02 UTC

[GitHub] [shardingsphere] flycash opened a new pull request #13398: support parsing RESET statement for mysql

flycash opened a new pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398


   Fixes #13148.
   
   Changes proposed in this pull request:
   - MySQLResetStatement
   - MySQLResetPersistStatement
   -  Override methods `visitResetOption`, `visitBinaryLogFileIndexNumber` and visitChannelOption`
   
   Notice to reviewer:
   - I found that the syntax is different from MySQL preference, and I choose to follow the definition in .g4 files
   - I think it's better to use `MySQLResetStatement` and `MySQLResetPersistStatement`, because it makes code clear
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-960536733


   sorry, some files are not committed. I push again and fixed all issues


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-961089290


   Hi, I have resolved conflicts, please help to check 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742435867



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();

Review comment:
       @flycash Please use static import instead.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public final class ResetOptionSegment implements SQLSegment {

Review comment:
       @flycash Can we move these fields to MySQLResetMasterStatement and MySQLResetSlaveStatement?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       @flycash Can we split MySQLResetStatement to MySQLResetMasterStatement and MySQLResetSlaveStatement? They seem to be two statements. At the same time, there should be no need to parse keywords like MASTER, SLAVE, ALL.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetStatementTestCase.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.List;
+
+/**
+ * reset statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {

Review comment:
       @flycash Please use static import instead, and modify optCtx to each.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/DALStatementAssert.java
##########
@@ -167,6 +173,10 @@ public static void assertIs(final SQLCaseAssertContext assertContext, final DALS
             ShowFunctionStatusStatementAssert.assertIs(assertContext, (MySQLShowFunctionStatusStatement) actual, (ShowFunctionStatusStatementTestCase) expected);
         } else if (actual instanceof MySQLShowProcedureStatusStatement) {
             ShowProcedureStatusStatementAssert.assertIs(assertContext, (MySQLShowProcedureStatusStatement) actual, (ShowProcedureStatusStatementTestCase) expected);
+        } else if (actual instanceof MySQLResetStatement) {
+            MySQLResetStatementAssert.assertIs(assertContext, (MySQLResetStatement) actual, (ResetStatementTestCase) expected);

Review comment:
       @flycash Same suggestion here. Split MySQLResetStatementAssert to MySQLResetMasterStatementAssert and MySQLResetSlaveStatementAssert.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.

Review comment:
       @flycash The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetPersistStatementAssert {
+
+    /**
+     * Assert reset persist statement is correct with expected reset persist statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset persist statement
+     * @param expected      expected reset persist statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetPersistStatement actual, final ResetPersistStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual reset persist exist clause does not match: "), actual.isContainsExistClause(), is(expected.isContainsExistClause()));
+        Optional.ofNullable(expected.getIdentifier()).ifPresent(identifier -> {

Review comment:
       Can you refer to the writing of other scenes? We expect to maintain the same coding style.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetStatementAssert {
+
+    /**
+     * Assert reset statement is correct with expected reset statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset statement
+     * @param expected      expected reset statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetStatement actual, final ResetStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual options size assertion error: "), actual.getOptions().size(), is(expected.getOptions().size()));
+        assertOptions(assertContext, actual.getOptions(), expected.getOptions());
+    }
+
+    private static void assertOptions(final SQLCaseAssertContext assertContext, final List<ResetOptionSegment> actual, final List<ExpectedResetOptionSegment> expected) {
+        for (int i = 0; i < actual.size(); i++) {
+            ResetOptionSegment current = actual.get(i);
+            ExpectedResetOptionSegment exp = expected.get(i);
+            assertThat(assertContext.getText("Actual reset target does not match: "), current.getTarget(), is(exp.getTarget()));
+            assertThat(assertContext.getText("Actual reset option start index does not match: "), current.getStartIndex(), is(exp.getStartIndex()));

Review comment:
       @flycash Please use the common method(SQLSegmentAssert.assertIs) to handle the index assertion.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetPersistStatementTestCase.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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+/**
+ * reset persist statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();

Review comment:
       @flycash Please use static import instead.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public final class ResetOptionSegment implements SQLSegment {

Review comment:
       @flycash Can we move these fields to MySQLResetMasterStatement and MySQLResetSlaveStatement?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       @flycash Can we split MySQLResetStatement to MySQLResetMasterStatement and MySQLResetSlaveStatement? They seem to be two statements. At the same time, there should be no need to parse keywords like MASTER, SLAVE, ALL.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetStatementTestCase.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.List;
+
+/**
+ * reset statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {

Review comment:
       @flycash Please use static import instead, and modify optCtx to each.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/DALStatementAssert.java
##########
@@ -167,6 +173,10 @@ public static void assertIs(final SQLCaseAssertContext assertContext, final DALS
             ShowFunctionStatusStatementAssert.assertIs(assertContext, (MySQLShowFunctionStatusStatement) actual, (ShowFunctionStatusStatementTestCase) expected);
         } else if (actual instanceof MySQLShowProcedureStatusStatement) {
             ShowProcedureStatusStatementAssert.assertIs(assertContext, (MySQLShowProcedureStatusStatement) actual, (ShowProcedureStatusStatementTestCase) expected);
+        } else if (actual instanceof MySQLResetStatement) {
+            MySQLResetStatementAssert.assertIs(assertContext, (MySQLResetStatement) actual, (ResetStatementTestCase) expected);

Review comment:
       @flycash Same suggestion here. Split MySQLResetStatementAssert to MySQLResetMasterStatementAssert and MySQLResetSlaveStatementAssert.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.

Review comment:
       @flycash The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetPersistStatementAssert {
+
+    /**
+     * Assert reset persist statement is correct with expected reset persist statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset persist statement
+     * @param expected      expected reset persist statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetPersistStatement actual, final ResetPersistStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual reset persist exist clause does not match: "), actual.isContainsExistClause(), is(expected.isContainsExistClause()));
+        Optional.ofNullable(expected.getIdentifier()).ifPresent(identifier -> {

Review comment:
       Can you refer to the writing of other scenes? We expect to maintain the same coding style.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetStatementAssert {
+
+    /**
+     * Assert reset statement is correct with expected reset statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset statement
+     * @param expected      expected reset statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetStatement actual, final ResetStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual options size assertion error: "), actual.getOptions().size(), is(expected.getOptions().size()));
+        assertOptions(assertContext, actual.getOptions(), expected.getOptions());
+    }
+
+    private static void assertOptions(final SQLCaseAssertContext assertContext, final List<ResetOptionSegment> actual, final List<ExpectedResetOptionSegment> expected) {
+        for (int i = 0; i < actual.size(); i++) {
+            ResetOptionSegment current = actual.get(i);
+            ExpectedResetOptionSegment exp = expected.get(i);
+            assertThat(assertContext.getText("Actual reset target does not match: "), current.getTarget(), is(exp.getTarget()));
+            assertThat(assertContext.getText("Actual reset option start index does not match: "), current.getStartIndex(), is(exp.getStartIndex()));

Review comment:
       @flycash Please use the common method(SQLSegmentAssert.assertIs) to handle the index assertion.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetPersistStatementTestCase.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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+/**
+ * reset persist statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742435867



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();

Review comment:
       @flycash Please use static import instead.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public final class ResetOptionSegment implements SQLSegment {

Review comment:
       @flycash Can we move these fields to MySQLResetMasterStatement and MySQLResetSlaveStatement?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       @flycash Can we split MySQLResetStatement to MySQLResetMasterStatement and MySQLResetSlaveStatement? They seem to be two statements. At the same time, there should be no need to parse keywords like MASTER, SLAVE, ALL.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetStatementTestCase.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.List;
+
+/**
+ * reset statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {

Review comment:
       @flycash Please use static import instead, and modify optCtx to each.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.

Review comment:
       The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/DALStatementAssert.java
##########
@@ -167,6 +173,10 @@ public static void assertIs(final SQLCaseAssertContext assertContext, final DALS
             ShowFunctionStatusStatementAssert.assertIs(assertContext, (MySQLShowFunctionStatusStatement) actual, (ShowFunctionStatusStatementTestCase) expected);
         } else if (actual instanceof MySQLShowProcedureStatusStatement) {
             ShowProcedureStatusStatementAssert.assertIs(assertContext, (MySQLShowProcedureStatusStatement) actual, (ShowProcedureStatusStatementTestCase) expected);
+        } else if (actual instanceof MySQLResetStatement) {
+            MySQLResetStatementAssert.assertIs(assertContext, (MySQLResetStatement) actual, (ResetStatementTestCase) expected);

Review comment:
       @flycash Same suggestion here. Split MySQLResetStatementAssert to MySQLResetMasterStatementAssert and MySQLResetSlaveStatementAssert.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.

Review comment:
       @flycash The first letter of javadoc needs to be capitalized.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetPersistStatementAssert {
+
+    /**
+     * Assert reset persist statement is correct with expected reset persist statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset persist statement
+     * @param expected      expected reset persist statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetPersistStatement actual, final ResetPersistStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual reset persist exist clause does not match: "), actual.isContainsExistClause(), is(expected.isContainsExistClause()));
+        Optional.ofNullable(expected.getIdentifier()).ifPresent(identifier -> {

Review comment:
       Can you refer to the writing of other scenes? We expect to maintain the same coding style.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetStatementAssert {
+
+    /**
+     * Assert reset statement is correct with expected reset statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset statement
+     * @param expected      expected reset statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetStatement actual, final ResetStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual options size assertion error: "), actual.getOptions().size(), is(expected.getOptions().size()));
+        assertOptions(assertContext, actual.getOptions(), expected.getOptions());
+    }
+
+    private static void assertOptions(final SQLCaseAssertContext assertContext, final List<ResetOptionSegment> actual, final List<ExpectedResetOptionSegment> expected) {
+        for (int i = 0; i < actual.size(); i++) {
+            ResetOptionSegment current = actual.get(i);
+            ExpectedResetOptionSegment exp = expected.get(i);
+            assertThat(assertContext.getText("Actual reset target does not match: "), current.getTarget(), is(exp.getTarget()));
+            assertThat(assertContext.getText("Actual reset option start index does not match: "), current.getStartIndex(), is(exp.getStartIndex()));

Review comment:
       @flycash Please use the common method(SQLSegmentAssert.assertIs) to handle the index assertion.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetPersistStatementTestCase.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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+/**
+ * reset persist statement test case.

Review comment:
       The first letter of javadoc needs to be capitalized.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r740647818



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       @flycash Please add final for ExpectedResetOptionSegment.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       Add final for ResetOptionSegment is better.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       @flycash Please remove this useless blank line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       @flycash Please remove this black line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       @flycash Do you think it is better to use `ctx.resetOption()` instead of `opts`? Because it was only used once.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {
+
+    private int startIndex;
+
+    private int stopIndex;
+
+    // master or slave

Review comment:
       @flycash Please remove this comment.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

Review comment:
       @flycash Please remove this useless black 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin removed a comment on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin removed a comment on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu merged pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu merged pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398


   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741018988



##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-958968449


   @strongduanmu Hi, I have corrected all comments, could you help to review again?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-960306004


   > @strongduanmu Hi, I have corrected all comments, could you help to review again?
   
   @flycash Of course, I will review this pr again.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742577582



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       I use second solution




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-960306004






-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-958968449






-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742577859



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public final class ResetOptionSegment implements SQLSegment {

Review comment:
       I think we need this Option structure as ResetStatement accepts multiple options




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741017968



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r740647818



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       @flycash Please add final for ExpectedResetOptionSegment.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       Add final for ResetOptionSegment is better.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       @flycash Please remove this useless blank line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       @flycash Please remove this black line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       @flycash Do you think it is better to use `ctx.resetOption()` instead of `opts`? Because it was only used once.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {
+
+    private int startIndex;
+
+    private int stopIndex;
+
+    // master or slave

Review comment:
       @flycash Please remove this comment.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

Review comment:
       @flycash Please remove this useless black 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741017968



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       yes, corrected

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r740647818



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       @flycash Please add final for ExpectedResetOptionSegment.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       @flycash Please add a blank line before this line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       Add final for ResetOptionSegment is better.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       @flycash Please remove this useless blank line.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       @flycash Please remove this black line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       @flycash Do you think it is better to use `ctx.resetOption()` instead of `opts`? Because it was only used once.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       @flycash Please put null on the left side of the expression.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {
+
+    private int startIndex;
+
+    private int stopIndex;
+
+    // master or slave

Review comment:
       @flycash Please remove this comment.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

Review comment:
       @flycash Please remove this useless black 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742539665



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       Using the same statement is OK, but what I want to say is, do we need to parse out the keywords? These parsed field seem to be useless, whether they can be deleted?




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742594515



##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetPersistStatementTestCase.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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+/**
+ * reset persist statement test case.

Review comment:
       Why can't I see the changes? Not submitted?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -235,10 +242,64 @@ public ASTNode visitShowErrors(final ShowErrorsContext ctx) {
     public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
         return new MySQLShowWarningsStatement();
     }
-    

Review comment:
       @flycash Please keep the same indentation as the previous line.

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetMasterOptionSegment.java
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+
+@Getter
+@Setter
+public final class ResetMasterOptionSegment extends ResetOptionSegment {

Review comment:
       Add javadoc for ResetMasterOptionSegment, ResetSlaveOptionSegment and ResetOptionSegment.

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetStatementTestCase.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.List;
+
+/**
+ * reset statement test case.

Review comment:
       Why can't I see the changes? Not submitted?

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -235,10 +242,64 @@ public ASTNode visitShowErrors(final ShowErrorsContext ctx) {
     public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
         return new MySQLShowWarningsStatement();
     }
-    
+
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (ResetOptionContext each : ctx.resetOption()) {
+            result.getOptions().add((ResetOptionSegment) (visit(each)));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (null != ctx.existClause()) {
+            result.setContainsExistClause(true);
+        }
+        if (null != ctx.identifier()) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final ResetOptionContext ctx) {
+        if (null != ctx.MASTER()) {
+            ResetMasterOptionSegment result = new ResetMasterOptionSegment();
+            if (null != ctx.binaryLogFileIndexNumber()) {
+                result.setBinaryLogFileIndexNumber((NumberLiteralValue) visit(ctx.binaryLogFileIndexNumber()));
+            }
+            result.setStartIndex(ctx.start.getStartIndex());
+            result.setStopIndex(ctx.stop.getStopIndex());
+            return result;
+        }
+

Review comment:
       @flycash Please remove this 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741017968



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetPersistStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.value.identifier.IdentifierValue;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+
+/**
+ * MySQL reset statement.
+ */
+@ToString
+@Getter
+@Setter
+public final class MySQLResetPersistStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private boolean containsExistClause;

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/dal/MySQLResetStatement.java
##########
@@ -17,14 +17,21 @@
 
 package org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal;
 
+import lombok.Getter;
 import lombok.ToString;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.AbstractSQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.DALStatement;
 import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.MySQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+
+import java.util.LinkedList;
+import java.util.List;
 
 /**
  * MySQL reset statement.
  */
 @ToString
+@Getter
 public final class MySQLResetStatement extends AbstractSQLStatement implements DALStatement, MySQLStatement {
+    private List<ResetOptionSegment> options = new LinkedList<>();

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {
+    @XmlAttribute(name = "target")

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       yes, corrected

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin removed a comment on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin removed a comment on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu merged pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu merged pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398


   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-960306004


   > @strongduanmu Hi, I have corrected all comments, could you help to review again?
   
   @flycash Of course, I will review this pr again.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin removed a comment on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin removed a comment on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741019521



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/mysql/segment/ResetOptionSegment.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.dialect.statement.mysql.segment;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.shardingsphere.sql.parser.sql.common.segment.SQLSegment;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.NumberLiteralValue;
+import org.apache.shardingsphere.sql.parser.sql.common.value.literal.impl.StringLiteralValue;
+
+@Getter
+@Setter
+public class ResetOptionSegment implements SQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/segment/impl/reset/ExpectedResetOptionSegment.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.AbstractExpectedSQLSegment;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+@Getter
+public class ExpectedResetOptionSegment extends AbstractExpectedSQLSegment {

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/case/dal/reset.xml
##########
@@ -17,6 +17,32 @@
   -->
 
 <sql-parser-test-cases>
-    <common sql-case-id="reset_all" />
-    <common sql-case-id="reset_timezone" />
+    <common sql-case-id="reset_all"/>
+    <common sql-case-id="reset_timezone"/>
+    <reset sql-case-id="reset_master">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+    </reset>
+    <reset sql-case-id="reset_slave">
+        <option target="SLAVE" start-index="6" stop-index="10"/>
+    </reset>
+    <reset sql-case-id="reset_master_slave">
+        <option target="MASTER" start-index="6" stop-index="11"/>
+        <option target="SLAVE" start-index="14" stop-index="18"/>
+    </reset>
+    <reset sql-case-id="reset_master_with_binlog">
+        <option target="MASTER" binary-log-file-index-number="10" start-index="6" stop-index="17"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all">
+        <option target="SLAVE" all="ALL" start-index="6" stop-index="14"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_channel">
+        <option target="SLAVE" channel="TEST_CHANNEL" start-index="6" stop-index="37"/>
+    </reset>
+    <reset sql-case-id="reset_slave_with_all_channel">
+        <option target="SLAVE" all="ALL" channel="TEST_CHANNEL" start-index="6" stop-index="41"/>
+    </reset>
+

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/dal/reset.xml
##########
@@ -19,4 +19,15 @@
 <sql-cases>
     <sql-case id="reset_all" value="RESET ALL"  db-types="PostgreSQL,openGauss" />
     <sql-case id="reset_timezone" value="RESET timezone"  db-types="PostgreSQL,openGauss" />
+    <sql-case id="reset_master" value="RESET MASTER" db-types="MySQL"/>
+    <sql-case id="reset_slave" value="RESET SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_slave" value="RESET MASTER, SLAVE" db-types="MySQL"/>
+    <sql-case id="reset_master_with_binlog" value="RESET MASTER TO 10" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all" value="RESET SLAVE ALL" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_channel" value="RESET SLAVE FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+    <sql-case id="reset_slave_with_all_channel" value="RESET SLAVE ALL FOR CHANNEL 'TEST_CHANNEL'" db-types="MySQL"/>
+

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-960517803


   I fixed all issues, and I will fix other PRs after this PR was merged


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742535971



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       I think may be we cannot split MySQLResetStatement. Here is the syntax:
   ```g4
   resetStatement
       : RESET resetOption (COMMA_ resetOption)*
       | resetPersist
       ;
   ```
   We can use `RESET` for both `RESET MASTER, SLAVE`, and you can find that I have provided a test case for this scenario. 
   Any good idea?




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-961137046


   ……


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] strongduanmu merged pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
strongduanmu merged pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398


   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tuichenchuxin commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
tuichenchuxin commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-957057730


   @flycash Thank you for your pr. I found that you have noticed the difference between the definition in the g4 file and the actual mysql document, but I think the definition of g4 should be modified to adapt to the requirements of the mysql document.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#issuecomment-958968449


   @strongduanmu Hi, I have corrected all comments, could you help to review again?


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r741019158



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/database/visitor/SQLVisitorRule.java
##########
@@ -282,6 +282,11 @@
     
     REPAIR_TABLE("RepairTable", SQLStatementType.DAL),
     
+

Review comment:
       done

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,57 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (persistContext != null) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        List<MySQLStatementParser.ResetOptionContext> opts = ctx.resetOption();
+        for (MySQLStatementParser.ResetOptionContext optCtx : opts) {

Review comment:
       yes, corrected




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742541198



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLDALStatementSQLVisitor.java
##########
@@ -238,7 +243,56 @@ public ASTNode visitShowWarnings(final ShowWarningsContext ctx) {
     
     @Override
     public ASTNode visitResetStatement(final ResetStatementContext ctx) {
-        return new MySQLResetStatement();
+        MySQLStatementParser.ResetPersistContext persistContext = ctx.resetPersist();
+        if (null != persistContext) {
+            return visit(persistContext);
+        }
+        MySQLResetStatement result = new MySQLResetStatement();
+        for (MySQLStatementParser.ResetOptionContext optCtx : ctx.resetOption()) {
+            ResetOptionSegment opt = (ResetOptionSegment) (visit(optCtx));
+            result.getOptions().add(opt);
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetPersist(final MySQLStatementParser.ResetPersistContext ctx) {
+        MySQLResetPersistStatement result = new MySQLResetPersistStatement();
+        if (ctx.existClause() != null) {
+            result.setContainsExistClause(true);
+        }
+        if (ctx.identifier() != null) {
+            result.setIdentifier(new IdentifierValue(ctx.identifier().getText()));
+        }
+        return result;
+    }
+
+    @Override
+    public ASTNode visitResetOption(final MySQLStatementParser.ResetOptionContext ctx) {
+        ResetOptionSegment result = new ResetOptionSegment();
+        result.setStartIndex(ctx.start.getStartIndex());
+        result.setStopIndex(ctx.stop.getStopIndex());
+        if (ctx.MASTER() != null) {
+            result.setTarget(ctx.MASTER().getText());

Review comment:
       I can change them to boolean value like :
   ```java
   boolean master
   boolean all
   ```
   Another alternative solution is using two option classes:
   ```java
   class ResetOptionSegement {
   }
   class ResetMasterOptionSegment{
   }
   class ResetSlaveOptionSegment{}
   ```
   I think both of these two solutions are ok




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742578241



##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetPersistStatementTestCase.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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlAttribute;
+
+/**
+ * reset persist statement test case.

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetStatementAssert.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.segment.ResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetStatementTestCase;
+
+import java.util.List;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetStatementAssert {
+
+    /**
+     * Assert reset statement is correct with expected reset statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset statement
+     * @param expected      expected reset statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetStatement actual, final ResetStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual options size assertion error: "), actual.getOptions().size(), is(expected.getOptions().size()));
+        assertOptions(assertContext, actual.getOptions(), expected.getOptions());
+    }
+
+    private static void assertOptions(final SQLCaseAssertContext assertContext, final List<ResetOptionSegment> actual, final List<ExpectedResetOptionSegment> expected) {
+        for (int i = 0; i < actual.size(); i++) {
+            ResetOptionSegment current = actual.get(i);
+            ExpectedResetOptionSegment exp = expected.get(i);
+            assertThat(assertContext.getText("Actual reset target does not match: "), current.getTarget(), is(exp.getTarget()));
+            assertThat(assertContext.getText("Actual reset option start index does not match: "), current.getStartIndex(), is(exp.getStartIndex()));

Review comment:
       done

##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/jaxb/cases/domain/statement/dal/ResetStatementTestCase.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal;
+
+import lombok.Getter;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.segment.impl.reset.ExpectedResetOptionSegment;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.SQLParserTestCase;
+
+import javax.xml.bind.annotation.XmlElement;
+import java.util.List;
+
+/**
+ * reset statement test case.

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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] flycash commented on a change in pull request #13398: support parsing RESET statement for mysql

Posted by GitBox <gi...@apache.org>.
flycash commented on a change in pull request #13398:
URL: https://github.com/apache/shardingsphere/pull/13398#discussion_r742578064



##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/java/org/apache/shardingsphere/test/sql/parser/parameterized/asserts/statement/dal/impl/MySQLResetPersistStatementAssert.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.test.sql.parser.parameterized.asserts.statement.dal.impl;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLResetPersistStatement;
+import org.apache.shardingsphere.test.sql.parser.parameterized.asserts.SQLCaseAssertContext;
+import org.apache.shardingsphere.test.sql.parser.parameterized.jaxb.cases.domain.statement.dal.ResetPersistStatementTestCase;
+
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * reset persist statement assert.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class MySQLResetPersistStatementAssert {
+
+    /**
+     * Assert reset persist statement is correct with expected reset persist statement test case.
+     *
+     * @param assertContext assert context
+     * @param actual        actual reset persist statement
+     * @param expected      expected reset persist statement test case
+     */
+    public static void assertIs(final SQLCaseAssertContext assertContext, final MySQLResetPersistStatement actual, final ResetPersistStatementTestCase expected) {
+        assertThat(assertContext.getText("Actual reset persist exist clause does not match: "), actual.isContainsExistClause(), is(expected.isContainsExistClause()));
+        Optional.ofNullable(expected.getIdentifier()).ifPresent(identifier -> {

Review comment:
       done, please check 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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org