Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8569 )

Change subject: IMPALA-5310: Part 2: Add SAMPLED_NDV() function.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8569/2/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8569/2/be/src/exprs/aggregate-functions-ir.cc@1552
PS2, Line 1552:  int bucket_idx = (i + j) % SampledNdvState::NUM_HLL_BUCKETS;
              :       merged_count += *state->GetCountPtr(bucket_idx);
              :       counts[pidx] = merged_count;
              :       StringVal hll = StringVal(state->GetHllPtr(bucket_idx), 
HLL_LEN);
              :       HllMerge(ctx, hll, &merged_hll);
              :       ndvs[pidx] = HllFinalEstimate(merged_hll.ptr, HLL_LEN);
> Ah, I misread the part where we do HllMerge(ctx, hll, &merged_hll). I read
Your original comment was totally valid. It's easy to misread the code. As a 
result of your comment the code is now hopefully easier to understand for 
others, so definitely worthwhile!



--
To view, visit http://gerrit.cloudera.org:8080/8569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia51d56ee67ec6073e92f90bebb4005484138b820
Gerrit-Change-Number: 8569
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Comment-Date: Wed, 29 Nov 2017 19:19:10 +0000
Gerrit-HasComments: Yes

Reply via email to