~bigbes/sr-ht-dolt

5b00a51648a18e461bce9400d9fade6c9eeba28b — Eugene Blikh 5 days ago cdb9195
browse: say which cells are actually NULL
3 files changed, 193 insertions(+), 27 deletions(-)

M browse/fixture_test.go
A browse/rows_null_test.go
M browse/tables.go
M browse/fixture_test.go => browse/fixture_test.go +12 -4
@@ 149,13 149,21 @@ func buildFixture(root string) (string, error) {
		{"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.
		// carrying one >4KB longtext row — the out-of-line-text fixture — plus a
		// nulls table mixing real NULLs with the stored text "NULL". Folding them
		// into C4 keeps the commit topology (and every commit-count assertion)
		// unchanged; TestTables looks tables up by name, so the extra tables are
		// invisible to it while TestRowsResolvesLongText reads docs and the
		// null-mask tests read nulls 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)},
		{"sql", "-q", "create table nulls (id int primary key, note varchar(64))"},
		// id=2 has no value at all; id=3 stores the four characters N,U,L,L.
		// Both render as "NULL", so only the mask can tell them apart. Six rows
		// so a page can start in the middle and still be a full page.
		{"sql", "-q", "insert into nulls values " +
			"(1,'alpha'),(2,NULL),(3,'NULL'),(4,'delta'),(5,NULL),(6,'zeta')"},
		{"commit", "-Am", msgAddItem},
		{"push", "origin", "main"},
		{"push", "origin", "dev"},

A browse/rows_null_test.go => browse/rows_null_test.go +117 -0
@@ 0,0 1,117 @@
package browse

import (
	"context"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

// The nulls fixture table, in primary-key order. note is a real NULL at id=2
// and id=5, and the literal four-character string "NULL" at id=3.
//
//	id | note
//	 1 | 'alpha'
//	 2 |  NULL
//	 3 | 'NULL'
//	 4 | 'delta'
//	 5 |  NULL
//	 6 | 'zeta'

// TestRowsNullMaskSeparatesStoredNULLText is the whole point of the mask: the
// rendered strings of a real NULL and of a stored "NULL" are equal, and only
// Nulls tells a caller which is which.
func TestRowsNullMaskSeparatesStoredNULLText(t *testing.T) {
	db := openFixture(t)
	ctx := context.Background()

	page, err := db.Rows(ctx, "main", "nulls", 0, 4)
	require.NoError(t, err)
	require.Len(t, page.Rows, 4)
	require.Equal(t, []string{"id", "note"}, page.Columns)

	realNull := page.Rows[1]   // id=2
	storedText := page.Rows[2] // id=3
	assert.Equal(t, placeholderNull, realNull[1])
	assert.Equal(t, placeholderNull, storedText[1])
	assert.Equal(t, realNull[1], storedText[1],
		"both must still render identically; the rendered strings are not allowed to change")

	require.Len(t, page.Nulls, len(page.Rows))
	assert.True(t, page.Nulls[1][1], "id=2 stores no value: the mask must say so")
	assert.False(t, page.Nulls[2][1], "id=3 stores the text \"NULL\": the mask must not claim it is null")

	// Ordinary values and primary keys are never null.
	assert.False(t, page.Nulls[0][0])
	assert.False(t, page.Nulls[0][1])
	assert.False(t, page.Nulls[1][0], "a primary key cannot be null")
	assert.False(t, page.Nulls[3][1])
}

// TestRowsNullMaskAllFalseWhenPopulated: a table with no NULL in it yields a
// mask that is present, correctly shaped, and entirely false.
func TestRowsNullMaskAllFalseWhenPopulated(t *testing.T) {
	db := openFixture(t)
	ctx := context.Background()

	page, err := db.Rows(ctx, "main", "users", 0, 10)
	require.NoError(t, err)
	require.Len(t, page.Rows, 4)
	require.Len(t, page.Nulls, len(page.Rows))

	for i, mask := range page.Nulls {
		for j, isNull := range mask {
			assert.Falsef(t, isNull, "users row %d cell %d (%q) must not be null", i, j, page.Rows[i][j])
		}
	}
}

// TestRowsNullMaskShape: every mask row is as long as its row, which is as long
// as Columns — so a caller may index Nulls with any index valid for Rows.
func TestRowsNullMaskShape(t *testing.T) {
	db := openFixture(t)
	ctx := context.Background()

	for _, table := range []string{"users", "nulls", "docs", "items"} {
		page, err := db.Rows(ctx, "main", table, 0, 10)
		require.NoErrorf(t, err, "table %q", table)
		require.Lenf(t, page.Nulls, len(page.Rows), "table %q: one mask per row", table)
		for i := range page.Rows {
			assert.Lenf(t, page.Nulls[i], len(page.Rows[i]),
				"table %q row %d: mask width must equal row width", table, i)
			assert.Lenf(t, page.Nulls[i], len(page.Columns),
				"table %q row %d: mask width must equal column count", table, i)
		}
	}
}

// TestRowsNullMaskPagingAlignment reads a page that neither starts at the first
// row nor ends at the last one: an off-by-one between the mask and the rows
// would show up here and nowhere else.
func TestRowsNullMaskPagingAlignment(t *testing.T) {
	db := openFixture(t)
	ctx := context.Background()

	// Rows 3, 4, 5 of six: stored "NULL", 'delta', real NULL.
	page, err := db.Rows(ctx, "main", "nulls", 2, 3)
	require.NoError(t, err)
	require.Equal(t, 2, page.Offset)
	require.Equal(t, 6, page.Total)
	require.Equal(t, [][]string{
		{"3", placeholderNull},
		{"4", "delta"},
		{"5", placeholderNull},
	}, page.Rows)
	assert.Equal(t, [][]bool{
		{false, false},
		{false, false},
		{false, true},
	}, page.Nulls, "the mask must be aligned with the rows this page actually returned")

	// A page past the end has no rows and therefore no mask rows.
	past, err := db.Rows(ctx, "main", "nulls", 6, 3)
	require.NoError(t, err)
	assert.Empty(t, past.Rows)
	assert.Empty(t, past.Nulls)
}

M browse/tables.go => browse/tables.go +64 -23
@@ 38,11 38,38 @@ type TableInfo struct {
// RowPage is a paginated slice of a table's rows rendered to strings. Columns
// lists the column names in the same order as each row's cells. For keyed
// tables columns are primary-key columns first, then the rest.
//
// # Why a NULL renders as "NULL" and there is a mask beside it
//
// A cell that holds no value renders as the string "NULL" in Rows, which is
// exactly what a row that genuinely stores the four characters N,U,L,L renders
// as. That flattening stays: Rows is what pages and projections put on screen,
// where "NULL" is the conventional and readable answer, and every consumer of
// this package reads Rows as display strings — encoding nullness into the
// string instead (a sentinel, an empty string, a marker) would ripple through
// every template and every projection for no gain to a reader.
//
// Nulls is for the callers where the two are not interchangeable: an API that
// hands rows to a machine (the MCP row reader) gives out strings with no schema
// beside them, so without the mask an agent cannot tell an absent value from
// the text "NULL" — and answering "is there a value here?" is not something it
// can recover from the rendered string afterwards.
//
// Nulls is parallel to Rows: Nulls[i][j] reports whether Rows[i][j] was a real
// SQL NULL, so any index valid for Rows is valid for Nulls. It costs one bool
// per cell, which is what makes it affordable on a full page.
//
// Only NULL gets this treatment. The other two placeholders, "<binary>" and
// "<unreadable>", answer what a value *is*; NULL answers whether there is one
// at all, and only that question is unanswerable from the rendered string.
type RowPage struct {
	Columns []string
	Rows    [][]string
	Offset  int
	Total   int
	// Nulls[i][j] is true when Rows[i][j] is a real NULL rather than a value
	// that happens to render like one. Same shape as Rows.
	Nulls  [][]bool
	Offset int
	Total  int
}

// Tables lists the tables in the committed root at ref (a branch name or


@@ 206,6 233,7 @@ func (db *DB) Rows(ctx context.Context, refStr, table string, offset, limit int)
	page := &RowPage{
		Columns: colNames,
		Rows:    [][]string{},
		Nulls:   [][]bool{},
		Offset:  offset,
		Total:   int(total),
	}


@@ 244,14 272,18 @@ func (db *DB) Rows(ctx context.Context, refStr, table string, offset, limit int)
		}

		row := make([]string, len(refs))
		// One bool per cell, appended in lockstep with the row so the two can
		// never drift apart — including on a page that starts mid-table.
		nulls := make([]bool, len(refs))
		for i, r := range refs {
			if r.fromKey {
				row[i] = renderCell(ctx, ns, keyDesc, r.idx, key)
				row[i], nulls[i] = renderCell(ctx, ns, keyDesc, r.idx, key)
			} else {
				row[i] = renderCell(ctx, ns, valDesc, r.idx, value)
				row[i], nulls[i] = renderCell(ctx, ns, valDesc, r.idx, value)
			}
		}
		page.Rows = append(page.Rows, row)
		page.Nulls = append(page.Nulls, nulls)
	}

	return page, nil


@@ 302,20 334,27 @@ func rowLayout(sch schema.Schema) ([]string, []cellRef) {
// resolved through ns to their real content — without this they would render as
// "<binary>" (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) {
//
// isNull is true only when the field holds no value. It is the answer the
// rendered string cannot carry, since a stored "NULL" renders the same way; see
// the RowPage doc. It comes from the nullness test the renderer already had to
// do, so reporting it costs no extra read of the tuple. A field that is
// unreadable (out of range, or a panic recovered here) is not null: nothing is
// known about it, which is a different answer from "there is no value".
func renderCell(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) (out string, isNull bool) {
	defer func() {
		if r := recover(); r != nil {
			out = placeholderUnreadable
			out, isNull = placeholderUnreadable, false
		}
	}()

	if i < 0 || i >= td.Count() {
		// Column not present in this tuple (e.g. a virtual/dropped column that
		// isn't materialized): degrade rather than index out of range.
		return placeholderUnreadable
		return placeholderUnreadable, false
	}
	if td.IsNull(i, tup) {
		return placeholderNull
		return placeholderNull, true
	}

	switch td.Types[i].Enc {


@@ 328,47 367,49 @@ func renderCell(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int
		val.GeomAddrEnc, val.CommitAddrEnc,
		val.BytesAdaptiveEnc, val.GeomAdaptiveEnc:
		// Genuine binary / opaque out-of-band values: never dump raw bytes.
		return placeholderBinary
		return placeholderBinary, false
	}

	return td.FormatValue(ctx, i, td.GetField(i, tup))
	return td.FormatValue(ctx, i, td.GetField(i, tup)), false
}

// 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 {
// stored out-of-line) to its full content. It returns the same (string,
// isNull) pair as renderCell.
func resolveStringAddr(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) (string, bool) {
	h, ok := td.GetStringAddr(i, tup)
	if !ok {
		return placeholderNull
		return placeholderNull, true
	}
	s, err := val.NewTextStorage(h, ns).Unwrap(ctx)
	if err != nil {
		return placeholderUnreadable
		return placeholderUnreadable, false
	}
	return s
	return s, false
}

// 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 {
// *val.TextStorage to Unwrap) in the same field. It returns the same (string,
// isNull) pair as renderCell.
func resolveStringAdaptive(ctx context.Context, ns tree.NodeStore, td *val.TupleDesc, i int, tup val.Tuple) (string, bool) {
	v, ok, err := td.GetStringAdaptiveValue(ctx, i, ns, tup)
	if err != nil || !ok {
		if err != nil {
			return placeholderUnreadable
			return placeholderUnreadable, false
		}
		return placeholderNull
		return placeholderNull, true
	}
	switch s := v.(type) {
	case string:
		return s
		return s, false
	case *val.TextStorage:
		str, err := s.Unwrap(ctx)
		if err != nil {
			return placeholderUnreadable
			return placeholderUnreadable, false
		}
		return str
		return str, false
	default:
		return placeholderUnreadable
		return placeholderUnreadable, false
	}
}