ibzib commented on pull request #13306:
URL: https://github.com/apache/beam/pull/13306#issuecomment-745736494


   As requested I rewrote this without a dependency on Beam Java core. Right 
now it is a shameless copy-paste of some stuff from `CombineFn`, since the 
implementation will still depend on CombineFn anyway. But now we're decoupled 
at the API level, so we have more flexibility to make whatever changes are 
needed later (such as Kenn's suggestion to add a SQL type).
   
   > I don't agree in this case that creating a wrapper is trivial. For 
UDAF/CombeFn, I think it is non-trival work to create the wrapper.
   
   There already is a wrapper that's basically the same as what we're doing 
here: 
https://github.com/apache/beam/blob/release-2.25.0/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/transform/agg/AggregationCombineFnAdapter.java
   
   Anyway, I will figure it out :)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to