Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -866,12 +866,25 @@ func encodeBytes(data []byte, typ sql.Type) ([]byte, error) {
// in the returned |data| slice. The |typ| parameter is used to determine the maximum byte length of the serialized
// type, in order to determine how many bytes to use for the length prefix.
func encodeBytesFromAddress(ctx *sql.Context, addr hash.Hash, ns tree.NodeStore, typ sql.Type) (data []byte, err error) {
if addr.IsEmpty() {
// Empty TEXT/BLOB column — the tuple stores a zero hash (no out-of-band data).
// Serialize as a zero-length value instead of attempting a ChunkStore lookup.
blobType := typ.(sql.StringType)
if blobType.MaxByteLength() > 0xFFFFFF {
return []byte{0, 0, 0, 0}, nil
} else if blobType.MaxByteLength() > 0xFFFF {
return []byte{0, 0, 0}, nil
} else if blobType.MaxByteLength() > 0xFF {
return []byte{0, 0}, nil
}
return []byte{0}, nil
}
if ns == nil {
return nil, fmt.Errorf("nil NodeStore used to encode bytes from address")
}
bytes, err := ns.ReadBytes(ctx, addr)
if err != nil {
return nil, err
return nil, fmt.Errorf("binlog serialization: failed to read out-of-band data at address %s for type %s: %w", addr.String(), typ.String(), err)
}

blobType := typ.(sql.StringType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,49 @@ func TestTextSerializer(t *testing.T) {
})
}

// TestTextSerializer_MissingChunk verifies that when a TEXT column's out-of-band
// blob chunk is missing from the ChunkStore, the serializer returns an error instead
// of panicking. This reproduces a production crash where encodeBytesFromAddress
// panicked with "empty chunk returned from ChunkStore" during DOLT_COMMIT on a
// table with TEXT columns and binlog enabled.
func TestTextSerializer_MissingChunk(t *testing.T) {
s := textSerializer{}

// Create a blob in one NodeStore
_, addr := createTestBlob(t, []byte("some text data that is stored out of band"))

// Create a DIFFERENT NodeStore that does NOT have the blob chunk
differentStorage := &chunks.MemoryStorage{}
differentCS := differentStorage.NewViewWithFormat("__DOLT__")
differentNS := tree.NewNodeStore(differentCS)

// Build a tuple with the blob address from the first NodeStore
tupleDesc := val.NewTupleDescriptor(val.Type{Enc: val.StringAddrEnc})
tupleBuilder := val.NewTupleBuilder(tupleDesc, differentNS)
tupleBuilder.PutStringAddr(0, addr)
tuple, err := tupleBuilder.Build(buffPool)
require.NoError(t, err)

// Purge the shared node cache — all NodeStores share a global cache, so the
// blob written by createTestBlob is cached and would be found by differentNS.
// In production, cache eviction under pressure causes the same fallthrough to
// ChunkStore.Get, which returns EmptyChunk for missing hashes.
differentNS.PurgeCaches()

sqlCtx := sql.NewEmptyContext()

// Verify the different NodeStore truly cannot read this address
_, readErr := differentNS.Read(sqlCtx, addr)
require.Error(t, readErr, "differentNS should not be able to read blob from separate storage")

// Serialize using the DIFFERENT NodeStore (which doesn't have the chunk).
// This should return an error, NOT panic.
typ := gmstypes.Text
_, err = s.serialize(sqlCtx, typ, tupleDesc, tuple, 0, differentNS)
require.Error(t, err)
require.Contains(t, err.Error(), "failed to read out-of-band data")
}

func TestGeometrySerializer(t *testing.T) {
s := geometrySerializer{}

Expand Down
5 changes: 4 additions & 1 deletion go/store/prolly/tree/node_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package tree
import (
"bytes"
"context"
"fmt"
"sync"

"github.com/dolthub/dolt/go/store/chunks"
Expand Down Expand Up @@ -100,7 +101,9 @@ func (ns *nodeStore) Read(ctx context.Context, ref hash.Hash) (*Node, error) {
if err != nil {
return nil, err
}
assertTrue(c.Size() > 0, "empty chunk returned from ChunkStore")
if c.Size() == 0 {
return nil, fmt.Errorf("empty chunk returned from ChunkStore for hash %s", ref.String())
}

n, _, err = NodeFromBytes(c.Data())
if err != nil {
Expand Down
Loading