morningman opened a new pull request, #69369:
URL: https://github.com/apache/doris/pull/69369
### What problem does this PR solve?
Issue Number: None
Related PR: #66538
Problem Summary:
**In short.** This ports a fix from branch-4.1 #66538 (Iceberg V3 default
values): both BE Iceberg readers parse the JSON form of a complex initial
default with `rapidjson::kParseFullPrecisionFlag`, so a DOUBLE inside it reads
back bit-identical to Spark.
**Background.** Iceberg stores the initial default of a struct, list or map
field as single-value JSON, for example `[0.18172760479972437302]` for a
`list<double>` column added after some data files were written. When Doris
reads such an older file, it materializes the missing column from that JSON:
FileScanner V1 in `create_initial_default_column`
(`be/src/format/table/iceberg_default_value.h`) and FileScanner V2 when it
builds the default expression (`be/src/format_v2/table/iceberg_reader.cpp`).
Master added both in #65851.
**The problem, and what it cost.** Both readers call rapidjson's default
`Parse`. Its fast path rounds a long decimal significand to a double and then
scales it by a power of ten, so a number with a long significand can be rounded
twice and land on the neighbouring double.
| | Master today | After this PR |
|---|---|---|
| `[0.18172760479972437302]` read by V1 / V2 | `0x3fc742d9a3b296db` |
`0x3fc742d9a3b296dc` (correctly rounded, same as Spark and Python) |
| Rows from files written before the column existed | can differ from Spark
in the last bit | identical to Spark |
**How this PR fixes it.** The two `Parse` calls become
`Parse<rapidjson::kParseFullPrecisionFlag>`, as on branch-4.1 and branch-4.2.
Top-level scalar defaults do not go through this JSON parse and are unchanged.
**Results.** The two unit tests ported from #66538,
`IcebergReaderTest.materializes_complex_double_initial_default_at_full_precision`
(V1) and
`IcebergV2ReaderTest.AnnotateBuildsComplexDoubleDefaultAtFullPrecision` (V2),
check the exact bits of the decoded double. Without the fix both fail with
`0x3fc742d9a3b296db`; with it both pass, and all 144 tests of the two reader
suites pass.
Original author: @hubgeter
### Release note
Fix: a DOUBLE inside the initial default of an Iceberg struct, list or map
column could be read one ULP off; it now matches Spark exactly.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Doubles inside complex Iceberg initial defaults read back
bit-identical to Spark instead of possibly one ULP off.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
