andygrove opened a new issue, #78: URL: https://github.com/apache/datafusion-spark/issues/78
### Describe the bug `time_trunc` differs from Spark in two ways: 1. It accepts units that Spark rejects. `SparkTimeTrunc::simplify` only checks that the unit is a non-null string literal, then passes it to DataFusion's `date_trunc` ([time_trunc.rs#L80-L93](https://github.com/apache/datafusion-spark/blob/fc2babe8c153d95e70fd303d55368a800d90930c/crates/spark/src/function/datetime/time_trunc.rs#L80-L93)). So `'MINS'`, for example, truncates to the minute. Spark accepts only `HOUR`, `MINUTE`, `SECOND`, `MILLISECOND` and `MICROSECOND`, and raises `INVALID_PARAMETER_VALUE.TIME_UNIT` for anything else. 2. A NULL unit is a planning error. Spark returns NULL. ### To Reproduce Spark 4.2.0: ```sql SELECT time_trunc('MINS', TIME '09:32:05.123456'); -- [INVALID_PARAMETER_VALUE.TIME_UNIT] The value of parameter(s) `unit` in `time_trunc` is invalid: expects one of the units 'HOUR', 'MINUTE', 'SECOND', 'MILLISECOND', 'MICROSECOND', but got ''MINS''. SQLSTATE: 22023 SELECT time_trunc(NULL, TIME '09:32:05.123456'); -- NULL ``` DataFusion, existing tests: - [time_trunc.slt#L18-L20](https://github.com/apache/datafusion-spark/blob/fc2babe8c153d95e70fd303d55368a800d90930c/crates/sqllogictest/test_files/datetime/time_trunc.slt#L18-L20) expects `09:32:00` for `'MINS'`. - [time_trunc.slt#L55-L56](https://github.com/apache/datafusion-spark/blob/fc2babe8c153d95e70fd303d55368a800d90930c/crates/sqllogictest/test_files/datetime/time_trunc.slt#L55-L56) expects `First argument of `TIME_TRUNC` must be non-null scalar Utf8` for a NULL unit. ### Expected behavior Accept only the five units above, case-insensitively, and raise an error like Spark's for any other unit. Return NULL for a NULL unit. The expectations listed above need to change. ### Additional context Found by running the sqllogictest files on Spark 4.2.0 with the PySpark runner from #54 (ANSI mode off, UTC session). The Spark results above come from that run. DataFusion results are the expectations in the `.slt` files at fc2babe8, which CI enforces. #40 covers the same kinds of difference for `trunc` and `date_trunc`. -- 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]
