From 419fb13352c9b01cb51fefea00b2b9b86a4b2c5b Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 19 Jul 2026 21:13:32 +0300 Subject: [PATCH] fix(browse): resolve out-of-line text instead of rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dolt stores text/longtext past a small inline threshold out-of-line, addressed by a content hash — StringAddrEnc (older writers) or StringAdaptiveEnc (inline-or-address, current writers). renderCell handled neither: address-encoded strings fell into the binary-placeholder switch and rendered "", while adaptive ones fell through to FormatValue and rendered the raw address hash. Every long description, close reason, comment body, and audit-event payload was lost on real databases. Resolve both through the prolly map's NodeStore (which satisfies val.ValueStore): GetStringAddr + NewTextStorage().Unwrap for the plain address case, GetStringAdaptiveValue (string | *TextStorage) for the adaptive case. Genuine binary/opaque encodings (blob, geometry, json-addr, commit-addr) still degrade to "". Fixture grows a >4KB longtext row (folded into C4 so the commit topology is untouched) and TestRowsResolvesLongText guards it. --- browse/browse_test.go | 24 ++++++++++++++++ browse/fixture_test.go | 15 +++++++++- browse/tables.go | 63 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/browse/browse_test.go b/browse/browse_test.go index 6ac1793adf42ca2dffa34143b2bb9887ddfc0023..cdd7533c9c315033d71a918669fa80f07be70763 100644 --- a/browse/browse_test.go +++ b/browse/browse_test.go @@ -275,6 +275,30 @@ func TestRowsEmptyTable(t *testing.T) { } } +// TestRowsResolvesLongText guards the fix for out-of-line (address-encoded) +// text: a >4KB longtext column must render its actual content, not the +// "" placeholder that inline-only rendering produced. +func TestRowsResolvesLongText(t *testing.T) { + db := openFixture(t) + ctx := context.Background() + + page, err := db.Rows(ctx, "main", "docs", 0, 10) + if err != nil { + t.Fatal(err) + } + if len(page.Rows) != 1 { + t.Fatalf("docs rows = %d, want 1", len(page.Rows)) + } + body := page.Rows[0][1] // columns: id, body + if body == placeholderBinary { + t.Fatalf("longtext rendered as %q; address-encoded text was not resolved", body) + } + if body != longDocBody { + t.Errorf("longtext body mismatch: got %d bytes, want %d bytes (prefix %.40q)", + len(body), len(longDocBody), body) + } +} + func TestCommitSummaryAddTable(t *testing.T) { db := openFixture(t) ctx := context.Background() diff --git a/browse/fixture_test.go b/browse/fixture_test.go index ff2ce9fcbca40a2ff3ec607fc0170438c59e778c..3d25b6288ff4172f811dda481a343771b72cef87 100644 --- a/browse/fixture_test.go +++ b/browse/fixture_test.go @@ -36,9 +36,15 @@ const ( msgAddUser = "add users" // C1: create empty users table msgInsert = "insert users" // C2: insert 3 rows msgModify = "modify users" // C3: update 1 row, insert 1 row - msgAddItem = "add items" // C4: create empty items table + msgAddItem = "add items" // C4: create empty items + docs tables ) +// longDocBody is a >4KB longtext value; at this size Dolt stores the string +// out-of-line (address-encoded) rather than inline, exercising the browse +// layer's address-resolution path (a short string would stay inline and never +// hit it). +var longDocBody = strings.Repeat("The quick brown fox. ", 300) // 6300 bytes + var ( haveDolt bool fixtureStore string // path to the bare store, main head = C4 @@ -142,7 +148,14 @@ func buildFixture(root string) (string, error) { {"branch", "dev"}, // dev head = C2 {"sql", "-q", "update users set name='alicia' where id=1; insert into users values (4,'dave')"}, {"commit", "-Am", msgModify}, + // C4 adds the empty items table and, in the same commit, a docs table + // carrying one >4KB longtext row — the out-of-line-text fixture. Folding + // it into C4 keeps the commit topology (and every commit-count assertion) + // unchanged; TestTables looks tables up by name, so the extra table is + // invisible to it while TestRowsResolvesLongText reads docs directly. {"sql", "-q", "create table items (sku varchar(16) primary key, qty int)"}, + {"sql", "-q", "create table docs (id int primary key, body longtext)"}, + {"sql", "-q", fmt.Sprintf("insert into docs values (1, '%s')", longDocBody)}, {"commit", "-Am", msgAddItem}, {"push", "origin", "main"}, {"push", "origin", "dev"}, diff --git a/browse/tables.go b/browse/tables.go index 7c08df9f8f29cd063de994a270580018b0e04d8e..f6040b4143ea3493368e3ab55a94c78b0955e98e 100644 --- a/browse/tables.go +++ b/browse/tables.go @@ -8,6 +8,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/schema" + "github.com/dolthub/dolt/go/store/prolly/tree" "github.com/dolthub/dolt/go/store/val" ) @@ -190,6 +191,9 @@ func (db *DB) Rows(ctx context.Context, refStr, table string, offset, limit int) return nil, fmt.Errorf("browse: iterate rows of %q: %w", table, err) } + // ns dereferences out-of-line (address-encoded) values — text/longtext that + // Dolt stores in a separate chunk rather than inline in the tuple. + ns := m.NodeStore() for { key, value, err := iter.Next(ctx) if err == io.EOF { @@ -202,9 +206,9 @@ func (db *DB) Rows(ctx context.Context, refStr, table string, offset, limit int) row := make([]string, len(refs)) for i, r := range refs { if r.fromKey { - row[i] = renderCell(ctx, keyDesc, r.idx, key) + row[i] = renderCell(ctx, ns, keyDesc, r.idx, key) } else { - row[i] = renderCell(ctx, valDesc, r.idx, value) + row[i] = renderCell(ctx, ns, valDesc, r.idx, value) } } page.Rows = append(page.Rows, row) @@ -251,7 +255,14 @@ func rowLayout(sch schema.Schema) ([]string, []cellRef) { // renderCell renders one tuple field to a display string. It never panics // (recovering into a placeholder) and maps binary / out-of-band encodings to // "" so a row preview never dumps opaque bytes. -func renderCell(ctx context.Context, td *val.TupleDesc, i int, tup val.Tuple) (out string) { +// +// text/longtext columns are the exception: Dolt stores anything past a small +// inline threshold out-of-line, addressed by a content hash (StringAddrEnc, or +// StringAdaptiveEnc which is inline-or-address in the same field). Those are +// resolved through ns to their real content — without this they would render as +// "" (addr) or a raw hash string (adaptive), losing every long +// description, close reason, comment body, and audit payload. +func renderCell(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) (out string) { defer func() { if r := recover(); r != nil { out = placeholderUnreadable @@ -268,12 +279,56 @@ func renderCell(ctx context.Context, td *val.TupleDesc, i int, tup val.Tuple) (o } switch td.Types[i].Enc { + case val.StringAddrEnc: + return resolveStringAddr(ctx, ns, td, i, tup) + case val.StringAdaptiveEnc: + return resolveStringAdaptive(ctx, ns, td, i, tup) case val.ByteStringEnc, val.Hash128Enc, val.CellEnc, - val.BytesAddrEnc, val.StringAddrEnc, val.JSONAddrEnc, + val.BytesAddrEnc, val.JSONAddrEnc, val.GeomAddrEnc, val.CommitAddrEnc, val.BytesAdaptiveEnc, val.GeomAdaptiveEnc: + // Genuine binary / opaque out-of-band values: never dump raw bytes. return placeholderBinary } return td.FormatValue(ctx, i, td.GetField(i, tup)) } + +// resolveStringAddr dereferences a StringAddrEnc field (text/longtext always +// stored out-of-line) to its full content. +func resolveStringAddr(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) string { + h, ok := td.GetStringAddr(i, tup) + if !ok { + return placeholderNull + } + s, err := val.NewTextStorage(h, ns).Unwrap(ctx) + if err != nil { + return placeholderUnreadable + } + return s +} + +// resolveStringAdaptive reads a StringAdaptiveEnc field, which stores its value +// either inline (returned as a string) or out-of-line (returned as a +// *val.TextStorage to Unwrap) in the same field. +func resolveStringAdaptive(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) string { + v, ok, err := td.GetStringAdaptiveValue(ctx, i, ns, tup) + if err != nil || !ok { + if err != nil { + return placeholderUnreadable + } + return placeholderNull + } + switch s := v.(type) { + case string: + return s + case *val.TextStorage: + str, err := s.Unwrap(ctx) + if err != nil { + return placeholderUnreadable + } + return str + default: + return placeholderUnreadable + } +}