fix(partitions): align paths with compacted partition fields#1332
fix(partitions): align paths with compacted partition fields#1332fallintoplace wants to merge 2 commits into
Conversation
tanmayrauth
left a comment
tanmayrauth
left a comment
There was a problem hiding this comment.
Nice, well-scoped fix, traced it through the snapshot-summary caller and the compacted record lines up with the new active-field indexing. LGTM. One out-of-scope question and a naming nit inline.
getRecordPartitions in table/partitioned_fanout_writer.go has the same shape this PR is fixing, it sizes its loop by spec.PartitionType(schema).FieldList (compacted) but indexes spec.Field(i) (full spec). It shouldn't trigger today since the active write spec shouldn't have a dropped source column, but is that guaranteed? Might be worth aligning it on activePartitionFields too (or dropping a comment) so the two paths don't drift. Happy to leave it for a follow-up since it's outside this PR's scope.
| return id | ||
| } | ||
|
|
||
| type partitionFieldType struct { |
There was a problem hiding this comment.
nit: this is really a (field, resolvedType) pair rather than a "type" — activePartitionField might read a touch clearer.
2 participants
Summary
make PartitionToPath walk the same active compacted partition-field sequence as PartitionType
reuse that active-field computation in both code paths to keep them aligned
add a regression test for a spec whose leading partition source column has been dropped from the current schema
Why
PartitionSpec.PartitionTypeexplicitly compacts away partition fields whose source columns are no longer present in the current schema. ButPartitionSpec.PartitionToPathstill indexed intops.fieldspositionally while reading values from that compacted partition record.When an earlier partition source column had been dropped, the compacted record shifted left while
PartitionToPathkept using names and transforms from the full spec. That could label values under the wrong partition field names or apply the wrong transform when generating partition paths.Testing
go test . -run 'Test(PartitionType|PartitionSpecToPath|PartitionSpecToPathWithDroppedLeadingSourceColumn)$' -count=1
go test . -count=1
go test ./table -count=1
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.4 run --timeout=10m . ./table/...
git diff --check