----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52542 -----------------------------------------------------------
I obivously don't have context here but I do have a few items which I think should be addressed. Thx! common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/25414/#comment91348> I think this should be HIVE_IN_TEZ_TEST ql/src/java/org/apache/hadoop/hive/ql/Context.java <https://reviews.apache.org/r/25414/#comment91349> there is a setter/getter for this field so I think it can be private. ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java <https://reviews.apache.org/r/25414/#comment91350> assert is almost never enabled. Should we use preconditions? ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java <https://reviews.apache.org/r/25414/#comment91340> constants should be all caps. If we fix this one can we fix bucketFileFilter as well. ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/25414/#comment91342> seems like we might want to log the exception here ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/25414/#comment91343> seems like we might want to log the exception here ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/25414/#comment91344> hmm, why not just log as INFO or DEBUG? ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/25414/#comment91341> no need for stringifyException here, you can pass e as a second arg ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java <https://reviews.apache.org/r/25414/#comment91345> looks like this can be final ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/25414/#comment91346> why not log the exception as well ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java <https://reviews.apache.org/r/25414/#comment91347> I think we should change this to IllegalStateException - Brock Noland On Sept. 6, 2014, 4:32 p.m., Alan Gates wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25414/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2014, 4:32 p.m.) > > > Review request for hive, Ashutosh Chauhan, Eugene Koifman, Jason Dere, and > Thejas Nair. > > > Bugs: HIVE-7788 > https://issues.apache.org/jira/browse/HIVE-7788 > > > Repository: hive-git > > > Description > ------- > > This patch adds plan generation as well as making modifications to some of > the exec operators to make insert/value, update, and delete work. The patch > is large, but about 2/3 of that are tests. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 54e2b18 > data/conf/tez/hive-site.xml 0b3877c > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/history/TestHiveHistory.java > 1a84024 > itests/src/test/resources/testconfiguration.properties 99049ca > metastore/src/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java > f1697bb > ql/src/java/org/apache/hadoop/hive/ql/Context.java 7fcbe3c > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 9953919 > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 4246d68 > ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java 7477199 > ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java f018ca0 > ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java e3bc3b1 > ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 7f1d71b > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java b1c4441 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 913d3ac > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 264052f > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java 8354ad9 > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 32d2f7a > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2b1a345 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 2f13ac2 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/BucketingSortingReduceSinkOptimizer.java > 96a5d78 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionOptimizer.java > 5c711cf > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > b5b2b60 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e4a30a2 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java > 026efe8 > > ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/LoadTableDesc.java 2dbf1c8 > ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 6dce30c > ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 5695f35 > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java c409ef5 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFToInteger.java 789c780 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestExecDriver.java 63ecb8d > > ql/src/test/org/apache/hadoop/hive/ql/parse/TestUpdateDeleteSemanticAnalyzer.java > PRE-CREATION > ql/src/test/queries/clientnegative/delete_not_acid.q PRE-CREATION > ql/src/test/queries/clientnegative/update_not_acid.q PRE-CREATION > ql/src/test/queries/clientnegative/update_partition_col.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_all_non_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/delete_all_partitioned.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_orig_table.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_tmp_table.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_where_no_match.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_where_non_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/delete_where_partitioned.q PRE-CREATION > ql/src/test/queries/clientpositive/delete_whole_partition.q PRE-CREATION > ql/src/test/queries/clientpositive/insert_orig_table.q PRE-CREATION > ql/src/test/queries/clientpositive/insert_update_delete.q PRE-CREATION > ql/src/test/queries/clientpositive/insert_values_dynamic_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/insert_values_non_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/insert_values_orig_table.q PRE-CREATION > ql/src/test/queries/clientpositive/insert_values_partitioned.q PRE-CREATION > ql/src/test/queries/clientpositive/insert_values_tmp_table.q PRE-CREATION > ql/src/test/queries/clientpositive/update_after_multiple_inserts.q > PRE-CREATION > ql/src/test/queries/clientpositive/update_all_non_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/update_all_partitioned.q PRE-CREATION > ql/src/test/queries/clientpositive/update_all_types.q PRE-CREATION > ql/src/test/queries/clientpositive/update_orig_table.q PRE-CREATION > ql/src/test/queries/clientpositive/update_tmp_table.q PRE-CREATION > ql/src/test/queries/clientpositive/update_two_cols.q PRE-CREATION > ql/src/test/queries/clientpositive/update_where_no_match.q PRE-CREATION > ql/src/test/queries/clientpositive/update_where_non_partitioned.q > PRE-CREATION > ql/src/test/queries/clientpositive/update_where_partitioned.q PRE-CREATION > ql/src/test/results/clientnegative/delete_not_acid.q.out PRE-CREATION > ql/src/test/results/clientnegative/update_not_acid.q.out PRE-CREATION > ql/src/test/results/clientnegative/update_partition_col.q.out PRE-CREATION > ql/src/test/results/clientpositive/delete_all_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/delete_all_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/delete_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/delete_tmp_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/delete_where_no_match.q.out PRE-CREATION > ql/src/test/results/clientpositive/delete_where_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/delete_where_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/delete_whole_partition.q.out > PRE-CREATION > ql/src/test/results/clientpositive/insert_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/insert_update_delete.q.out PRE-CREATION > ql/src/test/results/clientpositive/insert_values_dynamic_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/insert_values_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/insert_values_orig_table.q.out > PRE-CREATION > ql/src/test/results/clientpositive/insert_values_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/insert_values_tmp_table.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_all_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_all_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_tmp_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_where_no_match.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_where_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_where_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/delete_whole_partition.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_update_delete.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/tez/insert_values_dynamic_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_values_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_values_orig_table.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_values_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/insert_values_tmp_table.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_after_multiple_inserts.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_all_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_all_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_all_types.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/update_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/update_tmp_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/update_two_cols.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/update_where_no_match.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_where_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/tez/update_where_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/update_after_multiple_inserts.q.out > PRE-CREATION > ql/src/test/results/clientpositive/update_all_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/update_all_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/update_all_types.q.out PRE-CREATION > ql/src/test/results/clientpositive/update_orig_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/update_tmp_table.q.out PRE-CREATION > ql/src/test/results/clientpositive/update_two_cols.q.out PRE-CREATION > ql/src/test/results/clientpositive/update_where_no_match.q.out PRE-CREATION > ql/src/test/results/clientpositive/update_where_non_partitioned.q.out > PRE-CREATION > ql/src/test/results/clientpositive/update_where_partitioned.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/25414/diff/ > > > Testing > ------- > > Many tests included in the patch, including insert/values, update, and delete > all tested against: non-partitioned tables, partitioned tables, and temp > tables. > > > Thanks, > > Alan Gates > >
