From 5f2470456e0c49e48ba1479cca923c7199656ce6 Mon Sep 17 00:00:00 2001 From: Seunghyun Hwang Date: Sat, 17 Oct 2026 20:55:26 +0900 Subject: [PATCH] server: a Get that selects nothing reads the row, and nothing else A Get with no select loaded every edge with a query of its own, reading the neighbour's key and nothing else, and so did the snapshot a recorder takes its images from. `Proto()` already answers an edge that is not a list from the column the row keeps it in, when nothing was loaded -- which is the whole of what such a Get answers for one. So each of those queries was spent on an answer the row already held, and the answer was worse for it: the neighbour read with its key alone, every other field of it at its zero value and its timestamps in the year 1, as though that were the row. `image()` has been trimming that back to the key ever since; the reference the column gives is that already. A list edge is still loaded: its keys are on the other table. What it cost, counted in Rove (lesomnus/rove#103), whose gate checks a reference by a Get through the layer below: an imported row sent 121 statements and sends 68, and a Get of a thing with four edges is one query where it was five. Its tests pass on SQLite and PostgreSQL with this generator. - An empty select answers an edge with its key and nothing else; it answered a tenant created in the year 1. - A Get of a seat, three edges, is one query; it was four. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk --- apps/server/app/x-select.go | 16 +++++- apps/server/app/x-snapshot.go | 5 ++ internal/apptest/server/bare/field.g.go | 3 -- internal/apptest/server/bare/note.g.go | 1 - internal/apptest/server/bare/seat.g.go | 9 ---- internal/apptest/server/bare/shift.g.go | 1 - internal/apptest/server/bare/tenant.g.go | 1 - internal/apptest/server/bare/user.g.go | 4 -- internal/apptest/server/bare/x-get_test.go | 57 ++++++++++++++++++++++ 9 files changed, 77 insertions(+), 20 deletions(-) diff --git a/apps/server/app/x-select.go b/apps/server/app/x-select.go index 09af0b1..534d535 100644 --- a/apps/server/app/x-select.go +++ b/apps/server/app/x-select.go @@ -118,8 +118,22 @@ func (w *fileWork) xSelectInit() { ") {") w.P(" if m != nil {") w.P(" ", name_x, "Select(q, m)") - w.P(" } else {") + // Only a list edge is loaded. A row holds the key of every other one in + // the column the edge is kept in, and `Proto()` answers with it when + // nothing was loaded -- which is the whole of what a Get with no select + // answers for an edge. Loading it anyway was a query for each edge of + // every row read, and an answer that was worse: the neighbour read with + // its key and nothing else, so every other field of it at its zero value + // and its timestamps in the year 1, as though that were the row. + opened := false for p := range w.Entity.Edges() { + if !p.IsList() { + continue + } + if !opened { + w.P(" } else {") + opened = true + } name_p := work.Name(p.Name()).Ent() name_y := p.Target().Name() w.P(" q.With", name_p, "(select", name_y, "Key)") diff --git a/apps/server/app/x-snapshot.go b/apps/server/app/x-snapshot.go index b5c6a83..6702382 100644 --- a/apps/server/app/x-snapshot.go +++ b/apps/server/app/x-snapshot.go @@ -16,6 +16,11 @@ func (w *fileWork) xSnapshots() { ", p ", w.entPkg().Ident(name_x), ", lock bool) (*", w.ent.Ident(name_x), ", error) {") w.P(" q := s.Db.", name_x, ".Query().Where(p)") for edge := range w.Entity.Edges() { + // A list edge only: the key of any other is in the row, and + // `Proto()` answers with it -- see [fileWork.xSelectInit]. + if !edge.IsList() { + continue + } target := edge.Target().Name() pkg := w.ent + "/" + protogen.GoImportPath(strings.ToLower(target)) w.P("q.With", work.Name(edge.Name()).Ent(), "(func(q *", w.ent.Ident(target+"Query"), "){q.Select(", pkg.Ident("FieldId"), ")})") diff --git a/internal/apptest/server/bare/field.g.go b/internal/apptest/server/bare/field.g.go index 0275c94..06843f5 100644 --- a/internal/apptest/server/bare/field.g.go +++ b/internal/apptest/server/bare/field.g.go @@ -1004,7 +1004,6 @@ func ValueFieldSelect(q *ent.ValueFieldQuery, m *apptest.ValueFieldSelect) { func ValueFieldSelectInit(q *ent.ValueFieldQuery, m *apptest.ValueFieldSelect) { if m != nil { ValueFieldSelect(q, m) - } else { } } @@ -1482,7 +1481,6 @@ func MessageFieldSelect(q *ent.MessageFieldQuery, m *apptest.MessageFieldSelect) func MessageFieldSelectInit(q *ent.MessageFieldQuery, m *apptest.MessageFieldSelect) { if m != nil { MessageFieldSelect(q, m) - } else { } } @@ -1976,7 +1974,6 @@ func MapFieldSelect(q *ent.MapFieldQuery, m *apptest.MapFieldSelect) { func MapFieldSelectInit(q *ent.MapFieldQuery, m *apptest.MapFieldSelect) { if m != nil { MapFieldSelect(q, m) - } else { } } diff --git a/internal/apptest/server/bare/note.g.go b/internal/apptest/server/bare/note.g.go index 54fd076..d89fb84 100644 --- a/internal/apptest/server/bare/note.g.go +++ b/internal/apptest/server/bare/note.g.go @@ -249,7 +249,6 @@ func NoteSelect(q *ent.NoteQuery, m *apptest.NoteSelect) { func NoteSelectInit(q *ent.NoteQuery, m *apptest.NoteSelect) { if m != nil { NoteSelect(q, m) - } else { } } diff --git a/internal/apptest/server/bare/seat.g.go b/internal/apptest/server/bare/seat.g.go index 6291146..ac3783b 100644 --- a/internal/apptest/server/bare/seat.g.go +++ b/internal/apptest/server/bare/seat.g.go @@ -17,8 +17,6 @@ import ( ent "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent" predicate "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/predicate" seat "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/seat" - tenant "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/tenant" - user "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/user" entpatch "github.com/protobuf-orm/protoc-gen-orm-ent/runtime/entpatch" entuuid "github.com/protobuf-orm/protoc-gen-orm-ent/runtime/entuuid" codes "google.golang.org/grpc/codes" @@ -85,9 +83,6 @@ func (s SeatServiceServer) narrow(ctx context.Context, p predicate.Seat) (predic func (s SeatServiceServer) snapshot(ctx context.Context, p predicate.Seat, lock bool) (*ent.Seat, error) { q := s.Db.Seat.Query().Where(p) - q.WithHolder(func(q *ent.UserQuery) { q.Select(user.FieldId) }) - q.WithOrg(func(q *ent.TenantQuery) { q.Select(tenant.FieldId) }) - q.WithDeputy(func(q *ent.UserQuery) { q.Select(user.FieldId) }) q.Modify(func(q *sql.Selector) { if lock && s.Db.Dialect() != dialect.SQLite { q.ForUpdate() @@ -301,10 +296,6 @@ func SeatSelect(q *ent.SeatQuery, m *apptest.SeatSelect) { func SeatSelectInit(q *ent.SeatQuery, m *apptest.SeatSelect) { if m != nil { SeatSelect(q, m) - } else { - q.WithHolder(selectUserKey) - q.WithOrg(selectTenantKey) - q.WithDeputy(selectUserKey) } } diff --git a/internal/apptest/server/bare/shift.g.go b/internal/apptest/server/bare/shift.g.go index 2975a60..af53e58 100644 --- a/internal/apptest/server/bare/shift.g.go +++ b/internal/apptest/server/bare/shift.g.go @@ -215,7 +215,6 @@ func ShiftSelect(q *ent.ShiftQuery, m *apptest.ShiftSelect) { func ShiftSelectInit(q *ent.ShiftQuery, m *apptest.ShiftSelect) { if m != nil { ShiftSelect(q, m) - } else { } } diff --git a/internal/apptest/server/bare/tenant.g.go b/internal/apptest/server/bare/tenant.g.go index 3a70a61..60e86f4 100644 --- a/internal/apptest/server/bare/tenant.g.go +++ b/internal/apptest/server/bare/tenant.g.go @@ -236,7 +236,6 @@ func TenantSelect(q *ent.TenantQuery, m *apptest.TenantSelect) { func TenantSelectInit(q *ent.TenantQuery, m *apptest.TenantSelect) { if m != nil { TenantSelect(q, m) - } else { } } diff --git a/internal/apptest/server/bare/user.g.go b/internal/apptest/server/bare/user.g.go index 190f557..7bb8064 100644 --- a/internal/apptest/server/bare/user.g.go +++ b/internal/apptest/server/bare/user.g.go @@ -16,7 +16,6 @@ import ( apptest "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest" ent "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent" predicate "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/predicate" - tenant "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/tenant" user "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent/user" entpatch "github.com/protobuf-orm/protoc-gen-orm-ent/runtime/entpatch" entuuid "github.com/protobuf-orm/protoc-gen-orm-ent/runtime/entuuid" @@ -84,7 +83,6 @@ func (s UserServiceServer) narrow(ctx context.Context, p predicate.User) (predic func (s UserServiceServer) snapshot(ctx context.Context, p predicate.User, lock bool) (*ent.User, error) { q := s.Db.User.Query().Where(p) - q.WithTenant(func(q *ent.TenantQuery) { q.Select(tenant.FieldId) }) q.Modify(func(q *sql.Selector) { if lock && s.Db.Dialect() != dialect.SQLite { q.ForUpdate() @@ -282,8 +280,6 @@ func UserSelect(q *ent.UserQuery, m *apptest.UserSelect) { func UserSelectInit(q *ent.UserQuery, m *apptest.UserSelect) { if m != nil { UserSelect(q, m) - } else { - q.WithTenant(selectTenantKey) } } diff --git a/internal/apptest/server/bare/x-get_test.go b/internal/apptest/server/bare/x-get_test.go index 716b48a..edc091f 100644 --- a/internal/apptest/server/bare/x-get_test.go +++ b/internal/apptest/server/bare/x-get_test.go @@ -2,10 +2,15 @@ package bare_test import ( context "context" + "sync/atomic" "testing" "github.com/lesomnus/z" + "github.com/protobuf-orm/ent/dialect" + "github.com/protobuf-orm/ent/dialect/sql" pb "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest" + "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/ent" + "github.com/protobuf-orm/protoc-gen-orm-ent/internal/apptest/server/bare" "github.com/stretchr/testify/require" ) @@ -22,6 +27,22 @@ func TestGet(t *testing.T) { x.NotEmpty(w.GetTenant().GetId()) x.Equal(tenant.GetId(), w.GetTenant().GetId()) })) + t.Run("empty select answers an edge with its key and nothing else", T(func(ctx context.Context, x *require.Assertions, c *Client) { + tenant, err := c.Tenant().Add(ctx, pb.TenantAddRequest_builder{}.Build()) + x.NoError(err) + x.True(tenant.HasDateCreated()) + + v, err := c.User().Add(ctx, pb.UserAddRequest_builder{Tenant: tenant.Ref()}.Build()) + x.NoError(err) + + // A reference, and not the tenant read with its key alone: that + // answered every other field of it at its zero value, and a tenant + // created in the year 1. + w, err := c.User().Get(ctx, v.Pick()) + x.NoError(err) + x.Equal(tenant.GetId(), w.GetTenant().GetId()) + x.False(w.GetTenant().HasDateCreated()) + })) t.Run("select field of edge", T(func(ctx context.Context, x *require.Assertions, c *Client) { tenant, err := c.Tenant().Add(ctx, pb.TenantAddRequest_builder{ Alias: z.Ptr("foo"), @@ -47,3 +68,39 @@ func TestGet(t *testing.T) { x.Equal(tenant.GetAlias(), w.GetTenant().GetAlias()) })) } + +// A Get that selects nothing is one query, however many edges the row has: the +// row holds each neighbour's key, which is all such a Get answers for one. It +// was a query for the row and one more for each edge. +func TestAGetThatSelectsNothingIsOneQuery(t *testing.T) { + x := require.New(t) + ctx := t.Context() + + driver, err := sql.Open("sqlite3", "file:get-one-query?mode=memory&cache=shared&_fk=1") + x.NoError(err) + defer driver.Close() + driver.DB().SetMaxOpenConns(1) + + var n atomic.Int32 + db := ent.NewClient(ent.Driver(dialect.DebugWithContext(driver, func(context.Context, ...any) { n.Add(1) }))) + x.NoError(db.Schema.Create(ctx)) + s, err := bare.NewServer(db) + x.NoError(err) + + org, err := s.Tenant().Add(ctx, pb.TenantAddRequest_builder{}.Build()) + x.NoError(err) + holder, err := s.User().Add(ctx, pb.UserAddRequest_builder{Tenant: org.Ref(), Alias: z.Ptr("holder")}.Build()) + x.NoError(err) + deputy, err := s.User().Add(ctx, pb.UserAddRequest_builder{Tenant: org.Ref(), Alias: z.Ptr("deputy")}.Build()) + x.NoError(err) + seat, err := s.Seat().Add(ctx, pb.SeatAddRequest_builder{Holder: holder.Ref(), Org: org.Ref(), Deputy: deputy.Ref()}.Build()) + x.NoError(err) + + n.Store(0) + got, err := s.Seat().Get(ctx, seat.Pick()) + x.NoError(err) + x.EqualValues(1, n.Load(), "three edges, and one query") + x.Equal(holder.GetId(), got.GetHolder().GetId()) + x.Equal(org.GetId(), got.GetOrg().GetId()) + x.Equal(deputy.GetId(), got.GetDeputy().GetId()) +}