alamb commented on PR #19893: URL: https://github.com/apache/datafusion/pull/19893#issuecomment-3792484037
> > I understand that we wanna avoid breaking changes and hence are kinda liberal with `impl Into<...>`/`impl AsRef<...>` parameters. However these methods are generic and will be compiled on the call-site, i.e. whoever calls these methods has to recompile them. Furthermore calling these methods with different types duplicates these methods in LLVM and likely also in the resulting binary. This means: longer compile times and binary size bloat. > > Hence I would favor if we could avoid these `impl` parameters. > > Actually, I also think that using a generic here isn't justified, since `&Vec<u8>` can always be converted to a slice. I believe the API should be as simple as possible. Let's get @alamb opinion. Indeed my concern is the impact on (all) downstream users during the upgrade. I believe the initial PR from @askalt was substantially larger because it required changing a bunch of callsites. If we can avoid the use of `impl` and not require many changes (I understand some may be impossible to avoid) that sounds good to me -- 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]
