zeroshade commented on code in PR #767:
URL: https://github.com/apache/iceberg-go/pull/767#discussion_r2891783873
##########
table/pos_delete_partitioned_fanout_writer.go:
##########
@@ -132,12 +130,22 @@ func (p *positionDeletePartitionedFanoutWriter)
processBatch(ctx context.Context
}
func (p *positionDeletePartitionedFanoutWriter) partitionPath(partitionContext
partitionContext) (string, error) {
- data :=
partitionRecord(slices.Collect(maps.Values(partitionContext.partitionData)))
spec := p.metadata.PartitionSpecByID(int(partitionContext.specID))
if spec == nil {
return "", fmt.Errorf("unexpected missing partition spec in
metadata for spec id %d", partitionContext.specID)
}
+ data := make(partitionRecord, spec.NumFields())
+ i := 0
+ for field := range spec.Fields() {
+ val, ok := partitionContext.partitionData[field.FieldID]
+ if !ok {
+ return "", fmt.Errorf("unexpected missing partition
value for field id %d in spec id %d", field.FieldID, partitionContext.specID)
+ }
+ data[i] = val
+ i++
+ }
Review Comment:
Do we utilize `AppendSeq` much at all? It still might be better making the
change to simplify scenarios like this (and users get a more familiar
interaction with `range` that returns index and field) than to optimize for the
AppendSeq scenario.
> If we change, I think we should change SortOrder.Fields() as well and do
it separately and then use here since it's a bigger scope. Happy to do if agreed
I say go for it unless either of you can think of a reason not to do so. We
can hold off on merging this until you make the change and then we can update
this. Thanks for volunteering!
--
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]
