andygrove commented on PR #4232: URL: https://github.com/apache/datafusion-comet/pull/4232#issuecomment-4406857625
> I've extended this pretty far on my experimental branches and had to work around some limitations at times. Let me reference back to those, see what maps back on to this, and come up with concise feedback that still generalizes to whatever approach we decide to take with the actual UDF implementations (hand-coded `CometUDF` functions, codegen, etc.). We're just looking to land the framework here, which I think is the right scope. > > For example, this morning I learned we need `num_rows` argument passed to `CometUDF.evaluate()` in case of UDFs with no input columns (e.g., Rand(), UUID(), CurrentTimestamp()). We need to know how many values to return, even if we don't anticipate this being the common case for this codepath. > > I'll come up with some feedback later today. Thanks @andygrove! Thanks @mbutrovich. Another possible approach here would be to just merge as-is ... the code is not integrated with Comet, so cannot do any harm yet. You could then follow up with PRs to address the issues that you have discovered so far? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
