raghav-reglobe commented on PR #69222: URL: https://github.com/apache/doris/pull/69222#issuecomment-6288030516
Thanks for the careful read — all four points were right, and the PR is reworked along the first direction you named: the number's text now reaches the write path. **What changed (force-pushed):** 1. **The pre-pass is gone.** With DECIMAL template paths present, the document is read with simdjson's **ondemand** parser (`OndemandTreeCollector`, the twin of `JsonTreeCollector`), which has every number's `raw_json_token()`. A number on a declared path is read from its text straight into a Variant decimal (`Row::add_decimal`), so the shredder's cast is decimal → decimal. No quoted strings, no second scan, no extra copy beyond the padded buffer the parse itself needs. Documents without paths, numbers off the declared paths and root scalars go through the DOM collector exactly as before; any ondemand error falls back to the DOM parser's verdict (the row is rolled back first). 2. **Every number syntax on a declared path is exact now:** fractions, exponent forms and integers beyond 64 bits (`parse_json_number_as_decimal` folds the exponent into the scale, ≤ 38 significant digits). Beyond 38 significant digits the text is kept as a string and the template's cast decides — a correctly rounded value or NULL — never a binary64. So the contract becomes one sentence: a JSON number on a DECIMAL template path keeps its digits. 3. **Query results:** the typed parse call now yields a decimal holding the document's own digits on that path, which is what a cast to that templated type declares; it no longer turns a number into a JSON string. The FE hook is unchanged (it is the only place where the merged `CAST(parse_to_variant(x) AS variant<…>)` shape is visible); I'm happy to narrow it to the sink if you prefer that shape. 4. **The matcher is `fnmatch(…, FNM_PATHNAME)`,** the same call the shredder uses for typed paths — the stray `glob_match_re2` reference was mine and is gone, and the branch is rebased onto current master. On "contract vs. defect": agreed that the FAQ documents today's behaviour; the PR body now frames this as the change it is (the FAQ paragraph becomes advice rather than a requirement, follow-up docs PR once this lands). The regression expectation change is the same single value as before (`188118222.011121920` → `…933`). BE tests: `VariantDecimalPathsTest` (matcher + the decimal text reader), `DataTypeVariantV2SerdeInputTest` (exact fractions, exponent, big integer, >38 digits kept as text, off-path and invalid-text verdicts unchanged), `FunctionVariantParseTest`; FE: `VariantTypeDecimalPathsThriftTest`, `SimplifyCastRuleTest`. -- 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]
