----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23799/#review49091 -----------------------------------------------------------
In general this feels a bit awkward. I think better CSV/TSV support is a good idea but "quotedCsv" seems misleading as the old "csv" and "tsv" now quote as well if the separator is contained in the column value. beeline/src/java/org/apache/hive/beeline/BeeLine.java <https://reviews.apache.org/r/23799/#comment85924> Missing space here and next line beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85920> remove "this" and call to "getSeparator", can just be "separator". beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85915> Can be converted to a variable arity function (e.g. String... vals) beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85916> Rename to writer? beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85917> Same as above: Can be converted to variable arity method beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85918> ...variable arity beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85919> Remove this and probably replace the call to isSingleQuoted with just "singleQuoted", no need to go through a simple getter beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85923> Missing spaces around the else beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85922> I'd either remove the getter and setters entirely or they need changing so that things are properly updated when separator/singleQuoted/csvPreference are changed. Example: Someone passes in a CsvPreference with a different separator than the one set in here. I think part of this patch needs to be the removal of all these simple (getter/)setters. If you don't want that then you need some verification logic that things make sense. beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java <https://reviews.apache.org/r/23799/#comment85921> This is not a getter but a setter. - Lars Francke On July 30, 2014, 8:30 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23799/ > ----------------------------------------------------------- > > (Updated July 30, 2014, 8:30 a.m.) > > > Review request for hive. > > > Bugs: HIVE-7390 > https://issues.apache.org/jira/browse/HIVE-7390 > > > Repository: hive-git > > > Description > ------- > > HIVE-7390: refactor csv output format with in RFC mode and add one more > option to support formatting as the csv format in hive cli > > > Diffs > ----- > > beeline/pom.xml 6ec1d1aff3f35c097aa6054aae84faf2d63854f1 > beeline/src/java/org/apache/hive/beeline/BeeLine.java > 528a98e29c23421f9352bdf7c5edd3a9fae0e3ea > beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java > 7853c3f38f3c3fb9ae0b9939c714f1dc940ba053 > beeline/src/main/resources/BeeLine.properties > 390d062b8dc52dfa790c7351f3db44c1e0dd7e37 > > itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java > 8888bd97aff5959fd9040fc0f0a1f6b782f2aa6f > pom.xml b5a5697e6a3b689c2b244ba0338be541261eaa3d > > Diff: https://reviews.apache.org/r/23799/diff/ > > > Testing > ------- > > > Thanks, > > cheng xu > >
