Re: Review Request: alter table concatenate fails and deletes data
On 2011-04-26 05:33:19, Carl Steinbach wrote: trunk/ql/src/test/queries/clientpositive/alter_merge_index.q, line 1 https://reviews.apache.org/r/665/diff/1/?file=17323#file17323line1 Please add some comments here too. Carl, what kind of comments do you want here? On 2011-04-26 05:33:19, Carl Steinbach wrote: trunk/ql/src/test/queries/clientnegative/alter_merge_index.q, line 1 https://reviews.apache.org/r/665/diff/1/?file=17322#file17322line1 Please add some comments explaining what it is that you're testing, e.g. why is this a negative test? Why is expected to fail? why is this a negative test? Why is expected to fail? should i put a positive test on a negative test dir? Should we explain all test cases in client negative dir? Come on, please put some positive suggestions. - Yongqiang --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/#review557 --- On 2011-04-25 22:47:17, Yongqiang He wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- (Updated 2011-04-25 22:47:17) Review request for hive. Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096599 trunk/conf/hive-default.xml 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096599 trunk/ql/src/test/queries/clientnegative/alter_merge_index.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_merge_index.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_merge_index.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_merge_index.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
Re: Review Request: alter table concatenate fails and deletes data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/#review560 --- trunk/conf/hive-default.xml https://reviews.apache.org/r/665/#comment1187 Besides describing the effect of this configuration property, can you also please explain the motivation? Why would the user want to set this to true? Is there a valid reason for setting it to false? trunk/ql/src/test/queries/clientnegative/alter_merge_index.q https://reviews.apache.org/r/665/#comment1183 Should we explain all test cases in client negative dir? Emphatically yes! And since I'm a helpful person I'll even write the comment for you: Verify that an error is thrown if the user attempts to concatenate an indexed table when hive.exec.concatenate.check.index=true Also, please explicitly set hive.exec.concatenate.check.index=true at the beginning of this test. Also, please change the name of this file and the name of the table so that it matches the semantics of the ALTER TABLE command, i.e. alter_concatenate_indexed_table.q and src_rc_concatenate_test. trunk/ql/src/test/queries/clientpositive/alter_merge_index.q https://reviews.apache.org/r/665/#comment1186 How about this: * Verify that an indexed table can be concatenated when hive.exec.concatenate.check.index=false. * Verify that a partitioned table can be concatenated when hive.exec.concatenate.check.index=false. The point of the comments is help the reader distinguish between details that are relevant to the intended purpose of the test from those that are not. trunk/ql/src/test/queries/clientpositive/alter_merge_index.q https://reviews.apache.org/r/665/#comment1188 Please replace merge with concatenate. A better name for this test would be alter_concatenate_indexed_table.q trunk/ql/src/test/queries/clientpositive/alter_merge_index.q https://reviews.apache.org/r/665/#comment1185 These tables get cleaned up automatically. Please remove the drop statements. - Carl On 2011-04-25 22:47:17, Yongqiang He wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- (Updated 2011-04-25 22:47:17) Review request for hive. Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096599 trunk/conf/hive-default.xml 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096599 trunk/ql/src/test/queries/clientnegative/alter_merge_index.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_merge_index.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_merge_index.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_merge_index.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
Re: Review Request: alter table concatenate fails and deletes data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- (Updated 2011-04-26 22:27:30.586885) Review request for hive. Changes --- address Carl's comments about .q files Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs (updated) - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096925 trunk/conf/hive-default.xml 1096925 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096925 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096925 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096925 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096925 trunk/ql/src/test/queries/clientnegative/alter_concatenate_indexed_table.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_concatenate_indexed_table.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_concatenate_indexed_table.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
Review Request: alter table concatenate fails and deletes data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- Review request for hive. Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096599 trunk/conf/hive-default.xml 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096599 trunk/ql/src/test/queries/clientnegative/alter_merge_index.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_merge_index.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_merge_index.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_merge_index.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
Re: Review Request: alter table concatenate fails and deletes data
On 2011-04-25 23:20:59, Ning Zhang wrote: trunk/conf/hive-default.xml, line 1042 https://reviews.apache.org/r/665/diff/1/?file=17317#file17317line1042 can you make the indentation consistent with the other property elements? It shows the same indentation on my local. So it might just be an review board display issue. On 2011-04-25 23:20:59, Ning Zhang wrote: trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java, line 1203 https://reviews.apache.org/r/665/diff/1/?file=17321#file17321line1203 So after adding this, does the block-level merge after INSERT OVERWRITE be automatically supported? No. Not automatically supported. We still need to do some work there. but it is a separate issue. - Yongqiang --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/#review554 --- On 2011-04-25 22:47:17, Yongqiang He wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- (Updated 2011-04-25 22:47:17) Review request for hive. Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096599 trunk/conf/hive-default.xml 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096599 trunk/ql/src/test/queries/clientnegative/alter_merge_index.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_merge_index.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_merge_index.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_merge_index.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
Re: Review Request: alter table concatenate fails and deletes data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/#review557 --- trunk/conf/hive-default.xml https://reviews.apache.org/r/665/#comment1177 The indentation looks the same in your editor because these are TABs. Please remove, and then configure your editor to disallow TABs. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java https://reviews.apache.org/r/665/#comment1176 Spacing. trunk/ql/src/test/queries/clientnegative/alter_merge_index.q https://reviews.apache.org/r/665/#comment1174 Please add some comments explaining what it is that you're testing, e.g. why is this a negative test? Why is expected to fail? trunk/ql/src/test/queries/clientpositive/alter_merge_index.q https://reviews.apache.org/r/665/#comment1175 Please add some comments here too. - Carl On 2011-04-25 22:47:17, Yongqiang He wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/665/ --- (Updated 2011-04-25 22:47:17) Review request for hive. Summary --- alter table concatenate fails and deletes data It is because the number of reducers is set to -1. In this patch, it is set to zero. Also added a move task as the child task of the merge task. added a conf to control whether to check index or not, and add the job name for the merge job. This addresses bug HIVE-2125. https://issues.apache.org/jira/browse/HIVE-2125 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1096599 trunk/conf/hive-default.xml 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1096599 trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 1096599 trunk/ql/src/test/queries/clientnegative/alter_merge_index.q PRE-CREATION trunk/ql/src/test/queries/clientpositive/alter_merge_index.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_merge_index.q.out PRE-CREATION trunk/ql/src/test/results/clientpositive/alter_merge_index.q.out PRE-CREATION Diff: https://reviews.apache.org/r/665/diff Testing --- Thanks, Yongqiang
