ilovesoup / hyracks

Automatically exported from code.google.com/p/hyracks
Apache License 2.0
0 stars 0 forks source link

Sum aggregator initialized improperly #51

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.Any SumAggregatorFactory test case

What is the expected output? What do you see instead?
The sum of attribute being aggregated. If the sum should be x, output is x+1

What version of the product are you using? On what operating system?
0.1.8

Please provide any additional information below.

specifically the init function of SumAggregatorFactory, defined as below:

            public void init(IFrameTupleAccessor accessor, int tIndex) throws HyracksDataException {
                sum++;
            }

It should be "sum = 0" instead of "sum++"

Original issue reported on code.google.com by manish.h...@gmail.com on 29 Nov 2011 at 1:44

GoogleCodeExporter commented 9 years ago
The accumulate() method also miscalculates the field offset.  It uses the 
expression (2 * fieldCount) to compute the length of a tuple's field slots, 
where fieldCount = accessor.getFieldCount().  This expands to 2 * 
recordDescriptor.getFields().length, but 
FrameTupleAccessor.getFieldSlotsLength() calculates 
recordDescriptor.getFields().length * 4.

It looks like FloatSumAggregatorFactory and MinMaxAggregatorFactory have the 
same problem; according to grep "accessor.getFieldCount()", 
CountAggregatorFactory, FloatSumAggregatorFactory, MinMaxAggregatorFactory, and 
SumAggregatorFactory compute the field slots length themselves, when they 
should defer the calculation to FrameTupleAccessor.

Original comment by rosenville@gmail.com on 29 Nov 2011 at 9:26

GoogleCodeExporter commented 9 years ago
Thanks for pointing out these bugs. For the calculation of the field slots, 
those legacy codes should be updated. I will fix them now and commit the 
changes soon.

Original comment by jarod...@gmail.com on 29 Nov 2011 at 9:45

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r833.

Original comment by jarod...@gmail.com on 29 Nov 2011 at 10:46

GoogleCodeExporter commented 9 years ago
Thank you for the prompt fix!

A question about the calculation of field slots in 
SumAggregatorFactory.java:line 72 -
shouldn't that be 

                sum += IntegerSerializerDeserializer.getInt(accessor.getBuffer().array(), tupleOffset + 4 * fieldCount
                        + fieldStart);

?

Thanks,
Manish

Original comment by manish.h...@gmail.com on 30 Nov 2011 at 12:43

GoogleCodeExporter commented 9 years ago
Thank you for the prompt fix!

A question about the calculation of field slots in 
SumAggregatorFactory.java:line 72 -
shouldn't that be 

                sum += IntegerSerializerDeserializer.getInt(accessor.getBuffer().array(), tupleOffset + 4 * fieldCount
                        + fieldStart);

?

Thanks,
Manish

Original comment by manish.h...@gmail.com on 30 Nov 2011 at 12:43

GoogleCodeExporter commented 9 years ago
Hi Manish, yes there were still some changes not committed to the 
SumAggregatorFactory. Should have been committed now. Thanks!

Original comment by jarod...@gmail.com on 30 Nov 2011 at 12:57