Pradeep Kamath commented on PIG-1062:

Review comments:
In SampleLoader.java
Isn't the idea of SampleLoader only to carry common code for RandomSampleLoader 
and PoissonLoader
and add a computeSamples() method? - Looks like now it has the getNext() 
needed by RandomSampleLoader in it now. Should we move that to 
RandomSampleLoader instead?

134             System.err.println("Sample " + samples[nextSampleIdx]);
Debug statement above should be removed.

Why is skipNext() needed? Can't loader.getNext() == null be used instead? If 
so, is recordReader

In RandomSampleLoader.java
XXX FIXME comment (put in by me :))should be removed

I think we should move the actual getNext() implementation code from 
SampleLoader to here

In PoissonSampleLoader.java

 40         // this will be value of first column in the special row   
I think this is no longer the case - should be removed.

    58     // memory per sample. divide this by avgTupleMemSize to get 
     59     private long memPerSample=0;
Should the above be called memToSkipPerSample?

 104         if(skipInterval == -1){
It doesn't look like skipInterval is initialized to -1

Instead of keeping track of max. num of columns in the different rows and then 
appending the
special marker string and num of rows at the end, would it be better to just 
have these as the
first two fields of the last tuple emitted and then introduce a split-union 
combination to 
ensure that the foreach pipeline gets the regular tuples (excluding the special 

> load-store-redesign branch: change SampleLoader and subclasses to work with 
> new LoadFunc interface 
> ---------------------------------------------------------------------------------------------------
>                 Key: PIG-1062
>                 URL: https://issues.apache.org/jira/browse/PIG-1062
>             Project: Pig
>          Issue Type: Sub-task
>            Reporter: Thejas M Nair
>            Assignee: Thejas M Nair
>         Attachments: PIG-1062.patch, PIG-1062.patch.3
> This is part of the effort to implement new load store interfaces as laid out 
> in http://wiki.apache.org/pig/LoadStoreRedesignProposal .
> PigStorage and BinStorage are now working.
> SampleLoader and subclasses -RandomSampleLoader, PoissonSampleLoader need to 
> be changed to work with new LoadFunc interface.  
> Fixing SampleLoader and RandomSampleLoader will get order-by queries working.
> PoissonSampleLoader is used by skew join. 

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to