drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Nadeau" <jacques.dr...@gmail.com>
Subject Re: Review Request 25949: changes to support configuring text formats (field separator, record delimiter, quote character, header line, strip quotes)
Date Mon, 03 Nov 2014 04:35:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25949/#review59555
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/TextFormatPlugin.java
<https://reviews.apache.org/r/25949/#comment100827>

    Please review the JSON plans that Drill generates.  They use this extensively.  Your examples
are passthroughs to Hive and HBase config objects.  We can't control those.  This is why they
are inconsistent with the Drill style.



exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/25949/#comment100828>

    The comment above doesn't discuss anywhere why we use three different types to represent
the same value (byte, char and string).  Please update to be consistent.


- Jacques Nadeau


On Sept. 23, 2014, 7:53 p.m., Jim Scott wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25949/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 7:53 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Updates for handling text formatted files to address: https://issues.apache.org/jira/browse/DRILL-1440
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/pom.xml 81dbeff 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
76c4ace 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/TextFormatPlugin.java
b64a032 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
7b8761c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
31b1fbe 
>   exec/java-exec/src/test/java/org/apache/drill/exec/store/easy/text/TextFormatConfigTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25949/diff/
> 
> 
> Testing
> -------
> 
> There is a test class added to the serialization json work of the data formats including
maintaing backward compatibility as well as altering the field separator name.
> 
> Maven tests were run, with these two errors:
>   TestWriter.simpleCsv » org.apache.drill.exec.rpc.RpcException: Failure while running
fragment. null [479d0e71-2bad-4f4c-b3fc-8bffc98f788b]
>   TestWriter>BaseTestQuery.closeClient:125 » java.lang.IllegalStateException: Failure
while trying to close allocator: Child level allocators not closed.
> 
> I could not figure out the cause for these failures, nor could I properly debug why they
were occurring. The code in the patch should have no impact on those tests (although, I could
be wrong).
> 
> 
> Thanks,
> 
> Jim Scott
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message