hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: Review Request 59096: HIVE-16607 ColumnStatsAutoGatherContext regenerates HiveConf.HIVEQUERYID
Date Wed, 10 May 2017 09:26:28 GMT


> On May 9, 2017, 4:17 p.m., Aihua Xu wrote:
> >

Thanks for the review Aihua!


> On May 9, 2017, 4:17 p.m., Aihua Xu wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/QueryState.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/59096/diff/1/?file=1711513#file1711513line116>
> >
> >     I suggest to rename it to just Builder and use this class as QueryState.Builder.

Renamed


> On May 9, 2017, 4:17 p.m., Aihua Xu wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/QueryState.java
> > Lines 96 (patched)
> > <https://reviews.apache.org/r/59096/diff/1/?file=1711513#file1711513line133>
> >
> >     Maybe name those functions like "withRunAsync", "withConfOverlay", "withGenerateNewQueryId"?
> >     
> >     And also add a new withConf() since conf can be optional as well. So build()
will not accept any parameters.

The first idea was, that hiveConf object is different - in some cases we might reuse it instead
of creating our own. I wanted to highlight the difference with the different pattern.
But you are rigth, it is better to have a withConf() method, and I will add an extra comment
explain the difference


- Peter


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


On May 10, 2017, 9:17 a.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59096/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 9:17 a.m.)
> 
> 
> Review request for hive, Aihua Xu and pengcheng xiong.
> 
> 
> Bugs: HIVE-16607
>     https://issues.apache.org/jira/browse/HIVE-16607
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> When creating a QueryState object the caller could specify if new QueryID should be created
or the exisiting should be used.
> Created a QueryStateBuilder to make the QueryState object creation more readable.
> New QueryId is only created in two places:
> - Driver constructor
> - Operation constructor
> Otherwise the existing queryId is used
> 
> 
> Diffs
> -----
> 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/mapreduce/TestHCatMultiOutputFormat.java
6ff48ee 
>   itests/src/test/resources/testconfiguration.properties 5ab3076 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   itests/util/src/main/java/org/apache/hive/beeline/QFile.java 3d9ca99 
>   itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java 7c50e18 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 29cce9a 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 6dfaa9f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cf575de 
>   ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/stats/PartialScanTask.java 77bce97

>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java b121eea

>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 3b719af

>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestExecDriver.java c7266bc 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestMacroSemanticAnalyzer.java c734988

>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 201622e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBJoinTreeApplyPredicate.java e607f10

>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBSubQuery.java 2674835 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestReplicationSemanticAnalyzer.java 80865bd

>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestSemanticAnalyzerFactory.java 5849950

>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestUpdateDeleteSemanticAnalyzer.java a573808

>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java
58cb4b4 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java 5d01080

>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java c552ba7

>   ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out PRE-CREATION

>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 0b27608 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 0b51591 
> 
> 
> Diff: https://reviews.apache.org/r/59096/diff/2/
> 
> 
> Testing
> -------
> 
> Added new BeeLine test - The original code made the test output different from the Cli
test output, since the QueryLog was truncated when the queryId was changed. After the change
the BeeLine test output is exactly the same as the Cli output.
> 
> 
> Thanks,
> 
> Peter Vary
> 
>


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