discord9 commented on PR #21908: URL: https://github.com/apache/datafusion/pull/21908#issuecomment-4436688712
> Thanks for this contribution! > > As @Jefffrey suggests, this PR would be easier to review if you clarified the behavior you want to change. For example, "forbid lossy cast" is quite vague about specifically which casts were previously allowed, why those casts are lossy, and which casts will now be disallowed as a result of this PR. It would also be nice to create a separate GitHub issue, as the PR template requests. > > I wonder whether we want closed-by-default or open-by-default behavior. That is, instead of this approach, we could start by assuming that all casts are unsafe and then enumerate the specific casts that we want to allow. That would be safer, e.g., in the event of new types or new casts being added. Implementation might be simpler as well. sounds reasonable, how about for now only allow downcast like optimize `cast(ts_ms)=lit_ns` to `ts_ms==lit_ms`, does sound much safer and feels like the original intend -- 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]
