hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-1512) Coprocessors: Support aggregate functions
Date Fri, 15 Apr 2011 18:51:07 GMT

    [ https://issues.apache.org/jira/browse/HBASE-1512?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13020395#comment-13020395
] 

jiraposter@reviews.apache.org commented on HBASE-1512:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/585/#review481
-----------------------------------------------------------



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java
<https://reviews.apache.org/r/585/#comment965>

    Of these, I think only two are optional. colQualifier & filer. OK?



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java
<https://reviews.apache.org/r/585/#comment966>

    Agreed it should be initialized null to handle null resultset.



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java
<https://reviews.apache.org/r/585/#comment967>

    same as the max one above. Yes, in case of a null qualifier, it computes the value for
the overall family 



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java
<https://reviews.apache.org/r/585/#comment968>

    ok



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java
<https://reviews.apache.org/r/585/#comment969>

    ok



/src/main/java/org/apache/hadoop/hbase/client/coprocessor/LongColumnInterpreter.java
<https://reviews.apache.org/r/585/#comment970>

    Yes, more description should be added.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateCpProtocol.java
<https://reviews.apache.org/r/585/#comment971>

    hmm. the return type can be different. I will make it more generic to have a different
return type.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateCpProtocol.java
<https://reviews.apache.org/r/585/#comment972>

    I thought about it. I return a list but I see its not a right one to pass as one element
contains the sum and other contains the rowCount. So, it should be like a Pair as you said.
I will look into it.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment973>

    its in the Interface? Shall it be repeated here too?
    Ok, will do the name refactoring.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment974>

    ok, will use the equals method.
    I thought since it is an internal scanner (local to a region), it should not cross out
the boundaries while setting start-end rows. Will check it (should also improve performance).



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment975>

    right. a null is more pertinent here. will do it.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment976>

    yes, the current one does return min value. But as you said, it will return null now.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment977>

    I thought about it and then just left it as its only three line of code and a separate
call will be kind of over-refactoring. But once I set the start-end row as suggested by Gary,
it should become more light.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment978>

    yes indeed. It occurred to me while I saw Stack's review last night and here you are :).
I will do it.



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment979>

    ok. And what if I need to send more than 2 parameters as in case of Standard deviation?



/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java
<https://reviews.apache.org/r/585/#comment980>

    you mean a pojo with these many attributes. Is there exists such an object that i can
reuse (should be rpc compatible--> like implementing writable).



/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggFunctions.java
<https://reviews.apache.org/r/585/#comment981>

    So yes, will do all the name/space/grammar refactorings as suggested. 
    
    Thanks a lot to all of you folks for this wonderful review.


- himanshu


On 2011-04-13 08:37:14, Ted Yu wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/585/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-04-13 08:37:14)
bq.  
bq.  
bq.  Review request for hbase and Gary Helmling.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This patch provides reference implementation for aggregate function support through Coprocessor
framework.
bq.  ColumnInterpreter interface allows client to specify how the value's byte array is interpreted.
bq.  Some of the thoughts are summarized at http://zhihongyu.blogspot.com/2011/03/genericizing-endpointcoprocessor.html
bq.  
bq.  Himanshu Vashishtha started the work. I provided some review comments and some of the
code.
bq.  
bq.  
bq.  This addresses bug HBASE-1512.
bq.      https://issues.apache.org/jira/browse/HBASE-1512
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /src/main/java/org/apache/hadoop/hbase/client/coprocessor/AggregationClient.java PRE-CREATION

bq.    /src/main/java/org/apache/hadoop/hbase/client/coprocessor/LongColumnInterpreter.java
PRE-CREATION 
bq.    /src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateCpProtocol.java PRE-CREATION

bq.    /src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocolImpl.java PRE-CREATION

bq.    /src/main/java/org/apache/hadoop/hbase/coprocessor/ColumnInterpreter.java PRE-CREATION

bq.    /src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggFunctions.java PRE-CREATION

bq.  
bq.  Diff: https://reviews.apache.org/r/585/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  TestAggFunctions passes.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ted
bq.  
bq.



> Coprocessors: Support aggregate functions
> -----------------------------------------
>
>                 Key: HBASE-1512
>                 URL: https://issues.apache.org/jira/browse/HBASE-1512
>             Project: HBase
>          Issue Type: Sub-task
>          Components: coprocessors
>            Reporter: stack
>         Attachments: 1512.zip, AggregateCpProtocol.java, AggregateProtocolImpl.java,
AggregationClient.java, ColumnInterpreter.java, patch-1512-2.txt, patch-1512-3.txt, patch-1512-4.txt,
patch-1512-5.txt, patch-1512.txt
>
>
> Chatting with jgray and holstad at the kitchen table about counts, sums, and other aggregating
facility, facility generally where you want to calculate some meta info on your table, it
seems like it wouldn't be too hard making a filter type that could run a function server-side
and return the result ONLY of the aggregation or whatever.
> For example, say you just want to count rows, currently you scan, server returns all
data to client and count is done by client counting up row keys.  A bunch of time and resources
have been wasted returning data that we're not interested in.  With this new filter type,
the counting would be done server-side and then it would make up a new result that was the
count only (kinda like mysql when you ask it to count, it returns a 'table' with a count column
whose value is count of rows).   We could have it so the count was just done per region and
return that.  Or we could maybe make a small change in scanner too so that it aggregated the
per-region counts.  

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message