hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Barna Zsombor Klara <zsombor.kl...@cloudera.com>
Subject Re: Review Request 59096: HIVE-16607 ColumnStatsAutoGatherContext regenerates HiveConf.HIVEQUERYID
Date Tue, 09 May 2017 15:50:32 GMT

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



Thanks for the patch Peter. LGTM, with some minor comments/questions.


ql/src/java/org/apache/hadoop/hive/ql/QueryState.java
Line 32 (original), 31 (patched)
<https://reviews.apache.org/r/59096/#comment247460>

    I'm not 100% against the current solution, but if possible I would rather see the queryId
and maybe the queryString as instance variables of the QueryState. Preferably immutable, final
ones. Currently we hand out the queryConf so it may end up being modified, which we should
probably prevent.



ql/src/java/org/apache/hadoop/hive/ql/QueryState.java
Lines 157 (patched)
<https://reviews.apache.org/r/59096/#comment247459>

    Is this side effect intended? It probably should be modified on the queryConf.


- Barna Zsombor Klara


On May 9, 2017, 3:05 p.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59096/
> -----------------------------------------------------------
> 
> (Updated May 9, 2017, 3:05 p.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/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 
> 
> 
> Diff: https://reviews.apache.org/r/59096/diff/1/
> 
> 
> 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