> On 2010-11-10 16:01:22, Nicolas wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java, line 1199
> > <http://review.cloudera.org/r/1208/diff/1/?file=17147#file17147line1199>
> >
> >     Would it be more straightforward to have a ReturnCode.MODIFY, that 
> > signals an include but suggests that the program must call modifyKeyValue() 
> > to get the transformed data.  Maybe this is too much of a one-off case...
> 
> Jonathan Gray wrote:
>     Not sure I completely follow.  You're saying the modification would 
> happen outside the filter?  No one needs to call modifyKeyValue() to get the 
> transformed data, it's done in the filter.
>     
>     In any case, yeah, I would not be for adding another ReturnCode just for 
> this.
> 
> Nicolas wrote:
>     I suggested this alternative because users normally expect filters to do 
> immutable operations on the data itself, and you're introducing side effects. 
>  If we stay with this paradigm, it's probably best to add a note in 
> Filter.filterKeyValue() that the KeyValue may be modified.

But a user would have to knowingly use this filter, right?  And the filter only 
has one purpose of mutating the KVs.  I do agree with what you're saying at 
some level but not sure what a note in the interface would do.  This is so if 
writing other filters, you would know that other filters in the chain could 
modify the KV?  How would you behave differently then in that case?


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1208/#review1896
-----------------------------------------------------------


On 2010-11-10 15:19:26, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1208/
> -----------------------------------------------------------
> 
> (Updated 2010-11-10 15:19:26)
> 
> 
> Review request for hbase, stack and Kannan Muthukkaruppan.
> 
> 
> Summary
> -------
> 
> Adds a new filter, KeyOnlyFilter.  The idea is that this will make it so only 
> the key portion of all the KVs are returned.  Could imagine a few use cases 
> where you just need the keys/index not the values.  We have one where we have 
> giant rows with big values and want to just get the qualifiers/versions w/o 
> values.
> 
> Adds a new method in KeyValue, convertToKeyOnly().  From javadoc:
> 
>   /**
>    * Converts this KeyValue to only contain the key portion (the value is
>    * changed to be null).  This method does a full copy of the backing byte
>    * array and does not modify the original byte array of this KeyValue.
>    * <p>
>    * This method is used by {...@link KeyOnlyFilter} and is an advanced 
> feature of
>    * KeyValue, proceed with caution.
>    */
> 
> 
> This addresses bug HBASE-3211.
>     http://issues.apache.org/jira/browse/HBASE-3211
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java 1033617 
>   trunk/src/main/java/org/apache/hadoop/hbase/filter/KeyOnlyFilter.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 
> 1033617 
>   trunk/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java 1033617 
>   trunk/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java 1033617 
> 
> Diff: http://review.cloudera.org/r/1208/diff
> 
> 
> Testing
> -------
> 
> Test of the KV method added to TestKeyValue.  Test of KeyOnlyFilter added to 
> TestFilter.  Both passing.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

Reply via email to