> 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 > >
