morningman opened a new pull request, #68963:
URL: https://github.com/apache/doris/pull/68963
### What problem does this PR solve?
Issue Number: None
Related PR: #68381, #68441, #68396, #68883, #68516
Problem Summary:
**In short.** This ports the tests of branch-4.x #68381 (explicit,
schema-owned Arrow block convertors) and #68441 (aligned Parquet and ORC
timestamp semantics) that did not come to master with their behavior. master
has the production code through #68396 and #68883, and through #65446 and
#68516; a hunk-by-hunk comparison found no gap there. This PR changes no
production code.
**Background.**
- #68381 moved format-specific Arrow writing into per-format convertors. For
Variant, Iceberg writes `arrow.parquet.variant` extension arrays with a
`metadata, value` storage struct, and Paimon writes its own `value, metadata`
struct. #68883 brought the Variant hooks of both convertors to master.
- #68441 made the Parquet and ORC timestamp semantics explicit: Paimon marks
every timestamp field with `timestamp_is_adjusted_to_utc` (INT96 alone cannot
tell a wall clock from an instant), a Hive write always sends
`hive.parquet.time-zone` (empty means wall clock), a Hudi MOR scan says up
front whether a slice may be read as native Parquet, ALTER CATALOG validates
the time zone, and only connectors that use the Hive INT96 time zone receive it.
**The problem, and what it cost.** These behaviors were not pinned on master:
| Behavior | Tested on master before this PR |
|---|---|
| Iceberg Variant through `IcebergArrowBlockConvertor`: extension array,
encoded bytes unchanged, outer NULLs, Parquet Variant logical type and field
ids, at the top level and nested in LIST, MAP and STRUCT | no; only a one-value
round trip of the SerDe hook |
| `PaimonArrowBlockConvertor`: bytes and outer NULLs for encoded and typed
Variants, `ARRAY<VARIANT>` through the Paimon SerDe, rejection of an
unsupported primitive at the Paimon hook | only through the generic Arrow path,
or one row without NULLs and without byte checks |
| ALTER CATALOG rejects an invalid IANA `hive.parquet.time-zone` and accepts
`+05:45` | only the rejection of `CST` |
| A Hive write without `hive.parquet.time-zone` still sends an empty time
zone | only an explicit `""` |
| Only a Parquet Hudi MOR table advertises native Parquet slices | Parquet
only; no ORC or HFile case |
| Paimon sets `timestamp_is_adjusted_to_utc`, not only its value, including
on nested timestamps | the value only, at the top level; the wall-clock check
also passes when the flag is missing |
| A connector that does not use the Hive INT96 time zone gets none from an
HMS catalog that sets one | only the opposite case |
**How this PR fixes it.**
- `DataTypeSerDeArrowTest`: `IcebergVariantExtensionAndParquetSchema` and
`NestedIcebergVariantExtensionsAndParquetSchema`, as in branch-4.x. They came
with #66620 (Iceberg Variant writes), but they build the Arrow schema
themselves and test only code master already has.
- `PaimonArrowBlockConvertorTest` (in
`data_type_variant_v2_serde_output_test.cpp`):
`BinaryStructPreservesEncodedAndTypedBytesAndOuterNulls`,
`NestedArrayUsesPaimonSerdeRecursively` and
`BinaryStructRejectsUnsupportedPaimonPrimitive`, with master's error text for
the last one. master's generic-path versions stay.
-
`HMSExternalCatalogParquetTimeZoneTest#testAlterValidatesCandidateTimezoneBeforePublishing`,
`HiveWritePlanProviderTest` (absent property, `UTC`, `+05:45`),
`HudiForceJniTest` (ORC and HFile MOR tables),
`PaimonScanPlanProviderTest#buildSchemaInfoCarriesTimestampSemantics` (set
flag, nested timestamp, other columns unflagged) and
`PluginDrivenScanNodeScanProviderSelectionTest#connectorWithoutHiveInt96SemanticsIgnoresCatalogParquetTimezone`.
Not ported: tests of branch-4.x-only code (`NonLocalizableInt32Predicate`
from #66802, the `{1,2,3,4}` expectation master changed on purpose in #66399,
VARBINARY mapping of file TVFs), and the Iceberg Variant writer tests that need
the Iceberg Variant write port (`VParquetWriterTest` Variant cases and the
Variant assertions of `IcebergBuildsItsOwnSchemaAndMetadata`).
**Results.** The BE suites pass (42 tests, 5 of them new) and the FE classes
pass. Each new FE assertion fails when the check it guards is removed:
| Change to the code under test | Test that fails |
|---|---|
| ALTER CATALOG skips the time zone check of the candidate properties |
`testAlterValidatesCandidateTimezoneBeforePublishing` |
| A Hive write sets the time zone only when the property is present |
`parquetTimezoneDistinguishesExplicitWallClockFromOldCoordinator` (absent
property) |
| Hudi advertises native Parquet for any input format |
`lazyMorScanAdvertisesNativeParquetBeforeRangesArrive` (ORC) |
| Paimon leaves `timestamp_is_adjusted_to_utc` unset for wall-clock
timestamps | `buildSchemaInfoCarriesTimestampSemantics` |
| The scan node passes the catalog's Hive time zone without asking the
provider | `connectorWithoutHiveInt96SemanticsIgnoresCatalogParquetTimezone` |
Original authors of the ported test cases: @Gabriel39 (#68381, #68441),
@hubgeter (#66620), @suxiaogang223 (#66321)
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- BE UT: `DataTypeSerDeArrowTest`,
`DataTypeVariantV2SerdeOutputTest` and `PaimonArrowBlockConvertorTest` pass (42
tests, 5 of them new).
- FE UT: `HMSExternalCatalogParquetTimeZoneTest`,
`PluginDrivenScanNodeScanProviderSelectionTest`, `HiveWritePlanProviderTest`,
`HudiForceJniTest` and `PaimonScanPlanProviderTest` pass, and fail under the
changes listed above.
- [ ] 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:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- 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]
