eldenmoon opened a new pull request, #70060:
URL: https://github.com/apache/doris/pull/70060
### What problem does this PR solve?
Issue Number: N/A
Related PR: #69995 (found during its review; this defect predates that PR
and also exists on branch-4.1 and branch-4.2)
Problem Summary:
A user JSON key such as `x__DORIS_VARIANT_SPARSE__y` is an ordinary Variant
path. Several BE paths identify the internal sparse and doc-value columns by
substring (`find("__DORIS_VARIANT_SPARSE__")`, `find(DOC_VALUE_COLUMN_PATH)`),
so such a key is mistaken for a physical binary column:
- **V3 external column meta** (`variant_ext_meta_writer.cpp`): the user
subcolumn is embedded in the root's children with no external path-index entry.
The reader accepts only the exact sparse name or `.b<n>` buckets there and
ignores the child, so the values read as NULL, and the next compaction no
longer sees the path and drops them.
- **Compaction statistics** (`variant_stats_calculator.cpp`, any storage
format): a materialized user column with the marker text is passed to the
sparse statistics code, which casts it to `ColumnMap`.
- **Doc-value marker**: writer selection, read plans, stats filtering and
NestedGroup path filtering use the same substring test, and the reader parses
the bucket with `std::stoi`, which throws for a user key.
Reproduction on an ASAN build of master, with
`variant_max_subcolumns_count=1` and three separate inserts
`{p:701,hot_a:11}/{hot_a:12}`, `{p:702,hot_b:21}/{hot_b:22}`,
`{p:703,hot_c:31}/{hot_c:32}` where `p = "x__DORIS_VARIANT_SPARSE__y"`:
| Case | master |
|---|---|
| V3, p materialized at load | `v[p]` is NULL right after the load |
| FULL compaction promotes p (3 > 2) | stops in the statistics calculator:
`Bad cast from type:doris::ColumnNullable to doris::ColumnMap` |
| key containing `__DORIS_VARIANT_DOC_VALUE__` in V3 | reader throws
`std::invalid_argument` from `std::stoi` |
Fix:
- One helper, `variant_util::variant_binary_column_bucket`, recognizes only
the relative names writers generate: the single sparse column and canonical
`.b<n>` sparse and doc buckets (`.b01`, `.b1tail`, `.b1.child` are user paths).
It replaces the substring tests at every site and the separate bucket parsers
in the reader and the doc compact writer.
- Existing V3 segments: an embedded child that is not an internal name is
registered as an ordinary subcolumn and read through the existing reader cache.
This runs once per segment open, with no per-row work. Values that an earlier
compaction already removed cannot be recovered.
Out of scope: a user key exactly equal to an internal name (for example a
top-level `__DORIS_VARIANT_SPARSE__`) still collides with the physical column;
on an ASAN build master stops at the reader's `col.has_variant_statistics()`
check.
### Release note
Fix wrong reads, compaction failures and possible data loss for Variant keys
that contain the internal sparse or doc-value column names, and make subcolumns
hidden by older V3 segments readable again.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test: `reserved_internal_path_key`, DUP/MOW × V2/V3 ×
ordinary/doc × 1/3 shards. Each case checks load-time materialization and two
FULL compactions (a new rowset in between; MOW also overwrites rows), comparing
every row's `v[p]` and the whole Variant.
- [x] Unit Test: exact identity against writer-generated names and
malformed suffixes, V3 metadata classification, statistics, an inline round
trip, and recovery of a V3 segment written with the old classifier. The
classifier, reader and statistics tests fail on master (the sparse statistics
test aborts with the bad cast above). Affected BE UT classes: 894 passed, 4
skipped (not applicable in this build).
- [x] Manual test: V3 segments written by unmodified master are read
correctly by the fixed BE, and their values survive two FULL compactions.
- [x] Full `variant_p0`: all suites pass except
`outfile_csv_variant_doc_value` (object storage 403) and the old-directory
`double_write_schema_change_with_variant` (`public` 1 vs true), which fail the
same way on unmodified master.
- Behavior changed:
- [x] Yes. Keys containing internal column names are ordinary paths.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 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]
