[fix](paimon) Handle special characters in partition values#65904
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Gabriel39
reviewed
Jul 22, 2026
Gabriel39
left a comment
Gabriel39
left a comment
Contributor
There was a problem hiding this comment.
[P1] Make partitionName a collision-free canonical key
Although this change correctly builds PartitionItem from the logical values, partitionName is still produced by concatenating the unescaped values as col=value/col=value and is then used as the unique key of both maps.
This encoding is not injective. For example, these two different partitions:
P1: a="x/b=y", b="z"
P2: a="x", b="y/b=z"
both produce a=x/b=y/b=z. The second partition silently overwrites the first one in both nameToPartition and nameToPartitionItem. Since both maps lose the same entry, PaimonPartitionInfo.isPartitionInvalid() still returns false, so partition pruning/MTMV partition tracking can consume incomplete metadata without detecting the collision.
Please use an escaped/canonical representation (for example Paimon PartitionPathUtils.generatePartitionPath, or another collision-free encoding) for the map key, and fail explicitly if put() replaces an existing entry. The regression test should contain both colliding partitions and assert that both maps retain two entries. Existing special-character tests only contain one partition, so they cannot detect this case.
Static review only; no tests were executed.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review completed with no additional unique inline findings. A blocking partition-identity issue remains, but it is already fully covered by review 4753864901 on the current head, so this review does not duplicate it.
- Goal and tests: Directly converting Paimon logical values fixes the single-partition Hive-path parsing failure and the added tests cover value preservation, map-order independence, and legacy DATE conversion. The existing review correctly identifies the missing two-partition collision case.
- Scope and mechanisms: The patch is otherwise small and focused. The direct value/type alignment is correct, and all supported Paimon partition-info creation paths share this loader.
- Concurrency and lifecycle: The changed maps/lists are constructed locally during snapshot-cache loading; no new thread, lock, publication, static-init, or resource-lifecycle issue was found.
- Compatibility and parallel paths: No FE-BE, RPC, EditLog, storage-format, transaction, data-write, or configuration contract changes. Partition-order changes can trigger a safe MTMV refresh. Paimon scans still receive predicates separately, but pruning metadata, partition-use collection, SHOW PARTITIONS, update-time aggregation, and MTMV mappings/snapshots consume these maps.
- Error handling and observability: Conversion failures still warn, but the already-reported collision is silent because both conversions succeed and the map-size validity check cannot detect equal overwrites.
- Performance: The new loop remains O(partitions * partition columns), avoids redundant Hive parsing, and adds no separate CPU/memory concern.
- Validation: Static review only as required by the review environment. CheckStyle is green on the live head; no FE unit-test result is present in the visible PR checks.
- User focus: No additional focus points were supplied.
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29010 ms |
Contributor
TPC-H: Total hot run time: 30178 ms |
Contributor
TPC-DS: Total hot run time: 177107 ms |
Contributor
ClickBench: Total hot run time: 24.93 s |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review completed with one P2 inline finding: the new escaped partition name still derives its component order from Doris's schema-field-ordered partitionColumns, not Paimon's declared partition-key order.
- Goal and tests: Direct logical-value conversion and Paimon's escaping fix special-character parsing and partition-name collisions. The five focused unit tests cover the intended local cases, but not the production field-order/key-order divergence in the inline finding.
- Scope and mechanisms: This is a small read-side metadata change. Filesystem/JDBC, normal Hive, and REST/fallback partition-loading paths converge on this helper.
- Concurrency and lifecycle: The change builds fresh local maps and lists before cache publication. It adds no thread, lock, shared mutable state, static initialization, or resource-lifecycle path.
- Compatibility and parallel paths: There is no FE-BE/RPC, EditLog, storage-format, configuration, transaction, or data-write compatibility change. Percent-escaped keys are otherwise consumed opaquely, but the declared-order mismatch changes the user-visible
SHOW PARTITIONSname and persisted MTMV partition identities. - Conditions, errors, and observability: Legacy DATE conversion and conversion-failure fallback remain intact. Duplicate identities now fail explicitly, loader failures do not publish partial maps, and no additional logging or metric issue was found.
- Persistence and refresh behavior: Old special-character names conservatively cause a one-time MTMV refresh. The inline issue is distinct because a valid field/key-order mismatch is noncanonical immediately and can change again after field-position evolution without changing the Paimon partition declaration.
- Performance: Complexity remains O(partitions * partition columns), and removing Hive-path reparsing introduces no separate CPU, memory, I/O, or blocking regression.
- Validation: On the live head, FE UT, External Regression, compile, and CheckStyle passed.
check_coverage_fefailed during JaCoCo report generation becauseavro-scanner.jarwas missing, not because a test or coverage threshold failed. Static review only; no local builds or tests were run under the review-environment contract. - User focus: No additional focus points were supplied.
| // Build partition values directly instead of parsing them as a Hive partition path. | ||
| List<String> partitionValues = Lists.newArrayListWithExpectedSize(partitionColumns.size()); | ||
| LinkedHashMap<String, String> orderedPartitionSpec = new LinkedHashMap<>(); | ||
| for (Column partitionColumn : partitionColumns) { |
Contributor
There was a problem hiding this comment.
[P2] Preserve Paimon's declared partition-key order
This loop uses partitionColumns as the canonical name order, but the production list is built by filtering TableSchema.fields() in PaimonExternalTable.initSchema; Paimon keeps partitionKeys() as an independent ordered list and emits Partition.spec() in that declared order. For fields [dt, region] with PARTITIONED BY (region, dt), this changes the identity from region=us/dt=2024-01-15 to dt=2024-01-15/region=us. Typed pruning values happen to remain aligned with Doris's field-ordered list, but SHOW PARTITIONS now exposes a non-Paimon name and persisted MTMV partition keys change whenever field order differs or is reordered.
Please build/cache the Doris partition columns in tableSchema.partitionKeys() order (or pass that order explicitly) and use it for the types, values, and escaped name. The test should exercise a schema whose field order differs from its declared partition-key order; the current test only permutes spec.entrySet() while manually passing the expected column order.
Gabriel39
approved these changes
Jul 23, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 23, 2026
Paimon `Partition.spec()` returns logical partition values. Doris previously concatenated them into a Hive-style partition path and parsed it with `HiveUtil.toPartitionValues()`. When a partition value contains `/`, `=`, or other path-related characters, it can be parsed incorrectly, causing invalid partition metadata, repeated warning logs, and potentially increased planning latency.
morningman
added a commit
that referenced
this pull request
Jul 23, 2026
…e-connector Upstream #65904 hardened PaimonUtil.generatePartitionInfo in fe-core to handle partition values containing path-special chars (/, =, [, ], *). That file was already deleted on this branch (P5 paimon migration, #64653), so the rebase surfaced only a modify/delete conflict; the equivalent logic lives in PaimonConnectorMetadata.collectPartitions. The core bug (re-parsing the concatenated Hive-style name back into values via HiveUtil.toPartitionValues) cannot occur here: the connector already supplies parsed values directly and fe-core never re-parses the name. This ports the remaining hardening: - drive value order from the partition columns, not Paimon's spec-map iteration order, so value i aligns with type i in fe-core; - render the partition NAME via PartitionPathUtils.generatePartitionPath so path-special chars are escaped exactly like the Paimon SDK, keeping names byte-identical to fe-core and collision-free; - fail loud on genuinely-duplicate partition names. Mirrors #65904's four new PaimonUtilTest cases at the connector level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
morningman
added a commit
that referenced
this pull request
Jul 23, 2026
…e-connector Upstream #65904 hardened PaimonUtil.generatePartitionInfo in fe-core to handle partition values containing path-special chars (/, =, [, ], *). That file was already deleted on this branch (P5 paimon migration, #64653), so the rebase surfaced only a modify/delete conflict; the equivalent logic lives in PaimonConnectorMetadata.collectPartitions. The core bug (re-parsing the concatenated Hive-style name back into values via HiveUtil.toPartitionValues) cannot occur here: the connector already supplies parsed values directly and fe-core never re-parses the name. This ports the remaining hardening: - drive value order from the partition columns, not Paimon's spec-map iteration order, so value i aligns with type i in fe-core; - render the partition NAME via PartitionPathUtils.generatePartitionPath so path-special chars are escaped exactly like the Paimon SDK, keeping names byte-identical to fe-core and collision-free; - fail loud on genuinely-duplicate partition names. Mirrors #65904's four new PaimonUtilTest cases at the connector level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2pc9qUhHnjcvLtzzuHaRE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Problem Summary:
Paimon
Partition.spec()returns logical partition values. Doris previously concatenated them into a Hive-style partition path and parsed it withHiveUtil.toPartitionValues().When a partition value contains
/,=, or other path-related characters, it can be parsed incorrectly, causing invalid partition metadata, repeated warning logs, and potentiallyincreased planning latency.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)