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/12 11:55:16 UTC

[GitHub] [shardingsphere] Liangda-w opened a new pull request #10052: Complete Oracle drop table partition grammar

Liangda-w opened a new pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052


   Relates #9694 .
   
   Changes proposed in this pull request:
   - Complete `drop_table_partition` of [ALTER TABLE](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/ALTER-TABLE.html#GUID-552E7373-BF93-477D-9DA3-B2C9386F2877) and add corresponding test cases
   
   @wgy8283335 
   


-- 
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] codecov-commenter edited a comment on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-822490830


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#10052](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c15b09) into [master](https://codecov.io/gh/apache/shardingsphere/commit/ae1cda02736e6ad8ea60872d66cae2a786f40790?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ae1cda0) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10052/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #10052   +/-   ##
   =========================================
     Coverage     67.97%   67.97%           
     Complexity      679      679           
   =========================================
     Files          1690     1690           
     Lines         28846    28846           
     Branches       5149     5149           
   =========================================
     Hits          19608    19608           
     Misses         7755     7755           
     Partials       1483     1483           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ae1cda0...7c15b09](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 edited a comment on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
wgy8283335 edited a comment on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-822876695


   > Hi, I think since we can't let `physicalAttributesClause` contains an empty String, we can indicate the possibility of empty String at a higher level (in the clauses conatining `physicalAttributesClause`). So for example I changed `oidIndexClause` to
   > 
   > `OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_`
   > 
   > What do think of this way to solve this issue? @wgy8283335 @tristaZero
   
   I think this way could solve this issue. But I am not sure whether this way is good enough.
   


-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   @Liangda-w @wgy8283335 @zhujunxxxxx Thanks for your nice work, all your guys.


-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (DOT_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (DOT_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (DOT_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?

Review comment:
       Hi @Liangda-w , `DOT_` mean `.` but there should be `,` so should use `COMMA_` here




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

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

[GitHub] [shardingsphere] wgy8283335 commented on a change in pull request #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       “(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)* ” is more suitable.




-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       “(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storage clause)* ” is more suitable, but if we change physicalAttributesClause like this, it will happen as bellow:
   "rule oidIndexClause contains a closure with at least one alternative that can match an empty string".
   @tristaZero how could we resolve this problem?




-- 
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] Liangda-w commented on a change in pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on a change in pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#discussion_r613640234



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       Yes! It should be `(segmentAttributesClause | indexCompression)+`. 
   
   Actually this relates to my question in #10016. Before when I add this `+`, it give me the following error
   `rule XXX contains an optional block with at least one alternative that can match an empty string`
   
   I notice that you changed the rule `physicalAttributesClause` from
   `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)*` to 
   `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)+` to avoid this error. 
   
   However, I think the former rule is more accurate as it could also be empty. Should we then use `physicalAttributesClause?` in rules which contains `physicalAttributesClause` to allow empty input as well?
   ![image](https://user-images.githubusercontent.com/66914151/114790430-07ae3280-9d85-11eb-9db7-55501a535ce8.png)




-- 
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] Liangda-w commented on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-822469483


   Hi, I think since we can't let `physicalAttributesClause` contains an empty String, we can indicate the possibility of empty String at a higher level (in the clauses conatining `physicalAttributesClause`). So for example I changed `oidIndexClause` to
   
   `OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_`
   
   What do think of this way to solve this issue? @wgy8283335 @tristaZero 


-- 
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] Liangda-w commented on a change in pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on a change in pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#discussion_r613640234



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       Yes! It should be `(segmentAttributesClause | indexCompression)+`. 
   
   Actually this relates to my question in #10016. Before when I add this `+`, it give me the following error
   `rule XXX contains an optional block with at least one alternative that can match an empty string`
   
   I notice that you changed the rule `physicalAttributesClause` from
   `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)*` to 
   `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)+` to avoid this error. 
   
   However, I think the former rule is more accurate as it could also be empty?
   
   ![image](https://user-images.githubusercontent.com/66914151/114790430-07ae3280-9d85-11eb-9db7-55501a535ce8.png)




-- 
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 #10052: Complete Oracle drop table partition grammar

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


   @wgy8283335  if use `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)+` will get an error. I think we can use `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause) (PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)*` to resolve to this error. @Liangda-w  do you think so.


-- 
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] Liangda-w commented on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-821201358


   > @wgy8283335 if use `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)+` will get an error. I think we can use `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause) (PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)*` to resolve to this error. @Liangda-w do you think so.
   
   Hi @zhujunxxxxx, thank you for the suggestion:) I think my expression before might not be so clear. 
   
   Actually `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)*` will causes this error 
   `rule XXX contains an optional block with at least one alternative that can match an empty string` for the rule `oidIndexClause` and `indexPartitionDesc`, which in my opinion couldn't be avoided due to their definitions. 
   
   Currently `(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)+` is fine, but it's not so suitable because `physicalAttributesClause`  also allows an empty string. 
   
   @tristaZero do you have any suggestions?


-- 
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] Liangda-w commented on a change in pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on a change in pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#discussion_r613629440



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)

Review comment:
       yes, thank you for pointing it out:)




-- 
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] codecov-commenter commented on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-822490830


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#10052](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c36f7a) into [master](https://codecov.io/gh/apache/shardingsphere/commit/d312e297b1233740f45455a28a546948a55ce590?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d312e29) will **decrease** coverage by `0.03%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10052/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #10052      +/-   ##
   ============================================
   - Coverage     68.08%   68.05%   -0.04%     
   - Complexity      676      679       +3     
   ============================================
     Files          1685     1690       +5     
     Lines         28620    28846     +226     
     Branches       5083     5149      +66     
   ============================================
   + Hits          19486    19630     +144     
   - Misses         7668     7732      +64     
   - Partials       1466     1484      +18     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...infra/rule/builder/ShardingSphereRulesBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9ydWxlL2J1aWxkZXIvU2hhcmRpbmdTcGhlcmVSdWxlc0J1aWxkZXIuamF2YQ==) | `37.50% <0.00%> (-7.96%)` | `0.00% <0.00%> (ø%)` | |
   | [...end/text/distsql/rdl/RDLBackendHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmRsL1JETEJhY2tlbmRIYW5kbGVyRmFjdG9yeS5qYXZh) | `80.00% <0.00%> (-5.72%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/governance/core/registry/RegistryCenterNode.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlck5vZGUuamF2YQ==) | `80.00% <0.00%> (-3.34%)` | `0.00% <0.00%> (ø%)` | |
   | [...nd/text/admin/mysql/MySQLAdminExecutorFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL015U1FMQWRtaW5FeGVjdXRvckZhY3RvcnkuamF2YQ==) | `42.85% <0.00%> (-3.30%)` | `1.00% <0.00%> (ø%)` | |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `61.71% <0.00%> (-1.45%)` | `0.00% <0.00%> (ø%)` | |
   | [...authentication/PostgreSQLAuthenticationEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9hdXRoZW50aWNhdGlvbi9Qb3N0Z3JlU1FMQXV0aGVudGljYXRpb25FbmdpbmUuamF2YQ==) | `80.39% <0.00%> (-0.38%)` | `1.00% <0.00%> (+1.00%)` | :arrow_down: |
   | [...or/statement/impl/MySQLDALStatementSQLVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWRpYWxlY3Qvc2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci1teXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9teXNxbC92aXNpdG9yL3N0YXRlbWVudC9pbXBsL015U1FMREFMU3RhdGVtZW50U1FMVmlzaXRvci5qYXZh) | `36.02% <0.00%> (-0.27%)` | `1.00% <0.00%> (ø%)` | |
   | [.../apache/shardingsphere/shadow/rule/ShadowRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhZG93L3NoYXJkaW5nc3BoZXJlLXNoYWRvdy1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NoYWRvdy9ydWxlL1NoYWRvd1J1bGUuamF2YQ==) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/shardingsphere/encrypt/rule/EncryptRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC9ydWxlL0VuY3J5cHRSdWxlLmphdmE=) | `78.18% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | [...che/shardingsphere/sharding/rule/ShardingRule.java](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zaGFyZGluZy9ydWxlL1NoYXJkaW5nUnVsZS5qYXZh) | `69.69% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | ... and [59 more](https://codecov.io/gh/apache/shardingsphere/pull/10052/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d312e29...0c36f7a](https://codecov.io/gh/apache/shardingsphere/pull/10052?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   @zhujunxxxxx Wait for your review here. :)
   Hi @Liangda-w 
   Thanks for this PR, BTW your application of GSoC 2021 is approved by me.


-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -106,7 +106,7 @@ oidClause
     ;
 
 oidIndexClause
-    : OIDINDEX indexName? LP_ (physicalAttributesClause | TABLESPACE tablespaceName)+ RP_
+    : OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_
     ;

Review comment:
       How about like this?
   `oidIndexClause
       : OIDINDEX indexName? LP_ (physicalAttributesClause | TABLESPACE tablespaceName)* RP_
       ;`




-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       “(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)* ” is more suitable.




-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   Hi everyone,
   
   Currently this PR seems blocked by `physicalAttributesClause`, does everyone have some ideas about 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] tristaZero merged pull request #10052: Complete Oracle drop table partition grammar

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


   


-- 
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 #10052: Complete Oracle drop table partition grammar

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


   @ tristaZero LGTM


-- 
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] Liangda-w commented on a change in pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on a change in pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#discussion_r616555502



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -106,7 +106,7 @@ oidClause
     ;
 
 oidIndexClause
-    : OIDINDEX indexName? LP_ (physicalAttributesClause | TABLESPACE tablespaceName)+ RP_
+    : OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_
     ;

Review comment:
       Yes, that's much more readable😄 Thanks!




-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (DOT_ (partitionName | partitionForClauses))*

Review comment:
       @Liangda-w Hi, `DOT_` mean `.` but there should be `,` so should use `COMMA_` here




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

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

[GitHub] [shardingsphere] wgy8283335 commented on pull request #10052: Complete Oracle drop table partition grammar

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


   Hi @tristaZero , the CI fails, as calcite dependency has a problem. Shall we merge the pr?


-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   > Hi, I think since we can't let `physicalAttributesClause` contains an empty String, we can indicate the possibility of empty String at a higher level (in the clauses conatining `physicalAttributesClause`). So for example I changed `oidIndexClause` to
   > 
   > `OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_`
   > 
   > What do think of this way to solve this issue? @wgy8283335 @tristaZero
   
   I think this way could solve this issue. But I am not sure whether this way is good enough.
   What did you do when you have a similar problem in writing the MySQL rule? @tristaZero 


-- 
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] Liangda-w commented on a change in pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on a change in pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#discussion_r611682406



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (DOT_ (partitionName | partitionForClauses))*

Review comment:
       @zhujunxxxxx yes, you're right! Thank you for pointing it out😄 




-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   > Relates #9694 .
   > 
   > 
   > 
   > Changes proposed in this pull request:
   > 
   > - Complete `drop_table_partition` of [ALTER TABLE](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/ALTER-TABLE.html#GUID-552E7373-BF93-477D-9DA3-B2C9386F2877) and add corresponding test cases
   > 
   > 
   > 
   > @wgy8283335 
   > 
   > 
   
   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.

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

[GitHub] [shardingsphere] wgy8283335 commented on a change in pull request #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -106,7 +106,7 @@ oidClause
     ;
 
 oidIndexClause
-    : OIDINDEX indexName? LP_ (physicalAttributesClause | TABLESPACE tablespaceName)+ RP_
+    : OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)+ | ) RP_
     ;

Review comment:
       How about like this?
   `OIDINDEX indexName? LP_ ((physicalAttributesClause | TABLESPACE tablespaceName)*) RP_`




-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   @zhujunxxxxx Wait for your review here. :)
   Hi @Liangda-w 
   Thanks for this PR, BTW your application of GSoC 2021 is approved by me.


-- 
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 pull request #10052: Complete Oracle drop table partition grammar

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


   @zhujunxxxxx Got it @wgy8283335 So How do you think this PR?


-- 
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] Liangda-w commented on pull request #10052: Complete Oracle drop table partition grammar

Posted by GitBox <gi...@apache.org>.
Liangda-w commented on pull request #10052:
URL: https://github.com/apache/shardingsphere/pull/10052#issuecomment-820219938


   Hi @tristaZero, thank you for the information🥰  


-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       “(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storageClause)* ” is more suitable. @Liangda-w 




-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       @Liangda-w  “(PCTFREE NUMBER_ | PCTUSED NUMBER_ | INITRANS NUMBER_ | storage clause)* ” is more suitable, but if we change physicalAttributesClause like this, it will happen as bellow:
   "rule oidIndexClause contains a closure with at least one alternative that can match an empty string".
   @tristaZero how could we resolve this problem?




-- 
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 #10052: Complete Oracle drop table partition grammar

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)
+    (COMMA_ indexName LP_ (updateIndexPartition | updateIndexSubpartition) RP_)* RP_)?
+    ;
+
+updateIndexPartition
+    : indexPartitionDesc indexSubpartitionClause?
+    (COMMA_ indexPartitionDesc indexSubpartitionClause?)*
+    ;
+
+indexPartitionDesc
+    : PARTITION
+    (partitionName
+    ((segmentAttributesClause | indexCompression) | PARAMETERS LP_ SQ_ odciParameters SQ_ RP_ )?

Review comment:
       The rule is not correct.
   ![image](https://user-images.githubusercontent.com/22066046/114637233-6a5aec00-9cfb-11eb-8438-22e27853f1bc.png)
   

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -941,9 +945,51 @@ addListPartitionClause
     ;
 
 dropTablePartition
-    : DROP partitionExtendedNames
+    : DROP partitionExtendedNames (updateIndexClauses parallelClause?)?
     ;
 
 partitionExtendedNames
-    : (PARTITION | PARTITIONS) partition
+    : (PARTITION | PARTITIONS) (partitionName | partitionForClauses) (COMMA_ (partitionName | partitionForClauses))*
+    ;
+
+partitionForClauses
+    : FOR LP_ partitionKeyValue (COMMA_ partitionKeyValue)* RP_
+    ;
+
+updateIndexClauses
+    : updateGlobalIndexClause | updateAllIndexesClause
+    ;
+
+updateGlobalIndexClause
+    : (UPDATE | INVALIDATE) GLOBAL INDEXES
+    ;
+
+updateAllIndexesClause
+    : UPDATE INDEXES
+    (LP_ indexName LP_ (updateIndexPartition | updateIndexSubpartition)

Review comment:
       The rule has the wrong number of RP_.




-- 
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 #10052: Complete Oracle drop table partition grammar

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


   It looks good. It's time to merge.


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