eldenmoon commented on PR #69222: URL: https://github.com/apache/doris/pull/69222#issuecomment-6263052252
Thanks — but I read this as the documented contract rather than a defect: > “Even declaring the subpath as DECIMAL via the Schema Template … does not fully guarantee precision, because the value is first parsed as DOUBLE and then converted to DECIMAL on the write path. If the JSON value is written as a string … the string is parsed directly into DECIMAL on write, preserving precision.” The Schema Template is documented as a storage-side declaration: it guides the subcolumn type, and precision for the JSON number form is bounded by binary64. The supported way to keep full precision is to send the value as a quoted string, and users who need exact DECIMAL already have that path. The PR body itself says the FAQ will have to be rewritten, and “Behavior changed: Yes” — so this is a change of a documented contract, not a bug fix, and it should be justified as such. I don’t think this patch can carry that change yet: It does not build. variant_util::glob_match_re2 (be/src/core/value/variant/variant_decimal_paths.cpp:48) is neither declared nor defined at this head: git grep glob_match_re2 returns only that call site, and variant_util.h has no such declaration. It only covers -?digits.digits. Integers beyond 2^64 and exponent forms still go through double, so the new contract would read “declaring DECIMAL guarantees precision, except for some number syntaxes” — much harder to document than today’s clean rule. It leaks the template into query results. The hint is attached by a global cast rule (SimplifyCastRule), while CAST(variant AS variant) is an identity in BE and only the shredder converts. So SELECT CAST(parse_to_variant(s) AS VARIANT<a:decimal>) now returns a JSON string on that path. It adds a full document copy + byte-wise scan per row on the ingestion path as soon as any DECIMAL path is declared. If the team does want to change the contract, the clean direction is to keep the number text available to the write path (ondemand raw_json_token, as discussed in #69220), or to type the parse call only in the insert sink and emit a DECIMAL — not a quoted string. Otherwise I’d keep the current documented behavior and close/rework this PR. -- 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]
