Jefffrey commented on code in PR #24140: URL: https://github.com/apache/datafusion/pull/24140#discussion_r3739647227
########## datafusion/spark/README.md: ########## @@ -27,23 +27,39 @@ This crate is a submodule of DataFusion that provides [Apache Spark] compatible [apache datafusion]: https://datafusion.apache.org/ [apache spark]: https://spark.apache.org/ -## Testing Guide +## Implementation Guidelines -When testing functions by directly invoking them (e.g., `test_scalar_function!()`), input coercion (from the `signature` -or `coerce_types`) is not applied. +When implementing these functions, you can check if there are existing implementations +in the [Sail] or [Comet] projects first. If you do port functionality from these +sources, make sure to port over the corresponding tests too, to ensure correctness +and compatibility. -Therefore, direct invocation tests should only be used to verify that the function is correctly implemented. +### `simplify()` Review Comment: This sounds great, especially for the datafusion UDFs; itll be easier to have tests in slts that can run with & without simplify to ensure they have same result, instead of having those tests split between slt and rust unit test -- 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]
