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/06/06 11:10:49 UTC

[GitHub] [shardingsphere] SivaTharun opened a new pull request #10676: 10374 - code refactoring for process report feature

SivaTharun opened a new pull request #10676:
URL: https://github.com/apache/shardingsphere/pull/10676


   
   Fixes #10374 .
   
   Changes proposed in this pull request:
   - replaced the if condition with ternary operator 
   - used collection utils and objects class for null check, instead of comparison


-- 
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] SivaTharun commented on pull request #10676: 10374 - code refactoring for process report feature

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






-- 
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 #10676: 10374 - code refactoring for process report feature

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


   Thanks, @SivaTharun.


-- 
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 #10676: 10374 - code refactoring for process report feature

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


   


-- 
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 #10676: 10374 - code refactoring for process report feature

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



##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/execute/engine/raw/RawExecutor.java
##########
@@ -62,10 +63,8 @@
             // TODO Load query header for first query
             List<ExecuteResult> results = execute(executionGroupContext, (RawSQLExecutorCallback) null, callback);
             ExecuteProcessEngine.finish(executionGroupContext.getExecutionID());
-            if (null == results || results.isEmpty() || null == results.get(0)) {
-                return Collections.singleton(new UpdateResult(0, 0L));
-            }
-            return results;
+            return CollectionUtils.isEmpty(results) || Objects.isNull(results.get(0)) ? Collections
+                .singleton(new UpdateResult(0, 0L)) : results;

Review comment:
       Hi @SivaTharun ,
   Do you think `return null == results || results.isEmpty() || null == results.get(0) ? Collections.singleton(new UpdateResult(0, 0L)) : results`

##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/execute/engine/raw/RawExecutor.java
##########
@@ -62,10 +63,8 @@
             // TODO Load query header for first query
             List<ExecuteResult> results = execute(executionGroupContext, (RawSQLExecutorCallback) null, callback);
             ExecuteProcessEngine.finish(executionGroupContext.getExecutionID());
-            if (null == results || results.isEmpty() || null == results.get(0)) {
-                return Collections.singleton(new UpdateResult(0, 0L));
-            }
-            return results;
+            return CollectionUtils.isEmpty(results) || Objects.isNull(results.get(0)) ? Collections
+                .singleton(new UpdateResult(0, 0L)) : results;

Review comment:
       Hi @SivaTharun Thanks for your clarification which is fine with 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] SivaTharun removed a comment on pull request #10676: 10374 - code refactoring for process report feature

Posted by GitBox <gi...@apache.org>.
SivaTharun removed a comment on pull request #10676:
URL: https://github.com/apache/shardingsphere/pull/10676#issuecomment-860086495


   closing the PR, as mistakenly used git rebase instead of merge on 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] SivaTharun commented on a change in pull request #10676: 10374 - code refactoring for process report feature

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



##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/execute/engine/raw/RawExecutor.java
##########
@@ -62,10 +63,8 @@
             // TODO Load query header for first query
             List<ExecuteResult> results = execute(executionGroupContext, (RawSQLExecutorCallback) null, callback);
             ExecuteProcessEngine.finish(executionGroupContext.getExecutionID());
-            if (null == results || results.isEmpty() || null == results.get(0)) {
-                return Collections.singleton(new UpdateResult(0, 0L));
-            }
-            return results;
+            return CollectionUtils.isEmpty(results) || Objects.isNull(results.get(0)) ? Collections
+                .singleton(new UpdateResult(0, 0L)) : results;

Review comment:
       Hi @tristaZero,   `CollectionUtils.isEmpty` method inside apache common,  would check for `results == null || results.isEmpty()`. That essentially would cover both the checks.  i guess you don't want to have any dependency on apache commons library.
   
   Also, `Objects.isNull` is the java utility method of `Objects` class, which encompasses the change for `null == results.get(0)`. i thought of using it, resulting in cleaner looking code.
   
   
   




-- 
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 #10676: 10374 - code refactoring for process report feature

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


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10676?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 [#10676](https://codecov.io/gh/apache/shardingsphere/pull/10676?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4efab0) into [master](https://codecov.io/gh/apache/shardingsphere/commit/8ffcfe51dfc84875f3bb65d4e1b803b370ca8e8c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8ffcfe5) will **decrease** coverage by `0.02%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/10676/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/10676?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   #10676      +/-   ##
   ============================================
   - Coverage     65.62%   65.60%   -0.03%     
   - Complexity      691      693       +2     
   ============================================
     Files          1777     1779       +2     
     Lines         30822    30821       -1     
     Branches       5559     5560       +1     
   ============================================
   - Hits          20227    20219       -8     
   - Misses         8983     8990       +7     
     Partials       1612     1612              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/10676?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...a/executor/sql/execute/engine/raw/RawExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9leGVjdXRlL2VuZ2luZS9yYXcvUmF3RXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...sql/parser/core/standard/DistSQLParserFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL0Rpc3RTUUxQYXJzZXJGYWN0b3J5LmphdmE=) | | |
   | [...e/distsql/parser/core/standard/DistSQLVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL0Rpc3RTUUxWaXNpdG9yLmphdmE=) | | |
   | [...re/distsql/parser/core/standard/DistSQLParser.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL0Rpc3RTUUxQYXJzZXIuamF2YQ==) | | |
   | [...core/standard/StandardDistSQLStatementVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL1N0YW5kYXJkRGlzdFNRTFN0YXRlbWVudFZpc2l0b3IuamF2YQ==) | `86.55% <0.00%> (ø)` | |
   | [...ql/parser/core/standard/StandardDistSQLParser.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL1N0YW5kYXJkRGlzdFNRTFBhcnNlci5qYXZh) | `100.00% <0.00%> (ø)` | |
   | [...sql/parser/core/standard/StandardDistSQLLexer.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL1N0YW5kYXJkRGlzdFNRTExleGVyLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | [.../feature/FeatureTypedSQLStatementParserEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL2ZlYXR1cmUvRmVhdHVyZVR5cGVkU1FMU3RhdGVtZW50UGFyc2VyRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ore/standard/StandardSQLStatementParserEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9jb3JlL3N0YW5kYXJkL1N0YW5kYXJkU1FMU3RhdGVtZW50UGFyc2VyRW5naW5lLmphdmE=) | `66.66% <0.00%> (ø)` | |
   | [...stsql/parser/api/DistSQLStatementParserEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/10676/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-c2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXIvc2hhcmRpbmdzcGhlcmUtZGlzdHNxbC1wYXJzZXItZW5naW5lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kaXN0c3FsL3BhcnNlci9hcGkvRGlzdFNRTFN0YXRlbWVudFBhcnNlckVuZ2luZS5qYXZh) | `50.00% <0.00%> (+13.33%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/10676?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/10676?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 [8ffcfe5...d4efab0](https://codecov.io/gh/apache/shardingsphere/pull/10676?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] SivaTharun closed pull request #10676: 10374 - code refactoring for process report feature

Posted by GitBox <gi...@apache.org>.
SivaTharun closed pull request #10676:
URL: https://github.com/apache/shardingsphere/pull/10676


   


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