----------------------------------------------------------- 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 > >
