zeroshade commented on code in PR #1618:
URL: https://github.com/apache/iceberg-go/pull/1618#discussion_r3732159607
##########
codec/file_scan_task.go:
##########
@@ -162,6 +165,16 @@ func DecodeFileScanTask(data []byte, spec
iceberg.PartitionSpec, schema *iceberg
}, nil
}
+func validateScanRange(start, length, fileSize int64) error {
Review Comment:
Non-blocking: This helper relies on both callers checking negative start and
length first. Consider documenting that precondition or consolidating those
checks here so a future caller cannot accidentally make `fileSize-start` the
only guard.
##########
codec/file_scan_task.go:
##########
@@ -58,6 +58,9 @@ func EncodeFileScanTask(task table.FileScanTask, spec
iceberg.PartitionSpec, sch
if err := checkDataFileSpecID(task.File, spec); err != nil {
return nil, fmt.Errorf("codec: EncodeFileScanTask: %w", err)
}
+ if err := validateScanRange(task.Start, task.Length,
task.File.FileSizeBytes()); err != nil {
Review Comment:
Non-blocking: `FileSizeBytes()` is the manifest's recorded
`file_size_in_bytes` (`manifest.go:2042,2208`), not a filesystem stat.
Iceberg's immutable-file model normally makes it authoritative, but stale or
incorrectly small metadata can reject a physically readable file during encode.
Consider documenting that tradeoff.
##########
codec/file_scan_task_internal_test.go:
##########
@@ -19,12 +19,37 @@ package codec
import (
"encoding/binary"
+ "math"
"testing"
"github.com/apache/iceberg-go"
"github.com/stretchr/testify/require"
)
+func TestValidateScanRange(t *testing.T) {
Review Comment:
Non-blocking: Consider rounding out the boundary matrix with `(fileSize,
1)`, `(MaxInt64, 0, MaxInt64)`, `(MaxInt64-1, 1, MaxInt64)`, and `(0, 0)`, plus
invalid or non-positive sizes from a custom `DataFile`.
--
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]
