andygrove commented on PR #4745: URL: https://github.com/apache/datafusion-comet/pull/4745#issuecomment-5004694715
> This looks good and reads as a faithful move of the static datatype checks into `getSupportLevel`. I confirmed the `.get` calls in `convert` are safe, since the framework only calls `convert` on the `Compatible` branch and both serdes return `Compatible()` exactly when `serializeDataType` is defined. > > Two small things, neither blocking: > > * The PR description still mentions the `FromUnixTime` change, but that is not in this diff anymore (it went to [fix: report FromUnixTime non-default format as Unsupported #4847](https://github.com/apache/datafusion-comet/pull/4847)). Could you trim the body so the changelog stays accurate? > * While you are touching both messages, it might be nice to align the casing. `AttributeReference` produces "unsupported datatype:" and `KnownFloatingPointNormalized` produces "Unsupported datatype". > > _This review was prepared with the assistance of an LLM (Claude)._ Sorry, ignore this. I'm happy to merge once CI is green. -- 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]
