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/04/05 13:38:01 UTC

[GitHub] [shardingsphere] zhujunxxxxx opened a new pull request #9940: Support oracle alter partition grammar

zhujunxxxxx opened a new pull request #9940:
URL: https://github.com/apache/shardingsphere/pull/9940


   Fixes #9694.
   
   Changes proposed in this pull request:
   - support oracle alter table add partition
   - support oracle alter table drop partition
   
   depend on: `https://docs.oracle.com/en/database/oracle/oracle-database/18/sqlrf/ALTER-TABLE.html#GUID-552E7373-BF93-477D-9DA3-B2C9386F2877`


-- 
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 #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       Hi, We can have a plan to support this grammar completely. Firstly, we can add the whole grammar definition, then we can consider improve the class of `CreateTableStatement`.




-- 
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 #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       Great.




-- 
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] wgy8283335 commented on a change in pull request #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       The hash partition and system partition also need to be added.




-- 
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] zhujunxxxxx commented on a change in pull request #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       @wgy8283335 
   I know, but I just used the existing syntax to build it. If want to support fully completed grammer maybe need @tristaZero open a issues.
   By the way, the grammer of partition in `CREATE TABLE` are also not support  fully completed grammer.




-- 
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 #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       So 
   ```
   The addRangePartitionClause and addListPartitionClause are not fully completed, below rules need to be done.
   table_partition_description
   external_part_subpart_data_props
   update_index_clauses
   ```
   Will these definitions be improved in the next PR?
   It is not a complete grammar definition, is it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [shardingsphere] wgy8283335 commented on a change in pull request #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       The addRangePartitionClause and addListPartitionClause are not fully completed, below rules need to be done.
   table_partition_description
   external_part_subpart_data_props
   update_index_clauses




-- 
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] zhujunxxxxx commented on pull request #9940: Support oracle alter partition grammar

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


   > Hi @zhujunxxxxx LGTM.
   > @wgy8283335 , could you give a review here? I am unsure your previous work covered this one or not.
   
   this PR just support SS can execute the SQL like `alter table t add partition .... ` and `alter table t drop partition ....`. 
   The question posed by @wgy8283335 is not of the same dimension as issues #9694


-- 
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] zhujunxxxxx commented on a change in pull request #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       Next PR complete the grammer of partition in `CreateTableStatement` and `AlterTableStatement` .




-- 
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 #9940: Support oracle alter partition grammar

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


   


-- 
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] zhujunxxxxx commented on a change in pull request #9940: Support oracle alter partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -919,3 +919,31 @@ flashbackArchiveClause
 alterSynonym
     : ALTER PUBLIC? SYNONYM (schemaName DOT_)? synonymName (COMPILE | EDITIONABLE | NONEDITIONABLE)
     ;
+
+alterTablePartition
+    : addTablePartition | dropTablePartition
+    ;
+
+addTablePartition
+    : ADD (addRangePartitionClause | addListPartitionClause)
+    ;
+
+addRangePartitionClause
+    : PARTITION partitionName? rangeValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+        | hashSubpartitionQuantity)?
+    ;
+
+addListPartitionClause
+    : PARTITION partitionName? listValuesClause tablePartitionDescription
+    ((LP_? rangeSubpartitionDesc (COMMA_ rangeSubpartitionDesc)* | listSubpartitionDesc (COMMA_ listSubpartitionDesc)* | individualHashSubparts (COMMA_ individualHashSubparts)* RP_?)
+    | hashSubpartitionQuantity)?
+    ;
+

Review comment:
       @tristaZero agree, this PR can merge first?




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