[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/706


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[arina-ielchiieva]

Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/706#discussion_r95005998
  
--- Diff: exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
---
@@ -17,8 +17,14 @@
  */
 <@pp.dropOutputFile />
 
+<#macro reassignHolder>
+previous.buffer = buf.reallocIfNeeded(length);
+previous.buffer.setBytes(0, in.buffer, in.start, length);
+previous.end = length;
+
 
-<@pp.changeOutputFile 
name="/org/apache/drill/exec/expr/fn/impl/GNewValueFunctions.java" />
+
+<@pp.changeOutputFile 
name="/org/apache/drill/exeBasec/expr/fn/impl/GNewValueFunctions.java" />
--- End diff --

Fixed. Thanks for noticing. Not sure how it has leaked out.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[amansinha100]

Github user amansinha100 commented on a diff in the pull request:

https://github.com/apache/drill/pull/706#discussion_r94995057
  
--- Diff: exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
---
@@ -17,8 +17,14 @@
  */
 <@pp.dropOutputFile />
 
+<#macro reassignHolder>
+previous.buffer = buf.reallocIfNeeded(length);
+previous.buffer.setBytes(0, in.buffer, in.start, length);
+previous.end = length;
+
 
-<@pp.changeOutputFile 
name="/org/apache/drill/exec/expr/fn/impl/GNewValueFunctions.java" />
+
+<@pp.changeOutputFile 
name="/org/apache/drill/exeBasec/expr/fn/impl/GNewValueFunctions.java" />
--- End diff --

Is this the correct output destination ?  I get an error in my IDE:
The declared package "org.apache.drill.exec.expr.fn.impl" does not match 
the expected package "org.apache.drill.exeBasec.expr.fn.impl"  
GNewValueFunctions.java 
/drill-java-exec/target/generated-sources/org/apache/drill/exeBasec/expr/fn/impl



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[arina-ielchiieva]

Github user arina-ielchiieva commented on a diff in the pull request:

https://github.com/apache/drill/pull/706#discussion_r94571676
  
--- Diff: exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
---
@@ -17,6 +17,12 @@
  */
 <@pp.dropOutputFile />
 
+<#macro reassignHolder>
+previous.buffer = buf.reallocIfNeeded(length);
+previous.buffer.setBytes(0, in.buffer, in.start, in.end - 
in.start);
--- End diff --

Done.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[chunhui-shi]

Github user chunhui-shi commented on a diff in the pull request:

https://github.com/apache/drill/pull/706#discussion_r94492239
  
--- Diff: exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
---
@@ -17,6 +17,12 @@
  */
 <@pp.dropOutputFile />
 
+<#macro reassignHolder>
+previous.buffer = buf.reallocIfNeeded(length);
+previous.buffer.setBytes(0, in.buffer, in.start, in.end - 
in.start);
--- End diff --

length=(in.end - in.start) here. I would suggest always use length.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #706: DRILL-5039: NPE - CTAS PARTITION BY (

[arina-ielchiieva]

GitHub user arina-ielchiieva opened a pull request:

https://github.com/apache/drill/pull/706

DRILL-5039: NPE - CTAS PARTITION BY ()

1. Moved varchar `newPartitionValue` functions to `NewValueFunctions` 
template:
a. took advantage of code generation for varchar `newPartitionValue` 
functions;
b. all `newPartitionValue` functions will be in one file.
2. Fixed logic for reassigning nullable varchar holders during 
`newPartitionValue` function execution (previously resulted in NPE if varchar 
partition column contained nulls).
3. Added `alltypes_optional.parquet` (each type contains at least one null 
value) in test resources and updated `alltypes_required.parquet` to have the 
same types as in `alltypes_optional.parquet`.
4. Added new unit test `testPartitionByForAllTypes` to cover all types and 
cases (with and without nulls) during CTAS with partitioning.
5. Updated `minMaxEmptyNonNullableInput` test to reflect changes in 
`alltypes_required.parquet`.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/arina-ielchiieva/drill DRILL-5039

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/706.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 #706


commit 3521339a8a39dc8cef5b769ee8abce00e6a88ba7
Author: Arina Ielchiieva 
Date:   2016-12-23T17:51:38Z

DRILL-5039: NPE - CTAS PARTITION BY ()




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---