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/05/06 21:28:04 UTC

[GitHub] [shardingsphere] Liangda-w commented on a change in pull request #10253: Add SQL Definition for `ALTER SYSTEM` of Oracle Database

Liangda-w commented on a change in pull request #10253:
URL: https://github.com/apache/shardingsphere/pull/10253#discussion_r627755744



##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -1421,3 +1421,258 @@ leadCdbUriClause
 propertyClause
     : PROPERTY (SET | REMOVE) DEFAULT_CREDENTIAL EQ_ qualifiedCredentialName
     ;
+
+alterSystem
+    : ALTER SYSTEM alterSystemOption
+    ;
+
+alterSystemOption
+    : archiveLogClause
+    | checkpointClause
+    | checkDatafilesClause
+    | distributedRecovClauses
+    | flushClause
+    | endSessionClauses
+    | alterSystemSwitchLogfileClause
+    | suspendResumeClause
+    | quiesceClauses
+    | rollingMigrationClauses
+    | rollingPatchClauses
+    | alterSystemSecuriyClauses

Review comment:
       Minor Typo -> `alterSystemSecurityClauses`

##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-oracle/src/main/antlr4/imports/oracle/DDLStatement.g4
##########
@@ -1421,3 +1421,258 @@ leadCdbUriClause
 propertyClause
     : PROPERTY (SET | REMOVE) DEFAULT_CREDENTIAL EQ_ qualifiedCredentialName
     ;
+
+alterSystem
+    : ALTER SYSTEM alterSystemOption
+    ;
+
+alterSystemOption
+    : archiveLogClause
+    | checkpointClause
+    | checkDatafilesClause
+    | distributedRecovClauses
+    | flushClause
+    | endSessionClauses
+    | alterSystemSwitchLogfileClause
+    | suspendResumeClause
+    | quiesceClauses
+    | rollingMigrationClauses
+    | rollingPatchClauses
+    | alterSystemSecuriyClauses
+    | affinityClauses
+    | shutdownDispatcherClause
+    | registerClause
+    | setClause
+    | resetClause
+    | relocateClientClause
+    | cancelSqlClause
+    | flushPasswordfileMetadataCacheClause
+    ;
+
+archiveLogClause
+    : ARCHIVE LOG instanceClause? (sequenceClause | changeClause | currentClause | groupClause | logfileClause | nextClause | allClause) toLocationClause?
+    ;
+
+checkpointClause
+    : CHECKPOINT (GLOBAL | LOCAL)?
+    ;
+
+checkDatafilesClause
+    : CHECK DATAFILES (GLOBAL | LOCAL)?
+    ;
+
+distributedRecovClauses
+    : (ENABLE | DISABLE) DISTRIBUTED RECOVERY
+    ;
+
+flushClause
+    : FLUSH flushClauseOption
+    ;
+
+endSessionClauses
+    : (disconnectSessionClause | killSessionClause) (IMMEDIATE | NOREPLY)?
+    ;
+
+alterSystemSwitchLogfileClause
+    : SWITCH LOGFILE
+    ;
+
+suspendResumeClause
+    : SUSPEND | RESUME
+    ;
+
+quiesceClauses
+    : QUIESCE RESTRICTED | UNQUIESCE
+    ;
+
+rollingMigrationClauses
+    : startRollingMigrationClause | stopRollingMigrationClause
+    ;
+
+rollingPatchClauses
+    : startRollingPatchClause | stopRollingPatchClause
+    ;
+
+alterSystemSecuriyClauses
+    : restrictedSessionClause | setEncryptionWalletOpenClause | setEncryptionWalletCloseClause | setEncryptionKeyClause
+    ;
+
+affinityClauses
+    : enableAffinityClause | disableAffinityClause
+    ;
+
+shutdownDispatcherClause
+    : SHUTDOWN IMMEDIATE? dispatcherName
+    ;
+
+registerClause
+    : REGISTER
+    ;
+
+setClause
+    : SET alterSystemSetClause+
+    ;
+
+resetClause
+    : RESET alterSystemResetClause+
+    ;
+
+relocateClientClause
+    : RELOCATE CLIENT clientId
+    ;
+
+cancelSqlClause
+    : CANCEL SQL SQ_ sessionId serialNumber (AT_ instanceId)? sqlId? SQ_
+    ;
+
+flushPasswordfileMetadataCacheClause
+    : FLUSH PASSWORDFILE_METADATA_CACHE
+    ;
+
+instanceClause
+    : INSTANCE instanceName
+    ;
+
+sequenceClause
+    : SEQUENCE numberLiterals
+    ;
+
+changeClause
+    : CHANGE numberLiterals
+    ;
+
+currentClause
+    : CURRENT NOSWITCH?
+    ;
+
+groupClause
+    : GROUP numberLiterals
+    ;
+
+logfileClause
+    : LOGFILE logFileName (USING BACKUP CONTROLFILE)?
+    ;
+
+nextClause
+    : NEXT
+    ;

Review comment:
       I see you're breaking the rules in many small pieces, which looks really organized 👍 
   However sometimes such clauses may not be necessary (these will be rarely reused), it will only make the list expand too much and may also confuse later contributor. And many of them could be written in a more concise way (and remaining organized).
   
   So just my little opinion and 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