⚡ Bolt: reduce heap allocations in query builders

[cungminh2710]

Reduced heap allocations in Rain ORM query builders. replaced pointer-based LIMIT/OFFSET with primitive values and de-virtualized the primary table source in SelectQuery to avoid interface boxing. confirmed performance improvement via benchmarks and verified correctness with the full test suite.

---

PR created automatically by Jules for task 16792089427694480247 started by @cungminh2710

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[greptile-apps]

Greptile Summary

This PR eliminates heap allocations in the Rain ORM query builders by replacing pointer-based limit/offset fields with inline int + bool sentinel pairs, and by de-virtualizing the primary FROM table source in SelectQuery from a selectTableSource interface to a concrete *schema.TableDef + tableSubquery/tableAlias trio.

• All guard paths (isBareCompound, compile, compileAggregate, writeSQL) are correctly updated to check both table and tableSubquery; the new writeTableSourceSQL helper deduplicates subquery rendering across both compile paths.
• clone() correctly deep-copies tableSubquery while the primitive limit/offset values copy by value automatically, removing the old pointer-duplication boilerplate.
• DeleteQuery and UpdateQuery gain dead offset/hasOffset fields (no setters exist) that could be dropped in favour of literal 0, false at the call site.

Confidence Score: 5/5

Safe to merge — all changed paths produce identical SQL output and the refactoring is well-guarded throughout.

The de-virtualization of the table source is correctly propagated across every branch (isBareCompound, compile, compileAggregate, writeSQL, clone, relation loading). The limit/offset pointer-to-value conversion faithfully preserves semantics. No functional regressions were identified.

No files require special attention.

Important Files Changed

Filename	Overview
pkg/rain/query_select.go	Major refactor: table source split into table *schema.TableDef + tableSubquery/tableAlias; limit/offset pointers replaced with int+bool sentinels. All guard checks, clone, isBareCompound, compile, writeSQL, and compileAggregate correctly updated. writeTableSourceSQL introduced to deduplicate subquery rendering.
pkg/rain/query_common.go	Removed tableDefFromSelectSource (no longer needed) and updated writeOrderLimit signature from *int to int+bool. Logic preserved faithfully.
pkg/rain/query_delete.go	limit/offset changed from pointer to value+bool sentinel; offset/hasOffset fields are dead (no setter), but behavior is unchanged since they're always zero/false.
pkg/rain/query_update.go	Same limit/offset pointer-to-value change as DeleteQuery; SQLite FROM+ORDER/LIMIT guard correctly updated to use q.hasLimit; dead offset/hasOffset fields present.
pkg/rain/relation_loading.go	Updated to use concrete q.table field directly instead of type-asserting the old selectTableSource interface; internal query construction simplified.
pkg/rain/query_common_internal_test.go	Test updated to use new SelectQuery.table field directly (dropping tableDefSource{} wrapper); tableDefFromSelectSource test cases removed.
pkg/rain/query_compile_internal_test.go	All test fixtures updated from tableDefSource{table: ...} to users.TableDef() to match the new concrete field type.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    SQ[SelectQuery] --> |table != nil| TBL["*schema.TableDef\n(direct, no boxing)"]
    SQ --> |tableSubquery != nil| TSUB["*SelectQuery\n(subquery source)"]
    TSUB --> ALIAS["tableAlias string"]
    SQ --> WTSS["writeTableSourceSQL()"]
    WTSS --> |table != nil| CTX_TABLE["ctx.writeTable(table)"]
    WTSS --> |tableSubquery != nil| CTX_SUB["'(' + subquery.writeSQL + ') AS alias'"]
    SQ --> LIM["limit int\nhasLimit bool"]
    SQ --> OFF["offset int\nhasOffset bool"]
    LIM --> WOL["writeOrderLimit()"]
    OFF --> WOL
    WOL --> |"hasLimit || hasOffset"| FEAT["feature check\nthen LimitOffset(l, o)"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    SQ[SelectQuery] --> |table != nil| TBL["*schema.TableDef\n(direct, no boxing)"]
    SQ --> |tableSubquery != nil| TSUB["*SelectQuery\n(subquery source)"]
    TSUB --> ALIAS["tableAlias string"]
    SQ --> WTSS["writeTableSourceSQL()"]
    WTSS --> |table != nil| CTX_TABLE["ctx.writeTable(table)"]
    WTSS --> |tableSubquery != nil| CTX_SUB["'(' + subquery.writeSQL + ') AS alias'"]
    SQ --> LIM["limit int\nhasLimit bool"]
    SQ --> OFF["offset int\nhasOffset bool"]
    LIM --> WOL["writeOrderLimit()"]
    OFF --> WOL
    WOL --> |"hasLimit || hasOffset"| FEAT["feature check\nthen LimitOffset(l, o)"]

Loading

_{Reviews (2): Last reviewed commit: "perf(rain): reduce heap allocations and ..." | Re-trigger Greptile}

[cungminh2710]

@jules Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

Issue 1 of 2

pkg/rain/query_select.go:519-538
Subquery table rendering duplicated across two codepaths

The block that writes a subquery table source (alias validation → ( → inner writeSQL → ) AS alias) now appears identically in both writeSQL (lines ~519–535) and compileAggregate (lines ~981–998). Before this PR the logic was factored into subqueryTableSource.writeSQL; inlining it in two places means a future change to the alias quoting or subquery wrapping must be made in two locations, and it is easy for them to drift. A small private helper such as writeSubqueryTableSource(ctx) error would restore the single-definition invariant without reintroducing interface boxing.

Issue 2 of 2

pkg/rain/query_select.go:184-188
Offset has no hasOffset sentinel, unlike Limit which gained hasLimit

Limit now tracks presence with a hasLimit bool, but Offset relies on the zero value (0) to mean "not set". This makes Offset(0) indistinguishable from "Offset never called": writeCompoundOperandSQL won't add parentheses (q.offset > 0 is false), and isBareCompound will still return true. While OFFSET 0 is semantically a no-op, the asymmetry between the two setters is surprising and could cause silent breakage if a caller deliberately passes 0 to reset or neutralise an offset that was cloned from a base query.

[google-labs-jules]

@jules Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

Issue 1 of 2

pkg/rain/query_select.go:519-538
Subquery table rendering duplicated across two codepaths

The block that writes a subquery table source (alias validation → ( → inner writeSQL → ) AS alias) now appears identically in both writeSQL (lines ~519–535) and compileAggregate (lines ~981–998). Before this PR the logic was factored into subqueryTableSource.writeSQL; inlining it in two places means a future change to the alias quoting or subquery wrapping must be made in two locations, and it is easy for them to drift. A small private helper such as writeSubqueryTableSource(ctx) error would restore the single-definition invariant without reintroducing interface boxing.

Issue 2 of 2

pkg/rain/query_select.go:184-188
Offset has no hasOffset sentinel, unlike Limit which gained hasLimit

Limit now tracks presence with a hasLimit bool, but Offset relies on the zero value (0) to mean "not set". This makes Offset(0) indistinguishable from "Offset never called": writeCompoundOperandSQL won't add parentheses (q.offset > 0 is false), and isBareCompound will still return true. While OFFSET 0 is semantically a no-op, the asymmetry between the two setters is surprising and could cause silent breakage if a caller deliberately passes 0 to reset or neutralise an offset that was cloned from a base query.

Thank you for the detailed feedback. I've addressed both issues:

1. Factored out subquery table rendering into a private writeTableSourceSQL helper in SelectQuery. This eliminates the code duplication in writeSQL and compileAggregate while maintaining the performance benefits of devirtualization.
2. Added a hasOffset bool sentinel to SelectQuery, UpdateQuery, and DeleteQuery to match the implementation of hasLimit. This ensures Offset(0) is correctly distinguished from an unset offset and properly handled in compound queries and cloning logic.

Verified the fixes with the full test suite and confirmed that performance improvements are preserved.