You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by amansinha100 <gi...@git.apache.org> on 2016/05/20 05:22:43 UTC
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
GitHub user amansinha100 opened a pull request:
https://github.com/apache/drill/pull/504
DRILL-4679: When convert() functions are present, ensure that Project\u2026
\u2026RecordBatch produces a schema even for empty result set.
Add unit tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/amansinha100/incubator-drill DRILL-4679
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/504.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 #504
----
commit 8342ddd3b2eb664bd67ed6f375bba3c22ec0b9a8
Author: Aman Sinha <as...@maprtech.com>
Date: 2016-05-17T21:35:06Z
DRILL-4679: When convert() functions are present, ensure that ProjectRecordBatch produces a schema even for empty result set.
Add unit tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the pull request:
https://github.com/apache/drill/pull/504#issuecomment-220645472
LGTM.
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/504#discussion_r64091394
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
@@ -146,6 +159,27 @@ protected IterOutcome doWork() {
if (next == IterOutcome.OUT_OF_MEMORY) {
outOfMemory = true;
return next;
+ } else if (next == IterOutcome.NONE) {
+ // since this is first batch and we already got a NONE, need to set up the schema
+
+ //allocate vv in the allocationVectors.
+ for (final ValueVector v : this.allocationVectors) {
--- End diff --
The doAlloc() was calling incoming.getRecordCount() which would fail for empty batches, so I did not use it but I can modify doAlloc to take the count parameter and have everyone call the modified version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/504#discussion_r64101122
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
@@ -136,6 +145,10 @@ public VectorContainer getOutgoingContainer() {
@Override
protected IterOutcome doWork() {
+ if (wasNone) {
+ return IterOutcome.NONE;
+ }
+
int incomingRecordCount = incoming.getRecordCount();
if (first && incomingRecordCount == 0) {
--- End diff --
Actually, if the first batch was non-empty, the new changes wouldn't apply because of the following check:
if (first && incomingRecordCount == 0) { ... }
Then if the next incoming batch is empty, it should continue to work since we have already produced the schema from the first batch. On the other hand if the first batch is empty and we see a NONE iterator outcome, we want to make sure that a schema is produced but at the same time not call next() since a NONE outcome has already been seen.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the pull request:
https://github.com/apache/drill/pull/504#issuecomment-220525522
@jinfengni could you pls review ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/504#discussion_r64064412
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
@@ -136,6 +145,10 @@ public VectorContainer getOutgoingContainer() {
@Override
protected IterOutcome doWork() {
+ if (wasNone) {
+ return IterOutcome.NONE;
+ }
+
int incomingRecordCount = incoming.getRecordCount();
if (first && incomingRecordCount == 0) {
--- End diff --
The new logic will handle the case for Project's first outgoing batch. Not sure whether Drill works properly after the first batch getting data and building the schema, but the next incoming batch contains empty result. We may treat as a separate issue for further investigation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on a diff in the pull request:
https://github.com/apache/drill/pull/504#discussion_r64063077
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
@@ -146,6 +159,27 @@ protected IterOutcome doWork() {
if (next == IterOutcome.OUT_OF_MEMORY) {
outOfMemory = true;
return next;
+ } else if (next == IterOutcome.NONE) {
+ // since this is first batch and we already got a NONE, need to set up the schema
+
+ //allocate vv in the allocationVectors.
+ for (final ValueVector v : this.allocationVectors) {
--- End diff --
the allocation logic may use existing method doAlloc().
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 closed the pull request at:
https://github.com/apache/drill/pull/504
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on a diff in the pull request:
https://github.com/apache/drill/pull/504#discussion_r64103528
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
@@ -146,6 +159,27 @@ protected IterOutcome doWork() {
if (next == IterOutcome.OUT_OF_MEMORY) {
outOfMemory = true;
return next;
+ } else if (next == IterOutcome.NONE) {
+ // since this is first batch and we already got a NONE, need to set up the schema
+
+ //allocate vv in the allocationVectors.
+ for (final ValueVector v : this.allocationVectors) {
--- End diff --
Updated PR to address review comment regarding doAlloc().
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4679: When convert() functions are prese...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the pull request:
https://github.com/apache/drill/pull/504#issuecomment-220730151
Committed in 3d92d2829.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---