eldenmoon opened a new pull request, #68437:
URL: https://github.com/apache/doris/pull/68437
### What problem does this PR solve?
Issue Number: None
Related PR: #66204, #67675
Problem Summary:
A Variant schema template converts every value of a typed path to its
declared type when a segment is written. The V2 path builder first infers one
type for the path over every row written into the segment, then CASTs that
column to the declared type and drops the values the CAST turns into NULL. When
the rows put different value kinds on the path (a date string and a number, a
string and a boolean, an integer and a double, an array and a scalar, or mixed
elements inside an array), the inferred type is JSONB. CAST(JSONB -> DATE,
DATETIME, IPV4, IPV6 or TIMESTAMPTZ) has no conversion at all (the FE also
rejects `CAST(json AS DATE)`), so every value of the path is dropped, valid
ones included. Whether a valid value survives therefore depends on the other
rows of its segment; in a large load one stray number can wipe out a whole
segment of dates.
```sql
CREATE TABLE tpl2 (k INT, v VARIANT<'ts': DATETIME(3), 'd': DATE, 'i': INT>)
DUPLICATE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES
("replication_num" = "1");
INSERT INTO tpl2 VALUES
(1, PARSE_TO_VARIANT('{"ts": "2024-01-01 10:00:00.123456", "d":
"2024-01-01", "i": "5"}')),
(2, PARSE_TO_VARIANT('{"ts": 5, "d": 5, "i": "x"}'));
SELECT k, CAST(v AS STRING) FROM tpl2 ORDER BY k;
```
Before, row 1 reads `{"i":5}`: the BE converted path `d` from JSONB to
DateV2 and dropped 2 of 2 values. Written alone, the same row keeps `d` and
`ts`. After, row 1 reads `{"d":"2024-01-01","i":5,"ts":"2024-01-01
10:00:00.123000"}`, exactly as when it is written alone. `ARRAY<DATE>` paths
had the same problem per element.
The fix, one commit per step:
1. When the inferred type holds JSONB, convert through Variant instead.
Variant CAST groups values by kind and converts each group as a column of that
kind, which is what a segment holding only that kind does, and what a query
does when it CASTs a Variant value.
2. A segment holding only a kind CAST cannot convert at all (a boolean on a
DATE path, an array on an INT path, a number on an ARRAY path) used to fail the
whole load with e.g. `[INVALID_ARGUMENT]CAST AS DateV2 not supported BOOL`,
while the same value next to another kind was silently dropped. Such a
conversion now retries through Variant, so the value is dropped either way.
3. Text targets (STRING, or ARRAY of it) stay on the JSONB CAST: Variant
CAST renders a null array element as the text "null", so `["a", null]` would
have been stored as `["a", "null"]`.
4. `create_ip_wrapper` admitted every number and IP type and
`create_timestamptz_wrapper` every base type as a source, but only some have a
`CastToImpl`; the rest failed at execution with `[RUNTIME_ERROR]not support
NonStrictMode: from BIGINT cast to IPv4` instead of being rejected as
unsupported. That error leaked out of the non-strict Variant CAST
(`CAST(PARSE_TO_VARIANT('13') AS IPV4)` failed) and failed loads of numbers
into IPV4/IPV6/TIMESTAMPTZ typed paths. Both wrappers now admit only the
sources they implement. The FE already rejects the others (`cannot cast INT to
IPV4`, also for TRY_CAST), so SQL CAST does not change.
5. Test only: the regression suite covers IPV6 rather than a TIMESTAMPTZ
template path, since #67675 rejects TIMESTAMPTZ in templates.
Not changed here (existing behavior, left for follow-ups): reading an
`ARRAY<STRING>` typed path returns null elements as "null"
(`CAST(PARSE_TO_VARIANT('["a", null]') AS ARRAY<STRING>)` gives `["a",
"null"]`); a STRING typed path stores a JSON `true` as "true" when kinds mix in
its segment but as "1" otherwise; Variant CAST from a DATE value to BOOLEAN
still fails with RUNTIME_ERROR.
### Release note
Fix Variant schema template (typed) paths losing valid values, such as date
strings on a DATE or DATETIME path or IP strings on an IPV4 path, when the same
load also wrote another kind of value (such as a number) on that path. A value
that cannot be converted to a typed path's declared type is now dropped instead
of failing the load, and CAST of a Variant number or boolean to IPV4, IPV6 or
TIMESTAMPTZ returns NULL instead of an error.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] 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? -->
New BE UTs (each fails on the code before its commit):
`VariantPathBuilderTest.TypedConversionDoesNotDependOnOtherValueKindsInBatch`,
`TypedArrayConversionDoesNotDependOnOtherElementKindsInBatch`,
`TypedStringArrayConversionKeepsNullElements`,
`TypedConversionDropsKindsCastCannotConvert`,
`VariantColumnWriterReaderTest.v2_typed_date_paths_keep_strings_batched_with_numbers`,
`CastVariantV2FromTest.KindsWithoutIpOrTimestampTzConversionBecomeNull`. New
regression suite `variant_p0/predefine/test_predefine_mixed_value_kinds`. Also
run locally: every BE UT matching `*Variant*:*variant*:*Cast*:*cast*` (978
passed, 6 skipped), all of `variant_p0` (173 of 174; the other needs S3
credentials), `datatype_p0/{timestamptz,ip,timestamp_ns}` and
`function_p0/cast` (201 suites), and the typed-path and IP suites in
`fault_injection_p0`, `inverted_index_p0`, `ddl_p0`, `load_p0`, `query_p0`,
`nereids_function_p0` and `jsonb_p0`.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Typed paths keep the valid values they used to drop. A value whose kind
cannot be converted to the declared type is dropped instead of failing the
load. CAST from VARIANT to IPV4, IPV6 or TIMESTAMPTZ returns NULL for numbers
and booleans instead of an error.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
apache/doris-website#4170 (draft) documents the old behavior of these
cases and should follow this PR.
### 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 -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
