You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Peter Vary <pv...@cloudera.com> on 2017/04/14 11:47:43 UTC
Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/
-----------------------------------------------------------
Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
Bugs: HIVE-16449
https://issues.apache.org/jira/browse/HIVE-16449
Repository: hive-git
Description
-------
Changes to support the following features in the BeeLine Qtests:
\u2013 SORT_QUERY_RESULTS
\u2013 HASH_QUERY_RESULTS
\u2013 SORT_AND_HASH_QUERY_RESULTS
The patch contains the following changes:
- Added the possibility to the OutputFile to add the existing FetchConverters
- Indicate the start and the end of the result fetching when fetching the resultset
- Parsed the query files for indentifing the cases when the converters are needed
Diffs
-----
beeline/src/java/org/apache/hive/beeline/Commands.java d179b37
beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java 9fae194
itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java 7244bf8
itests/util/src/main/java/org/apache/hive/beeline/qfile/package-info.java
ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out 70a37ca
ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 22a2d6a
ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out 6c9b8e4
Diff: https://reviews.apache.org/r/58454/diff/1/
Testing
-------
Checked the output changes of the existing BeeLineDriver tests
Thanks,
Peter Vary
Re: Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
Posted by Peter Vary <pv...@cloudera.com>.
> On April 21, 2017, 8:56 a.m., Zoltan Haindrich wrote:
> >
Thanks for the very usefull review Zoltan!
> On April 21, 2017, 8:56 a.m., Zoltan Haindrich wrote:
> > beeline/src/java/org/apache/hive/beeline/OutputFile.java
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/58454/diff/4/?file=1693664#file1693664line50>
> >
> > it seems like this 'coverter' currently leaked into this OutputFile class...which is part of the production code...
> >
> > Wouldn't it be possible to avoid this?
> > I'm think in something like:
> >
> > ```
> > myBeeLine.setOutputFile(new ConvertedOutputFile(new SortConverterre(), myBeeLine.getOutputFile() ) )
> > ```
> >
> > this may possibly also enable to remove the enums/etc from production code and move them into testing only
Fixed, good idea, thanks!
> On April 21, 2017, 8:56 a.m., Zoltan Haindrich wrote:
> > beeline/src/java/org/apache/hive/beeline/OutputFile.java
> > Lines 133 (patched)
> > <https://reviews.apache.org/r/58454/diff/4/?file=1693664#file1693664line134>
> >
> > I don't think that 'Supported' should appear in the enums name...it doesn't really helps in describing what this is :)
Done :)
> On April 21, 2017, 8:56 a.m., Zoltan Haindrich wrote:
> > beeline/src/java/org/apache/hive/beeline/OutputFile.java
> > Lines 134 (patched)
> > <https://reviews.apache.org/r/58454/diff/4/?file=1693664#file1693664line135>
> >
> > it would be nice to add the processing class to the enum; it would enable the cleanup of some switched
> >
> > ```
> > SORT_QUERY_RESULTS(SortAndDigestPrintStream.class)
> > ```
Please check the changes. I am not sure I understood your comment.
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/#review172617
-----------------------------------------------------------
On April 21, 2017, 4:53 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58454/
> -----------------------------------------------------------
>
> (Updated April 21, 2017, 4:53 p.m.)
>
>
> Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
>
>
> Bugs: HIVE-16449
> https://issues.apache.org/jira/browse/HIVE-16449
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Changes to support the following features in the BeeLine Qtests:
> \u2013 SORT_QUERY_RESULTS
> \u2013 HASH_QUERY_RESULTS
> \u2013 SORT_AND_HASH_QUERY_RESULTS
>
> The patch contains the following changes:
> - Added the possibility to the OutputFile to add the existing FetchConverters
> - Indicate the start and the end of the result fetching when fetching the resultset
> - Parsed the query files for indentifing the cases when the converters are needed
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 08d53ca
> beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
> itests/util/src/main/java/org/apache/hive/beeline/ConvertedOutputFile.java PRE-CREATION
> itests/util/src/main/java/org/apache/hive/beeline/QFile.java 0bde529
> itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java f1b53f7
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out c943b03
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 1ea6553
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out f639ba4
>
>
> Diff: https://reviews.apache.org/r/58454/diff/5/
>
>
> Testing
> -------
>
> Checked the output changes of the existing BeeLineDriver tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
Posted by Zoltan Haindrich <ki...@rxd.hu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/#review172617
-----------------------------------------------------------
beeline/src/java/org/apache/hive/beeline/OutputFile.java
Lines 49 (patched)
<https://reviews.apache.org/r/58454/#comment245700>
it seems like this 'coverter' currently leaked into this OutputFile class...which is part of the production code...
Wouldn't it be possible to avoid this?
I'm think in something like:
```
myBeeLine.setOutputFile(new ConvertedOutputFile(new SortConverterre(), myBeeLine.getOutputFile() ) )
```
this may possibly also enable to remove the enums/etc from production code and move them into testing only
beeline/src/java/org/apache/hive/beeline/OutputFile.java
Lines 133 (patched)
<https://reviews.apache.org/r/58454/#comment245699>
I don't think that 'Supported' should appear in the enums name...it doesn't really helps in describing what this is :)
beeline/src/java/org/apache/hive/beeline/OutputFile.java
Lines 134 (patched)
<https://reviews.apache.org/r/58454/#comment245701>
it would be nice to add the processing class to the enum; it would enable the cleanup of some switched
```
SORT_QUERY_RESULTS(SortAndDigestPrintStream.class)
```
- Zoltan Haindrich
On April 18, 2017, 8:32 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58454/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 8:32 a.m.)
>
>
> Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
>
>
> Bugs: HIVE-16449
> https://issues.apache.org/jira/browse/HIVE-16449
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Changes to support the following features in the BeeLine Qtests:
> \u2013 SORT_QUERY_RESULTS
> \u2013 HASH_QUERY_RESULTS
> \u2013 SORT_AND_HASH_QUERY_RESULTS
>
> The patch contains the following changes:
> - Added the possibility to the OutputFile to add the existing FetchConverters
> - Indicate the start and the end of the result fetching when fetching the resultset
> - Parsed the query files for indentifing the cases when the converters are needed
>
>
> Diffs
> -----
>
> beeline/src/java/org/apache/hive/beeline/Commands.java 08d53ca
> beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
> itests/util/src/main/java/org/apache/hive/beeline/QFile.java 0bde529
> itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java f1b53f7
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out c943b03
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 1ea6553
> ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out f639ba4
>
>
> Diff: https://reviews.apache.org/r/58454/diff/4/
>
>
> Testing
> -------
>
> Checked the output changes of the existing BeeLineDriver tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/
-----------------------------------------------------------
(Updated April 21, 2017, 4:53 p.m.)
Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
Changes
-------
Addressed review comments:
- Created ConvertedOutputFile.java, and moved converter specific code there
- Minimized the changes in OutputFile
Bugs: HIVE-16449
https://issues.apache.org/jira/browse/HIVE-16449
Repository: hive-git
Description
-------
Changes to support the following features in the BeeLine Qtests:
\u2013 SORT_QUERY_RESULTS
\u2013 HASH_QUERY_RESULTS
\u2013 SORT_AND_HASH_QUERY_RESULTS
The patch contains the following changes:
- Added the possibility to the OutputFile to add the existing FetchConverters
- Indicate the start and the end of the result fetching when fetching the resultset
- Parsed the query files for indentifing the cases when the converters are needed
Diffs (updated)
-----
beeline/src/java/org/apache/hive/beeline/Commands.java 08d53ca
beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
itests/util/src/main/java/org/apache/hive/beeline/ConvertedOutputFile.java PRE-CREATION
itests/util/src/main/java/org/apache/hive/beeline/QFile.java 0bde529
itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java f1b53f7
ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out c943b03
ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 1ea6553
ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out f639ba4
Diff: https://reviews.apache.org/r/58454/diff/5/
Changes: https://reviews.apache.org/r/58454/diff/4-5/
Testing
-------
Checked the output changes of the existing BeeLineDriver tests
Thanks,
Peter Vary
Re: Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/
-----------------------------------------------------------
(Updated April 18, 2017, 8:32 a.m.)
Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
Changes
-------
Rebased the patch, handled checkstyle and findbugs errors, and made it even more clear, that the converters are working only in test mode
Bugs: HIVE-16449
https://issues.apache.org/jira/browse/HIVE-16449
Repository: hive-git
Description
-------
Changes to support the following features in the BeeLine Qtests:
\u2013 SORT_QUERY_RESULTS
\u2013 HASH_QUERY_RESULTS
\u2013 SORT_AND_HASH_QUERY_RESULTS
The patch contains the following changes:
- Added the possibility to the OutputFile to add the existing FetchConverters
- Indicate the start and the end of the result fetching when fetching the resultset
- Parsed the query files for indentifing the cases when the converters are needed
Diffs (updated)
-----
beeline/src/java/org/apache/hive/beeline/Commands.java 08d53ca
beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
itests/util/src/main/java/org/apache/hive/beeline/QFile.java 0bde529
itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java f1b53f7
ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out c943b03
ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 1ea6553
ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out f639ba4
Diff: https://reviews.apache.org/r/58454/diff/4/
Changes: https://reviews.apache.org/r/58454/diff/3-4/
Testing
-------
Checked the output changes of the existing BeeLineDriver tests
Thanks,
Peter Vary
Re: Review Request 58454: HIVE-16449 BeeLineDriver should handle query
result sorting
Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58454/
-----------------------------------------------------------
(Updated April 14, 2017, 3:02 p.m.)
Review request for hive, Zoltan Haindrich, Vihang Karajgaonkar, and Yongzhi Chen.
Changes
-------
Added null check, so other tests will not fail
Bugs: HIVE-16449
https://issues.apache.org/jira/browse/HIVE-16449
Repository: hive-git
Description
-------
Changes to support the following features in the BeeLine Qtests:
\u2013 SORT_QUERY_RESULTS
\u2013 HASH_QUERY_RESULTS
\u2013 SORT_AND_HASH_QUERY_RESULTS
The patch contains the following changes:
- Added the possibility to the OutputFile to add the existing FetchConverters
- Indicate the start and the end of the result fetching when fetching the resultset
- Parsed the query files for indentifing the cases when the converters are needed
Diffs (updated)
-----
beeline/src/java/org/apache/hive/beeline/Commands.java d179b37
beeline/src/java/org/apache/hive/beeline/OutputFile.java 1014af3
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java 8c7057c
itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java 9fae194
itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java 7244bf8
itests/util/src/main/java/org/apache/hive/beeline/qfile/package-info.java
ql/src/test/results/clientpositive/beeline/smb_mapjoin_1.q.out 70a37ca
ql/src/test/results/clientpositive/beeline/smb_mapjoin_2.q.out 22a2d6a
ql/src/test/results/clientpositive/beeline/smb_mapjoin_3.q.out 6c9b8e4
Diff: https://reviews.apache.org/r/58454/diff/3/
Changes: https://reviews.apache.org/r/58454/diff/2-3/
Testing
-------
Checked the output changes of the existing BeeLineDriver tests
Thanks,
Peter Vary