AlexanderSaydakov opened a new issue, #141:
URL: https://github.com/apache/datasketches-bigquery/issues/141

   Despite of this code being quite recent we have already accumulated some 
inconsistencies. This is about aggregate functions.
   
   - Initialize state object (sketch or union) in initialState() or not. At 
some point we realized that aggregate() can be called after deserialize(), in 
which case initialState() is not called, so we still need lazy initialization 
in the aggregate(), and therefore there is no point having it in 
initialState(). but we did not clean up everywhere
   - At some point we added parameters to functions, but forgot to pass them 
through serialize-deserialize. So we thought that instead of creating a new 
state in serialize() it might be a better approach to return the existing 
state. While creating a new state we need to remember to put into it all 
parameters which might be needed to create a sketch or union object later. 
While passing the existing one, we need to remember to remove the sketch or 
union object (after converting it to a binary blob). It is not conclusive which 
way is cleaner, and we have a mix of both approaches now.
   - It seems that aggregate() is not even called by BQ with null inputs, so 
checking for nulls there is useless, and we should be ready that aggregate() 
might never be called
   - Regarding naming convention. We decided to have “agg” in the names of 
aggregate functions, but somehow did not follow through. For instance, we have 
kll_sketch_float_build instead of kll_sketch_float_agg_build. Same with merge 
function and with some other sketches.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@datasketches.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@datasketches.apache.org
For additional commands, e-mail: dev-h...@datasketches.apache.org

Reply via email to