pierrec/lz4 v4.1.23 breaks ltx

[ncruces]

Specifically, pierrec/lz4#229 (implementing frame concatenation) breaks this piece of code:

ltx/decoder.go

Lines 275 to 281 in 133c1b1

	// readLZ4Trailer reads the LZ4 trailer frame to ensure we hit EOF.
	func (dec *Decoder) readLZ4Trailer() error {
	if _, err := io.ReadFull(dec.zr, make([]byte, 1)); err != io.EOF {
	return fmt.Errorf("expected lz4 end frame")
	}
	return nil
	}

readLZ4Trailer wants pierrec/lz4 to consume the trailer - and verify the checksum - but in doing so, it peeks into the subsequent data, finds an invalid signature, and errors out.

Even if you ignore that error, your stream is now in an invalid state to continue reading the next page.

[ncruces]

I've spent some time trying to fix, or work around, this… and it's not easy.

If you don't call readLZ4Trailer, you won't validate the checksum, and your underlying stream will be at the wrong offset (you will not consume the end of frame or checksum).

It you do call it, you'll consume those 8 bytes and the next 4 (you can't unread them) and get an error.

I could change lz4 so a zero byte read (Read(nil) basically) consumes the checksum but doesn't go further, but that breaks other LTX tests.

Also, pierrec/lz4 tests and CI are broken since December 2024, so I don't even know where to start there. But there is no alternative pure Go lz4 implementation.

[ncruces]

Not incredibly happy but #71 fixes this.

All other options either involve:

1. changing the LTX format (which I imagine is out of the question now)
2. changing the pierrec/lz4 dependency (or re-implementing it)

For (1) page data, when compressed, could (e.g.) start with the compressed length. Then you might use the LZ4 block format, and skip the frame format altogether. That would skip most of the pierrec/lz4 API, and use just CompressBlock and UncompressBlock. For incompressible pages, you can already use non-compressed page data. You'd lose page checksums, however, because pierrec/lz4 does not expose block checksums, so you might want to add some checksum.

For (2), you could open up the internal package or reimplement it on top of CompressBlock and UncompressBlock.

I tried to fix this in pierrec/lz4, but it's a hard proposition to change a project with 24k dependents and broken tests. I'm afraid I'll be doing to others what pierrec/lz4#229 did to LTX.

I also couldn't come up with a good fix conceptually. LZ4 has no knowledge of if frames are concatenated, the only way is to peek and find out, but this messes up the LTX format as currently "specified" (in fact specifying it like #53 would be greatly appreciated).

[corylanou]

Fixed by PR #73

[ncruces]

Also, I raised pierrec/lz4#238 to fix tests in the dep.