[GitHub] [shardingsphere] tristaZero commented on pull request #9692: Oracle merge statement test cases for merge update clause

[GitBox <gi...@apache.org>]

tristaZero commented on pull request #9692:
URL: https://github.com/apache/shardingsphere/pull/9692#issuecomment-801800142


   Hi,
   
   First, `mergeSetAssignmentsClause` is OK for me, since it is like a definition with a group of `mergeSetAssignment`. Therefore a `merge` main definition will contain some of the keywords and some of the child group definitions, and each group definition is made up of its child definitions. 
   On the other hand, you can still put all specific definitions in the main clause, which does not have any group definitions.
   
   Which one is our best choice? I suppose we need to add another factor, i.e., `SQLStatement`. Suppose we want to visit out some specific `statement`, it is necessary to estimate which type of SQL definition of `g4` file is more reasonable and straightforward for visitors.
   
   That's my opinion and hope it can give you some hints for the following issues.
   
   Second, actually, a more fitting member putting into `MergeStatement`, I guess, is the `UpdateStatement` containing `setAssignment` and `where`. The reason is that `OracleUpdateStatement` has existed and owned the entire and complete definition about `update set where`. 
   But that will make it more challenging to do unit tests. I suggest you can have a try, and if that frustrates you too much, you can put it aside. Yep, it is optional. :)
   
   @ThanoshanMV 
   Hope you can succeed in GSoC 2021 and learn more from this community.  Also @wgy8283335 and I will provide our mentoring as much as we can.


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