ibzib commented on pull request #13305: URL: https://github.com/apache/beam/pull/13305#issuecomment-745718835
> The downside for an interface like this is that it cannot be instantiated dynamically. I cannot take some existing object or code and programmatically turn it into a scalar function. We have hit this problem with people who want to programmatically create DoFn but are unable to do so. The Euphoria DSL for example and also you cannot take a DoFn and wrap it into a related DoFn. Annotation-driven things don't compose well. > > It also means that if you have a handle on a `ScalarFn` you can only invoke it via reflection. If I recall, your full change includes the reflection helpers to make that invocation easy, the same way we map DoFn to (DoFnInvoker, DoFnSignature). But I want to say that it was probably a mistake to treat DoFnInvoker as an implementation detail and DoFn as the main object. Instead, I would now view (DoFnInvoker, DoFnSignature) pair as the fundamental composable object, and annotated DoFn just a user interface for defining one. It is essentially an embedded programming language: DoFn is the syntax, DoFnSignature is the type, DoFnInvoker is the semantic object. > > I'm not sure I was clear before, but I really would recommend taking this approach for SQL. Perhaps this does not make sense because actually the main thing you do with these is pass them to Calcite to plug into linq4j? I think it's important to be aware of these drawbacks, but I don't think they necessarily apply here. The existing DoFn implementation works well for the majority of regular users, and I'm not anticipating the kind of advanced usage we have with DoFns. SQL UDFs are a much more limited use cases than DoFn, and DSLs are best implemented at a lower abstraction layer like DoFn. It'd be nice to have lambdas, but I think this marker interface is the best suited to the problem (because UDFs have indefinite arity and typing) for the majority of, if not all potential users. ---------------------------------------------------------------- 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]
