[spark] Fix MSCK dropping the null partition of a value-only format table - #8878
[spark] Fix MSCK dropping the null partition of a value-only format table#8878sundapeng wants to merge 1 commit into
Conversation
sundapeng
changed the title
`__DEFAULT_PARTITION__` is `partition.default-name`, the value Paimon substitutes when a dynamic partition column is null or empty. It is the same value whatever the path layout. What the layout decides is the directory name: with `key=value` the null partition lives in `dt=__DEFAULT_PARTITION__`, whose first character is `d`; with a value-only layout the directory name is the value itself, so it begins with `_` and the generic hidden-directory rule swallows it. `PartitionPathUtils.isHiddenFile` already knows this and spares the directory whose name equals the table's default partition name, but only when it is told what that name is. `FormatTablePartitionRepair` called the five-argument `searchPartSpecAndPaths`, which passes `defaultPartValue` as null, so the rescue never applied and MSCK never saw the directory. Two ways to lose data followed: - `MSCK REPAIR TABLE ... ADD PARTITIONS` never registered the null partition; - `MSCK REPAIR TABLE ... SYNC PARTITIONS` read it as "registered but the directory is gone" and unregistered a partition that still holds data. Later queries then skip it without a word. `listStatusRecursively` applies the same rule while descending, so a null value on a non-leaf level hid the whole subtree rather than one directory. The scan path already gets this right: `FileSystemSplitEnumerator` passes `table.defaultPartName()` to the eight-argument overload. Repair now does the same. `partitionFilter` and `partitionType` stay null on purpose — repair diffs raw directory names, and handing it a partition type would reintroduce the cast that rewrites `month=01` to `1` and no longer round-trips to the real directory. The registered spec keeps the literal `__DEFAULT_PARTITION__`, which is what `FormatTableCommit` writes through the same `extractPartitionSpecFromPathOnlyValue` helper, so both sides of the diff agree and a second SYNC stays a no-op. Tests cover the missed registration, the wrongful unregistration, a subtree under a null non-leaf level, a table that overrides `partition.default-name` (hardcoding the literal passes every other case), and a key=value layout whose value starts with `_`, which is unaffected because the underscore sits on the value rather than on the first character of the directory name.
429c4ec to
1c1dce2
Compare
|
Closing this for now. The failure is confined to the opt-in value-only layout ( It is also only half of the class. A partition value that merely starts with The root of both is that a value-only layout puts partition directories and committer staging directories on the same level with no way to tell them apart by name, so relaxing the rule would start registering Recording it as a known issue on our side. Happy to reopen or file an issue for the naming ambiguity if that is useful to anyone. |
1 participant
Purpose
__DEFAULT_PARTITION__ispartition.default-name, the value Paimon substitutes when a dynamic partition column is null or empty. It is the same value whatever the path layout — what the layout decides is the directory name:key=value(default)dt=__DEFAULT_PARTITION__d— the hidden-directory rule never touches itformat-table.partition-path-only-value=true)__DEFAULT_PARTITION___— the rule swallows itPartitionPathUtils.isHiddenFilealready knows about this and spares the directory whose name equals the table's default partition name — but only when it is told what that name is.FormatTablePartitionRepaircalled the five-argumentsearchPartSpecAndPaths, which passesdefaultPartValueas null, so the rescue never applied and MSCK never saw the directory.Two ways to lose data followed:
MSCK REPAIR TABLE ... ADD PARTITIONSnever registered the null partition;MSCK REPAIR TABLE ... SYNC PARTITIONSread it as registered but the directory is gone and unregistered a partition that still holds data. Later queries then skip it without a word.listStatusRecursivelyapplies the same rule while descending, so a null value on a non-leaf level hid the whole subtree rather than a single directory.Tests
FormatTablePartitionRepairTest, 18 cases green. New cases cover:repairAddsTheNullPartitionDirectoryInValueOnlyLayoutrepairKeepsTheRegisteredNullPartitionInValueOnlyLayoutrepairDescendsIntoANullPartitionSubtreeInValueOnlyLayoutrepairReadsTheDefaultPartitionNameFromTableOptionspartition.default-name; hardcoding the literal passes every other caserepairKeepsAnUnderscoreValueInTheKeyValueLayoutdt=_abcis unaffected — the underscore sits on the value, not on the first character of the directoryCatalogManagedPartitionMsckRepairTest(11 SQL-level cases) stays green.API and Format
No API or format change.
partitionFilterandpartitionTypestay null on purpose: repair diffs raw directory names, and handing it a partition type would reintroduce the cast that rewritesmonth=01to1and no longer round-trips to the real directory. A comment now records that reason so the two nulls do not get "helpfully" filled in later.Note on scope
This fixes the default partition name. A partition value that merely starts with
_or.(dt='_abc') is still dropped by SYNC in a value-only layout, because_and.are not inCHAR_TO_ESCAPEand the directory is written bare. Widening the rule is a separate discussion: in a value-only layout a partition directory and a committer staging directory sit at the same level and cannot be told apart by name, so relaxing the rule would register_temporary/__magictrees as partitions. Happy to open a separate issue for that if you agree it needs a decision.