theirix commented on PR #24703: URL: https://github.com/apache/datafusion/pull/24703#issuecomment-5469153122
> ive skimmed this but makes sense to me; if we're going to enable this behaviour (of changing scale) we might as well go all the way like spark, unlike duckdb which doesnt tighten it as much as spark does Yes, this makes sense. I'll try to refactor Spark's UDF implementation to reuse more from the core. > the clickhouse one seems surprising, is that a bug? 😅 It is usually complicated with ClickHouse - the behaviour is documented, but still contradictory. Turns out, all operations on the decimal type are done on a backing type (int32 if precision is 4, as in the example with `Decimal(4,1)`) regardless of precision: "Internally data is represented as normal signed integers with respective bit width. Real value ranges that can be stored in memory are a bit larger than specified above, which are checked only on conversion from a string.". We do perform precision checks for most operations. So if the result fits int32, ClickHouse won't complain. ClickHouse is [pretty relaxed on overflow checks](https://clickhouse.com/docs/reference/data-types/decimal#overflow-checks) (works only for 32- and 64-bit decimals but not for wider). Also, it is not universal - you can easily construct an overflown decimal even when it's enabled, so there is a function `isDecimalOverflow`, that tells you if you have overflown a calculation. For example, `-99999999.9999` is the smallest number that can fit into `DECIMAL32(1)` aka `DECIMAL(9,1)`, and `-100000000` cannot fit and cannot be constructed - expected behaviour. However, overflowing a value via `floor` on a valid input silently produces an overflowed value, while `isDecimalOverflow` shows it is bad. ```sql select version() \G version(): 26.3.17.4 SET decimal_check_overflow = 1; SELECT floor(CAST('-99999999.9999', 'DECIMAL32(1)')), toTypeName(floor(CAST('-99999999.9999', 'DECIMAL32(1)'))) -100000000 │ Decimal(9, 1) select isDecimalOverflow(floor(CAST('-99999999.9999', 'DECIMAL32(1)'))); 1 SELECT CAST('-100000000', 'DECIMAL32(1)') DB::Exception: Decimal value is too big ``` > adding a upgrade notice would be good 👍 Done, thank you! -- 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]
