snmvaughan commented on PR #7578: URL: https://github.com/apache/datafusion-comet/pull/7578#issuecomment-6753438056
Thanks for checking the counting rules and the bound against Spark master, and for running the tests. You're right that the docs said more than the code does. With the setting on, a scale adjustment over 38 (`'6E+38'` to `DECIMAL(10,2)`) and a string of 40 or more digits both still end in `CAST_INVALID_INPUT` under ANSI, where Spark gives `NUMERIC_VALUE_OUT_OF_RANGE`. I pushed 24d3c802e with: - Both shapes in `test_parse_string_to_decimal_skip_integral_digit_limit`, and the error each mode gives in `test_cast_string_to_decimal_integral_digit_limit`. They assert what Comet does today and point at the issues. I left them out of the negative-scale `castTest`, because it compares Comet with Spark and the two differ here. - The caveat in the doc comment on `parse_string_to_decimal`, in the audit entry and in `cast.md`. I split the tracking in two. #7576 now covers the scale adjustment over 38, including `'6E+38'` with the setting on. It is a one-line change that is right for every target. The 40-digit shape is a different limit, so it has its own issue, #7581. It is not only a wrong error. Spark returns a value for it with a negative-scale target (`DECIMAL(10,-35)` gives `1.0000E+39`), and for a string with more than 38 fractional digits, with or without the setting (`'1.'` and 50 zeros gives `1.00`, where Comet returns NULL). Returning out-of-range for every overflow would be wrong for those. I'd rather fix both in their own PRs so this one stays about the limit. Is that all right with you? The workflow runs for the new commit are waiting at "Approve and run workflows". Could you or another committer approve them, and apply `run-all-spark-profiles` afterwards? The new tests compare Comet's error text with Spark's, which differs between Spark versions, and I can't add labels from a fork. -- 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]
