devanbenz commented on PR #12203: URL: https://github.com/apache/datafusion/pull/12203#issuecomment-2315936091
> There are a few things I noted when I reviewed this pull request, some of which may be good to either fix here or in other tickets/PRs: > > * The return type includes Utf8View, but the code doesn't do that > * The return type tests for Binary/LargeBinary/BinaryView/Dictionary but those are unsupported signature types > * The signature is missing `Exact(vec![Utf8View, Utf8, Utf8, Utf8]),` > * I don't see anywhere that the args are actually checked to have 3 or 4 elements anymore > * ~There are a bunch of macros in the tests that as best as I can see don't seem to be used at all. At least when I remove them the tests in that file all run. Given that I don't think the tests cover anything but StringArray as arg[0]~ Correction, the macros write out tests, my mistake. I've added some additional sqllogictests for flags & modified the type signature for ``` The signature is missing Exact(vec![Utf8View, Utf8, Utf8, Utf8]), ``` Could you please elaborate more on the return types in the first bullet? Thanks! -- 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]
