[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[
https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16247941#comment-16247941
]
ASF GitHub Bot commented on DRILL-5936:
---
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/1025
+1 with a minor comment. In the commit message and JIRA it would be
better to say 'code inspection' instead of code review which may be
interpreted to mean the normal code review process.
> Refactor MergingRecordBatch based on code review
>
>
> Key: DRILL-5936
> URL: https://issues.apache.org/jira/browse/DRILL-5936
> Project: Apache Drill
> Issue Type: Improvement
> Components: Tools, Build & Test
>Reporter: Vlad Rozov
>Assignee: Vlad Rozov
>Priority: Minor
>
> * Reorganize code to remove unnecessary {{pqueue.peek()}}
> * Reuse Node
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[
https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16247933#comment-16247933
]
ASF GitHub Bot commented on DRILL-5936:
---
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1025#discussion_r150310556
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
---
@@ -177,11 +177,11 @@ public IterOutcome innerNext() {
}
boolean schemaChanged = false;
-if (prevBatchWasFull) {
+if (!prevBatchNotFull) {
--- End diff --
The double negative makes it somewhat confusing. Perhaps rename the
variable to 'prevBatchHasSpace' .
> Refactor MergingRecordBatch based on code review
>
>
> Key: DRILL-5936
> URL: https://issues.apache.org/jira/browse/DRILL-5936
> Project: Apache Drill
> Issue Type: Improvement
> Components: Tools, Build & Test
>Reporter: Vlad Rozov
>Assignee: Vlad Rozov
>Priority: Minor
>
> * Reorganize code to remove unnecessary {{pqueue.peek()}}
> * Reuse Node
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[
https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246876#comment-16246876
]
Vlad Rozov commented on DRILL-5936:
---
[~amansinha100] It is based on my code walkthrough completed as part of
exchange operator analysis. The PR is mostly self-explanatory and the goal is
to address two deficiencies mentioned in the JIRA description.
> Refactor MergingRecordBatch based on code review
>
>
> Key: DRILL-5936
> URL: https://issues.apache.org/jira/browse/DRILL-5936
> Project: Apache Drill
> Issue Type: Improvement
> Components: Tools, Build & Test
>Reporter: Vlad Rozov
>Assignee: Vlad Rozov
>Priority: Minor
>
> * Reorganize code to remove unnecessary {{pqueue.peek()}}
> * Reuse Node
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[
https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246872#comment-16246872
]
Aman Sinha commented on DRILL-5936:
---
[~vrozov] the title says 'based on code review'..which code review are you
referring to ? can you point me to the other JIRA or PR ? thanks.
> Refactor MergingRecordBatch based on code review
>
>
> Key: DRILL-5936
> URL: https://issues.apache.org/jira/browse/DRILL-5936
> Project: Apache Drill
> Issue Type: Improvement
> Components: Tools, Build & Test
>Reporter: Vlad Rozov
>Assignee: Vlad Rozov
>Priority: Minor
>
> * Reorganize code to remove unnecessary {{pqueue.peek()}}
> * Reuse Node
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[ https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16246828#comment-16246828 ] ASF GitHub Bot commented on DRILL-5936: --- Github user priteshm commented on the issue: https://github.com/apache/drill/pull/1025 @amansinha100 can you review this change? > Refactor MergingRecordBatch based on code review > > > Key: DRILL-5936 > URL: https://issues.apache.org/jira/browse/DRILL-5936 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Reporter: Vlad Rozov >Assignee: Vlad Rozov >Priority: Minor > -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5936) Refactor MergingRecordBatch based on code review
[ https://issues.apache.org/jira/browse/DRILL-5936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16241417#comment-16241417 ] ASF GitHub Bot commented on DRILL-5936: --- GitHub user vrozov opened a pull request: https://github.com/apache/drill/pull/1025 DRILL-5936: Refactor MergingRecordBatch based on code review You can merge this pull request into a Git repository by running: $ git pull https://github.com/vrozov/drill DRILL-5936 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1025.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1025 commit 82c04413d63733a8aad58e4b328af97e8f2ad6d5 Author: Vlad Rozov Date: 2017-11-07T01:55:56Z DRILL-5936: Refactor MergingRecordBatch based on code review > Refactor MergingRecordBatch based on code review > > > Key: DRILL-5936 > URL: https://issues.apache.org/jira/browse/DRILL-5936 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Reporter: Vlad Rozov >Assignee: Vlad Rozov >Priority: Minor > -- This message was sent by Atlassian JIRA (v6.4.14#64029)
