morningman opened a new pull request, #68959:
URL: https://github.com/apache/doris/pull/68959
### What problem does this PR solve?
Issue Number: None
Related PR: #66714, #66910, #67016
Problem Summary:
**In short.** This ports branch-4.1 #66714 to master. When the native
Parquet reader reads an unshredded Variant column, master decodes every value
and encodes it again through `VariantBatchBuilder`, although the bytes already
are Variant Encoding V1. With this PR the reader validates those bytes and
copies them into `ColumnVariantV2`. Because the copy validates every metadata
dictionary, a corrupt metadata key is reported instead of being dropped.
**Background.** A Parquet Variant column without shredding stores two binary
fields per row: `metadata` (the key dictionary) and `value`. `ColumnVariantV2`
keeps rows the same way, as deduplicated metadata blobs plus one value per row.
master #67016, the port of branch-4.1 #66910, added a direct path seek for
these columns. In branch-4.1, #66910 builds on #66714, which never reached
master, so #67016 re-encoded the values it found with `append_variant_rows` and
left out the two #66910 tests that need #66714.
**The problem, and what it cost.**
| Reading an unshredded Parquet Variant | master | with this PR |
|---|---|---|
| The whole value (`SELECT v`) | every row decoded and re-encoded through
`VariantBatchBuilder` (`encode_variant_range`) | metadata and value validated,
then copied; the profile reports `VariantUnshreddedDirectImportTime`, `Rows`
and `Bytes` |
| One path (`SELECT v['a']`), direct seek | each found value re-encoded
through `append_variant_rows`, which keeps one metadata dictionary per batch
and splits the batch in halves when rows use different dictionaries | found
values copied through the same importer |
| A row whose metadata has a corrupt key that the value does not use (for
example invalid UTF-8) | re-encoding drops the key, and the row reads as if it
were valid | the read fails with a corruption error, as in branch-4.x |
**How this PR fixes it.**
- `ColumnVariantV2::EncodedRowsAppender`, as in branch-4.x: it validates
each distinct metadata dictionary once and every value, rejects input that
borrows bytes from the destination column (including its shredded
materialization), and then copies the rows, interning metadata through a hash
index that it keeps across batches. A failed append leaves a typed or shredded
destination unchanged. `insert_encoded_rows` and `insert_encoded_batch` now
share `_append_prevalidated_encoded_data`.
- `variant_column_reader.cpp`: when the column is materialized, complete
unshredded roots go through the appender in 4096-row batches, and
`normalize_unshredded_path` uses it instead of `append_variant_rows`, which is
removed. Three persistent profile counters report the time, rows and bytes.
- The code follows master's `ColumnVariantV2`, which holds typed
`ColumnString` members where branch-4.x cast `IColumn`s. Hunks of the same
files that belong to other branch-4.x PRs (#66575, #66941) are not included.
**Results.**
- BE UT: the 4 new `ColumnVariantV2Test` cases, the 4 new
`VariantColumnReaderTest` cases, the two #66910 corrupt-metadata cases and two
new assertions pass, together with the other Variant, JNI and Parquet schema
suites (755 tests).
- With the production change reverted, exactly the six new reader cases
fail; the two corrupt-metadata cases fail because the read throws nothing.
Original authors: @hubgeter (#66714), @eldenmoon (the two #66910 tests)
### Release note
Reading an unshredded Parquet Variant row whose metadata has a corrupt key
now fails instead of ignoring the key.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- BE UT: `ColumnVariantV2Test` (appender atomicity on a typed
column, rejection of rows borrowed from the destination, metadata interning
across chunks, empty appends) and `VariantColumnReaderTest` (direct import of
encoded bytes, a 4097-row import across the batch boundary, profile counters on
immediate and late failures, and the two corrupt-metadata cases of #66910).
Each new reader case fails with the production change reverted.
`ColumnVariantV2Test.TypedComparisonBoundariesMatchEncodedWithoutMaterializing`
fails on a local macOS build with and without this change and passes on CI.
- [ ] 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 -->
- A corrupt key in the metadata of an unshredded Parquet Variant row
is reported when the row is read instead of being dropped.
- 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]
