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/01/26 17:07:16 UTC

[GitHub] [shardingsphere] ThanoshanMV opened a new pull request #9178: Add SQL definition for `COMMIT` of Oracle database

ThanoshanMV opened a new pull request #9178:
URL: https://github.com/apache/shardingsphere/pull/9178


   #6480 
   
   Hi @tristaZero, I've added SQL definition for `COMMIT`
   
   Changes proposed in this pull request:
   - add SQL definition for `COMMIT` 
   - add tests for `COMMIT`
   - remove unnecessary parenthesis in `rollback` and `savepointClause` 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.

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

[GitHub] [shardingsphere] ThanoshanMV commented on a change in pull request #9178: Add SQL definition for `COMMIT` of Oracle database

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/TCLStatement.g4
##########
@@ -24,15 +24,27 @@ setTransaction
     ;
 
 commit
-    : COMMIT
+    : COMMIT WORK? (commentClause? | writeClause? | forceClause)? 

Review comment:
       Hi @wgy8283335, 
   
   Since there is a vertical bar between `commentClause` and `writeClause`, I specified it as `|`. 
   
   Please let me know what are the changed I need to make.
   
   ![commit](https://user-images.githubusercontent.com/48581379/106016447-7ca69000-60e5-11eb-8a95-10e65515e0e5.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] ThanoshanMV commented on pull request #9178: Add SQL definition for `COMMIT` of Oracle database

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


   Sorry @tristaZero, @wgy8283335. I forgot to add the relevant documentation. Here is the `COMMIT` [documentation](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/COMMIT.html#GUID-6CD5C9A7-54B9-4FA2-BA3C-D6B4492B9EE2).  


----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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


   Hi @ThanoshanMV ,
   
   This PR looks clear. BTW, could you provide the relevant doc links for later PRs to locate the SQL definitions quickly?
   Thanks.
   
    @wgy8283335 Could you give a look at this one?
   


----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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


   Hi @wgy8283335 ,
   
   It is a right time to merge it into master branch?


----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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


   Hi @wgy8283335 ,
   
   Friendly ping 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] ThanoshanMV commented on a change in pull request #9178: Add SQL definition for `COMMIT` of Oracle database

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/TCLStatement.g4
##########
@@ -24,15 +24,27 @@ setTransaction
     ;
 
 commit
-    : COMMIT
+    : COMMIT WORK? (commentClause? | writeClause? | forceClause)? 

Review comment:
       Hi @wgy8283335, I've changed the definition. 




----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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


   > Hi @wgy8283335 ,
   > 
   > It is a right time to merge it into master branch?
   
   yes


----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/TCLStatement.g4
##########
@@ -24,15 +24,27 @@ setTransaction
     ;
 
 commit
-    : COMMIT
+    : COMMIT WORK? (commentClause? | writeClause? | forceClause)? 

Review comment:
       I think it should be changed to "((commentClause? writeClause?) | forceClause)?".




----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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


   


----------------------------------------------------------------
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 #9178: Add SQL definition for `COMMIT` of Oracle database

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



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/TCLStatement.g4
##########
@@ -24,15 +24,27 @@ setTransaction
     ;
 
 commit
-    : COMMIT
+    : COMMIT WORK? (commentClause? | writeClause? | forceClause)? 

Review comment:
       According to the document. Should it like "((commentClause? writeClause?) | forceClause)?", right?




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