You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Yongqiang He <he...@gmail.com> on 2011/04/26 00:47:17 UTC

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

Posted by Yongqiang He <he...@gmail.com>.

> 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

Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request: alter table concatenate fails and deletes data

Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
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

Posted by Yongqiang He <he...@gmail.com>.
-----------------------------------------------------------
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


Re: Review Request: alter table concatenate fails and deletes data

Posted by Yongqiang He <he...@gmail.com>.

> 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

Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/665/#review554
-----------------------------------------------------------



trunk/conf/hive-default.xml
<https://reviews.apache.org/r/665/#comment1169>

    can you make the indentation consistent with the other property elements?



trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
<https://reviews.apache.org/r/665/#comment1170>

    So after adding this, does the block-level merge after INSERT OVERWRITE be automatically supported?


- Ning


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