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/09/22 17:51:15 UTC
[GitHub] [shardingsphere] Icesource opened a new pull request #12634: add SQLServer simple alter and drop grammar
Icesource opened a new pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634
For #6478.
Changes proposed in this pull request:
- add alter procedure, alter function, alter view grammar
- add drop database, drop function, drop procedure, drop view, drop trigger, drop sequence
- add tests for drop statement
Hi, @jingshanglu , I've added these changes, please check it. And alter statements have the same sytnax with corresponding create statements, do I need to add tests for the alter statements?
--
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] jingshanglu merged pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
jingshanglu merged pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634
--
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] jingshanglu commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r714441086
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerAlterFunctionStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.AlterFunctionStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer alter procedure statement.
+ */
+@ToString
+public class SQLServerAlterFunctionStatement extends AlterFunctionStatement implements SQLServerStatement {
+}
Review comment:
@Icesource should `alter function`.
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
@Icesource It's better to keep `SQLserver`, maybe we can use `xxxStatement` instead of `SQLServerxxxStatement` and modify visitor of these rule.
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropProcedureStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropProcedureStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropProcedureStatement extends DropProcedureStatement implements SQLServerStatement {
+}
Review comment:
should be `drop procedure`
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropFunctionStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropFunctionStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropFunctionStatement extends DropFunctionStatement implements SQLServerStatement {
+}
Review comment:
@Icesource Maybe should be `drop function` ,not `drop database`.
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropViewStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropViewStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropViewStatement extends DropViewStatement implements SQLServerStatement {
+}
Review comment:
should be `drop view`.
--
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r714930911
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerAlterFunctionStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.AlterFunctionStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer alter procedure statement.
+ */
+@ToString
+public class SQLServerAlterFunctionStatement extends AlterFunctionStatement implements SQLServerStatement {
+}
Review comment:
done
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropProcedureStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropProcedureStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropProcedureStatement extends DropProcedureStatement implements SQLServerStatement {
+}
Review comment:
done
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropViewStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropViewStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropViewStatement extends DropViewStatement implements SQLServerStatement {
+}
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r714930759
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/dialect/statement/sqlserver/ddl/SQLServerDropFunctionStatement.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.sqlserver.ddl;
+
+import lombok.ToString;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DropFunctionStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.sqlserver.SQLServerStatement;
+
+/**
+ * SQLServer drop database statement.
+ */
+@ToString
+public class SQLServerDropFunctionStatement extends DropFunctionStatement implements SQLServerStatement {
+}
Review comment:
@jingshanglu done. Sry about that I don't check these comments carefully. Now it has been completely modified.
--
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r716058092
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
```
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
```
@jingshanglu This shows that I added a test on my page. It turned out that this test was only performed for PostgreSQL. Since the delete statement is the same, I also added SQLServer here. I did not delete any tests and the tests did not fail.
Maybe you read it wrong ?
--
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r714934043
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
@jingshanglu. I don't understand what you mean here. ```SQLServerxxxStatement``` extends ```xxxStatement``` and implements ```SQLServerStatement``` interface. Is there anything wrong with that ?
--
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] jingshanglu commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r716052093
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
@Icesource there, you delete test case for SQLServer, can you tell me why？if it will fail, maybe relate to the definition of `statement` and visitor of these rules.
--
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r714934043
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
@jingshanglu. I don't understand what you mean here. ```SQLServerxxxStatement``` extends ```xxxStatement``` and implements ```SQLServerStatement``` interface. For example, mysql has the MysqlDropViewStatement. Is my understanding wrong with that ?
--
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] Icesource commented on a change in pull request #12634: add SQLServer simple alter and drop grammar
Posted by GitBox <gi...@apache.org>.
Icesource commented on a change in pull request #12634:
URL: https://github.com/apache/shardingsphere/pull/12634#discussion_r716058092
##########
File path: shardingsphere-test/shardingsphere-parser-test/src/main/resources/sql/supported/ddl/drop-view.xml
##########
@@ -17,5 +17,5 @@
-->
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
Review comment:
```
<sql-cases>
- <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL" />
+ <sql-case id="drop_view" value="DROP VIEW kinds" db-types="PostgreSQL, SQLServer" />
</sql-cases>
```
@jingshanglu This shows that I added a test on my page instead of deleteing. It turned out that this test was only performed for PostgreSQL. Since the delete statement is the same, I also added SQLServer here. I did not delete any tests and the tests did not fail.
Maybe you read it wrong ?
--
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