peterxcli commented on code in PR #5260: URL: https://github.com/apache/datafusion-comet/pull/5260#discussion_r3714963972
########## spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql: ########## Review Comment: we could also add test cases which is from the review of native `make_interval` impl: https://github.com/apache/datafusion-comet/pull/5039#discussion_r3659501474 > Spark's own sql-tests/inputs/interval.sql exercises exactly this range: > ```sql > select make_interval(1, 2, 3, 4, 0, 0, 123456789012.123456); > ``` ########## spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql: ########## Review Comment: ditto as `make_interval.sql. ref: https://github.com/apache/datafusion-comet/pull/5039#discussion_r3659506420 > Spark's IntervalExpressionsSuite ANSI mode block covers weeks = Int.MaxValue, and per-row overflow via hours/mins/seconds interactions. Something like: > ```sql > query expect_error(overflow) > SELECT make_interval(0, 0, 2147483647) > ``` ########## spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql: ########## Review Comment: other reference: https://github.com/apache/datafusion-comet/pull/5039#discussion_r3659505430 > Spark's own IntervalExpressionsSuite / interval.sql exercise that aren't covered yet: > > - Microsecond-precision seconds like Spark's docstring example make_interval(0, 1, 0, 1, 0, 0, 100.000001) asserted directly (it's currently only exercised via the column path where it can be hard to spot a per-row precision drift). > - Nulls in components other than years in the column path (currently only the years=NULL row is tested). > - Large-second cases from Spark's interval.sql: make_interval(1, 2, 3, 4, 0, 0, 123456789012.123456) and make_interval(0, 0, 0, 0, 0, 0, 1234567890123456789). If either is a known divergence (see the nanos-overflow comment on the Rust file), wrapping them in query ignore(<tracking issue>) would at least pin the behavior for future readers. > - Int.MinValue for a signed-overflow smoke test on the years column. -- 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]
