Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Jianguo Tian]

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

(Updated Oct. 21, 2016, 1:10 a.m.)


Review request for hive and cheng xu.


Bugs: HIVE-14679
https://issues.apache.org/jira/browse/HIVE-14679


Repository: hive-git


Description (updated)
---

1. Enabled double quoting by default for csv2, tsv2 and dsv.
2. Disabling quoting using beeline argument --disablequoting.
3. No quote character at all when disable quoting.


Diffs
-

  beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
  beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
  beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
  beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
66d9fd0 
  beeline/src/main/resources/BeeLine.properties ad79c01 
  beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52981/diff/


Testing
---

Ran JUnit test to validate csv2,tsv2,dsv,csv and tsv output format with 
enabling and disabling double quotes.


Thanks,

Jianguo Tian

Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Jianguo Tian]

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

(Updated Oct. 21, 2016, 12:48 a.m.)


Review request for hive and cheng xu.


Bugs: HIVE-14679
https://issues.apache.org/jira/browse/HIVE-14679


Repository: hive-git


Description (updated)
---

1. Enabled double quoting by default for csv2, tsv2 and dsv.
2. Disabling quoting using beeline argument --outputformat.
3. No quote character at all when disable quoting.


Diffs (updated)
-

  beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
  beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
  beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
  beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
66d9fd0 
  beeline/src/main/resources/BeeLine.properties ad79c01 
  beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52981/diff/


Testing (updated)
---

Ran JUnit test to validate csv2,tsv2,dsv,csv and tsv output format with 
enabling and disabling double quotes.


Thanks,

Jianguo Tian

Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[cheng xu]

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




beeline/src/java/org/apache/hive/beeline/Commands.java (line 650)


No space needed after set.



beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java (line 
105)


Why default value is true?



beeline/src/main/resources/BeeLine.properties (line 190)


If this option is not applicable for all outputformat, can you please make 
it clear what kind of outputformat the options can be applied?



beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 
48)


Please add a test like ""value""



beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 
69)


AFAIK, this option doesn't affect csv/tsv/dsv. So you can see no change 
after enabling it. Please remove them if my understanding is correct.



beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 
115)


I am not sure why default value is true. If the default value is true, no 
need to set it again here. And we should test the case when quoting is disabled.



beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 
182)


hint: is that possible to reuse the code in TestBufferedRows?



beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 
217)


Remove this line please.


- cheng xu


On Oct. 19, 2016, 7:55 p.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52981/
> ---
> 
> (Updated Oct. 19, 2016, 7:55 p.m.)
> 
> 
> Review request for hive and cheng xu.
> 
> 
> Bugs: HIVE-14679
> https://issues.apache.org/jira/browse/HIVE-14679
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 1. Quoting should be enabled by default for csv2, tsv2 and dsv.
> 2. Disabling quoting should be possible using a beeline argument.
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
>   beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
>   beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
> 66d9fd0 
>   beeline/src/main/resources/BeeLine.properties ad79c01 
>   beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52981/diff/
> 
> 
> Testing
> ---
> 
> Test csv2,tsv2,dsv,csv and tsv output format with enabling double quotes.
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Jianguo Tian]

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

(Updated Oct. 19, 2016, 11:55 a.m.)


Review request for hive and cheng xu.


Bugs: HIVE-14679
https://issues.apache.org/jira/browse/HIVE-14679


Repository: hive-git


Description
---

1. Quoting should be enabled by default for csv2, tsv2 and dsv.
2. Disabling quoting should be possible using a beeline argument.


Diffs (updated)
-

  beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
  beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
  beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
  beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
66d9fd0 
  beeline/src/main/resources/BeeLine.properties ad79c01 
  beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52981/diff/


Testing (updated)
---

Test csv2,tsv2,dsv,csv and tsv output format with enabling double quotes.


Thanks,

Jianguo Tian

Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Jianguo Tian]

> On Oct. 18, 2016, 9:30 a.m., Barna Zsombor Klara wrote:
> > Thank you for the patch Jianguo Tian. LGTM, pending tests.
> > Hope you don't mind that I had a look as well.

Really appreciate for your review. I'll fix these problems and add tests.


- Jianguo


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


On Oct. 18, 2016, 8:11 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52981/
> ---
> 
> (Updated Oct. 18, 2016, 8:11 a.m.)
> 
> 
> Review request for hive and cheng xu.
> 
> 
> Bugs: HIVE-14679
> https://issues.apache.org/jira/browse/HIVE-14679
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 1. Quoting should be enabled by default for csv2, tsv2 and dsv.
> 2. Disabling quoting should be possible using a beeline argument.
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
>   beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
>   beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
> 66d9fd0 
>   beeline/src/main/resources/BeeLine.properties ad79c01 
> 
> Diff: https://reviews.apache.org/r/52981/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Barna Zsombor Klara]

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



Thank you for the patch Jianguo Tian. LGTM, pending tests.
Hope you don't mind that I had a look as well.


beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java (line 559)


nit: extra spaces



beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java (line 567)


nit: extra spaces



beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java (line 
104)


I think we can remove this line, not just comment it out. (Same for line 
36).



beeline/src/main/resources/BeeLine.properties (line 190)


Missing verb in the sentence.
will not be visible around a value? or will not be present around a value?


- Barna Zsombor Klara


On Oct. 18, 2016, 8:11 a.m., Jianguo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52981/
> ---
> 
> (Updated Oct. 18, 2016, 8:11 a.m.)
> 
> 
> Review request for hive and cheng xu.
> 
> 
> Bugs: HIVE-14679
> https://issues.apache.org/jira/browse/HIVE-14679
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 1. Quoting should be enabled by default for csv2, tsv2 and dsv.
> 2. Disabling quoting should be possible using a beeline argument.
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
>   beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
>   beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
>   beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
> 66d9fd0 
>   beeline/src/main/resources/BeeLine.properties ad79c01 
> 
> Diff: https://reviews.apache.org/r/52981/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jianguo Tian
> 
>

Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable

[Jianguo Tian]

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

Review request for hive and cheng xu.


Summary (updated)
-

HIVE-14679: csv2/tsv2 output format disables quoting by default and it's 
difficult to enable


Bugs: HIVE-14679
https://issues.apache.org/jira/browse/HIVE-14679


Repository: hive-git


Description
---

1. Quoting should be enabled by default for csv2, tsv2 and dsv.
2. Disabling quoting should be possible using a beeline argument.


Diffs (updated)
-

  beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 
  beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 
  beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 
  beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java 
66d9fd0 
  beeline/src/main/resources/BeeLine.properties ad79c01 

Diff: https://reviews.apache.org/r/52981/diff/


Testing
---


Thanks,

Jianguo Tian