You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/07/14 15:57:05 UTC
[GitHub] [shardingsphere] strongduanmu opened a new pull request #6350: support mysql insert select statement parse and add some test cases
strongduanmu opened a new pull request #6350:
URL: https://github.com/apache/shardingsphere/pull/6350
Ref #6289 .
Changes proposed in this pull request:
- support mysql insert select statement parse
- add some test cases
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #6350: support mysql insert select statement parse and add some test cases
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #6350:
URL: https://github.com/apache/shardingsphere/pull/6350#discussion_r454851840
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dml/assignment/InsertSelectSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.segment.dml.assignment;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+/**
+ * Insert select segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public class InsertSelectSegment implements SQLSegment {
Review comment:
@tristaZero Thank you for your suggestion:
1. `InsertSelectSegment` and `SelectSegment` should be able to be merged into one class. The two classes were originally designed to be consistent with the syntax rules in antlr.
2. SelectSegment is created with reference to SubquerySegment. Is it misleading to use SubquerySegment because Select is not a subquery in the Insert Select statement?
3. The pr related to route and rewrite will be submitted later. In addition, subsequent pr will also contain replace select statement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero merged pull request #6350: support mysql insert select statement parse and add some test cases
Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #6350:
URL: https://github.com/apache/shardingsphere/pull/6350
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #6350: support mysql insert select statement parse and add some test cases
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #6350:
URL: https://github.com/apache/shardingsphere/pull/6350#discussion_r454851840
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dml/assignment/InsertSelectSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.segment.dml.assignment;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+/**
+ * Insert select segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public class InsertSelectSegment implements SQLSegment {
Review comment:
@tristaZero
1. `InsertSelectSegment` and `SelectSegment` should be able to be merged into one class. The two classes were originally designed to be consistent with the syntax rules in antlr.
2. SelectSegment is created with reference to SubquerySegment. Is it misleading to use SubquerySegment because Select is not a subquery in the Insert Select statement?
3. The pr related to route and rewrite will be submitted later. In addition, subsequent pr will also contain replace select statement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #6350: support mysql insert select statement parse and add some test cases
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #6350:
URL: https://github.com/apache/shardingsphere/pull/6350#discussion_r454810011
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/segment/dml/assignment/InsertSelectSegment.java
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.sql.parser.sql.segment.dml.assignment;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import org.apache.shardingsphere.sql.parser.sql.segment.SQLSegment;
+
+/**
+ * Insert select segment.
+ */
+@RequiredArgsConstructor
+@Getter
+public class InsertSelectSegment implements SQLSegment {
Review comment:
Hi @strongduanmu
This PR looks clean and complete with many unit tests. Good job!
Besides, here are some of my questions, waiting for your feedback.
1. `InsertSelectSegment` and `SelectSegment` looks similar, do you think it is possible to remain either of them?
2. The existing class of `SubquerySegment` and new `SelectSegment` are alike somehow. Could you give them a look again?
3. Are there any consequent PRs needed to complete this feature later?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org