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 2020/02/05 11:33:24 UTC
[GitHub] [incubator-shardingsphere] plazmdk opened a new pull request #4173:
Call setWasNull in GroupByStreamMergedResult
plazmdk opened a new pull request #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173
Changes proposed in this pull request:
- Set wasNull field which had wrong value when doing a LEFT JOIN FETCH via Hibernate resulting in trying getting an Entity with ID 0.
See demo application
[demoapp.zip](https://github.com/apache/incubator-shardingsphere/files/4159223/demoapp.zip)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] plazmdk commented on issue #4173: Call
setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
plazmdk commented on issue #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#issuecomment-582804090
I believe the code convensions are followed now - and another pr have been made for master
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] plazmdk edited a comment on issue #4173:
Call setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
plazmdk edited a comment on issue #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#issuecomment-582804090
I believe the code convensions are followed now - and another pr have been made for master
Unless I should have named `object` -> `result` ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 commented on issue #4173: Call
setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on issue #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#issuecomment-582791411
@plazmdk You are right. GroupByStreamMergedResult overwrite the getValue() method but miss set the wasNull value.
Can you fix the code conventions above in ShardingSphere(https://shardingsphere.apache.org/community/en/contribute/code-conduct/)?
I thinks this pr is also applicated to the master branch. Can you submit another more pr to master?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 commented on a change in pull
request #4173: Call setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on a change in pull request #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#discussion_r375692251
##########
File path: sharding-core/sharding-core-merge/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java
##########
@@ -129,11 +129,15 @@ private void setAggregationValueToCurrentRow(final Map<AggregationProjection, Ag
@Override
public Object getValue(final int columnIndex, final Class<?> type) {
- return currentRow.get(columnIndex - 1);
+ Object o = currentRow.get(columnIndex - 1);
Review comment:
o -> object
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 commented on issue #4173: Call
setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on issue #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#issuecomment-582806703
You are right, the return value should named result.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 merged pull request #4173:
Call setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
tuohai666 merged pull request #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-shardingsphere] tuohai666 commented on a change in pull
request #4173: Call setWasNull in GroupByStreamMergedResult
Posted by GitBox <gi...@apache.org>.
tuohai666 commented on a change in pull request #4173: Call setWasNull in GroupByStreamMergedResult
URL: https://github.com/apache/incubator-shardingsphere/pull/4173#discussion_r375692472
##########
File path: sharding-core/sharding-core-merge/src/main/java/org/apache/shardingsphere/sharding/merge/dql/groupby/GroupByStreamMergedResult.java
##########
@@ -129,11 +129,15 @@ private void setAggregationValueToCurrentRow(final Map<AggregationProjection, Ag
@Override
public Object getValue(final int columnIndex, final Class<?> type) {
- return currentRow.get(columnIndex - 1);
+ Object o = currentRow.get(columnIndex - 1);
+ setWasNull(o == null);
Review comment:
o == null -> null == object
----------------------------------------------------------------
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
With regards,
Apache Git Services