apache / apache/iceberg

Bug: `ValueWriters.FixedByteBufferWriter` uses `writeBytes()` instead of `writeFixed()` for Avro `fixed[N]` fields

Open Beginner friendly
#17,501 12 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
9.2k
Forks
3.5k
Avg merge
2d 11h
Merged PRs (30d)
132

Description

### Apache Iceberg version

1.11.0 (latest release)

### Query engine

Snowflake

### Please describe the bug 🐞

## Bug description

`ValueWriters.FixedByteBufferWriter.write(ByteBuffer, Encoder)` calls `encoder.writeBytes(bytes)`
instead of `encoder.writeFixed(arr, 0, length)`. For Avro `fixed[N]` fields this is wrong:
`writeBytes` prepends a zigzag-encoded length (e.g. `0x06` for a 3-byte value) before the data,
but the Avro reader consumes exactly N bytes for a `fixed` field and has no length prefix to skip.
The stray prefix byte spills into the next field in the record.

**Affected version:** 1.11.0 (also present in prior versions)

## Where the bug is

`core/src/main/java/org/apache/iceberg/avro/ValueWriters.java`, inner class `FixedByteBufferWriter`:

```java
private static class FixedByteBufferWriter implements ValueWriter {
private final int length;

@Override
public void write(ByteBuffer bytes, Encoder encoder) throws IOException {
Preconditions.checkArgument(bytes.remaining() == length, ...);
encoder.writeBytes(bytes); // ← BUG: should be writeFixed
}
}
```

The sibling class `FixedWriter` (which handles `byte[]`) already does it correctly:

```java
private static class FixedWriter implements ValueWriter {
@Override
public void write(byte[] bytes, Encoder encoder) throws IOException {
Preconditions.checkArgument(bytes.length == length, ...);
encoder.writeFixed(bytes); // ← correct
}
}
```

## Concrete corruption example

Writing a `FIXED(3)` partition value `[AB CD EF]` via `FixedByteBufferWriter`:

| Encoding | Output bytes | What the reader sees |
|---|---|---|
| `writeBytes` (current) | `06 AB CD EF` | `fixed[3]` consumes `06 AB CD`; `EF` spills into the next field |
| `writeFixed` (correct) | `AB CD EF` | `fixed[3]` consumes `AB CD EF` ✓ |

When the spill byte `EF` prefixes the subsequent `record_count` varint `08` (zigzag for 4 rows),
the combined varint `EF 08` decodes to raw value `1135`, which zigzag-decodes to `-568`. Every
reader (Java, native, Spark) trusts this value from the manifest and sees `record_count = -568`
for a file that has 4 rows.

## How to reproduce

1. Create a partitioned Iceberg table with a `FIXED(3)` identity-partition column.
2. Write a data file via the Iceberg Java writer (triggers `InternalWriter` → `FixedByteBufferWriter`).
3. Read the written manifest Avro directly and inspect the `data_file.record_count` field for the
partition entry — it will be a large negative number instead of the actual row count.
4. Alternatively: observe that `data_file.partition.val8` in the manifest contains `06abcd` instead
of `abcdef` for a `[AB CD EF]` partition value.

## Proposed fix

In `FixedByteBufferWriter.write()`, extract the bytes from the `ByteBuffer` and call
`encoder.writeFixed()`:

```java
@Override
public void write(ByteBuffer bytes, Encoder encoder) throws IOException {
Preconditions.checkArgument(bytes.remaining() == length,
"Cannot write byte buffer of length %s as fixed[%s]", bytes.remaining(), length);
byte[] arr = new byte[length];
bytes.duplicate().get(arr);
encoder.writeFixed(arr);
}
```

A regression test should assert that a round-trip write/read of a manifest entry with a `FIXED(N)`
identity-partition column preserves the exact partition value bytes and the correct `record_count`.

### Willingness to contribute

- [ ] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time

Contributor guide

Open the contributing guide

Research direction

Start in core/src/main/java/org/apache/iceberg/avro/ValueWriters.java at the FixedByteBufferWriter inner class, comparing it with FixedWriter. Reproduce the manifest write/read case described for a FIXED(3) identity partition, then add a regression test that preserves the partition bytes and record_count. Done means fixed fields are encoded without a length prefix and the manifest round trip retains both values.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.