diff --git a/pkg/rain/model.go b/pkg/rain/model.go index f363d21..486bac9 100644 --- a/pkg/rain/model.go +++ b/pkg/rain/model.go @@ -334,20 +334,23 @@ func scanRowsAgainstTableDirect(rows *sql.Rows, dest any, table *schema.TableDef return err } } else { - // Re-derive a reflect.Value for the element to reset it and handle non-offset columns. - item := reflect.NewAt(elemType, ptr).Elem() - - // OPTIMIZATION: Skip zeroing existing elements if the plan is a full scan. - // This allows us to reuse existing pointer allocations in the struct fields. - if !plan.isFullScan { - // Reset existing element to its zero state before reuse to avoid data carry-over. - item.Set(zeroElem) - } - var targetVal reflect.Value if plan.needsTargetValue { - targetVal = item + // Re-derive a reflect.Value for the element to reset it and handle non-offset columns. + targetVal = reflect.NewAt(elemType, ptr).Elem() + + // OPTIMIZATION: Skip zeroing existing elements if the plan is a full scan. + // This allows us to reuse existing pointer allocations in the struct fields. + if !plan.isFullScan { + // Reset existing element to its zero state before reuse to avoid data carry-over. + targetVal.Set(zeroElem) + } + } else if !plan.isFullScan { + // Even if we don't need a reflect.Value for assignments, we still need it to zero the struct + // if this isn't a full scan to avoid data carry-over from previous rows. + reflect.NewAt(elemType, ptr).Elem().Set(zeroElem) } + if err := scanDirectRowAddr(ptr, targetVal, plan, scratch); err != nil { return err } @@ -1026,19 +1029,15 @@ func scanCachedRowsAgainstTable(result *cachedSelectRows, dest any, table *schem return err } - items := target + items := reflect.MakeSlice(target.Type(), 0, len(result.Rows)) for _, row := range result.Rows { var item reflect.Value - if pointerElems { - item = reflect.New(structType) - } else { - item = reflect.New(structType).Elem() - } - var scanTarget reflect.Value if pointerElems { + item = reflect.New(structType) scanTarget = item.Elem() } else { + item = reflect.New(structType).Elem() scanTarget = item } @@ -1226,9 +1225,11 @@ func newRowScanPlanForColumns(cols []string, modelType reflect.Type, table *sche plan.timeValueCols = append(plan.timeValueCols, colPlan) } } else { + plan.needsTargetValue = true plan.otherCols = append(plan.otherCols, colPlan) } default: + plan.needsTargetValue = true plan.otherCols = append(plan.otherCols, colPlan) } } else { diff --git a/pkg/rain/query_compile.go b/pkg/rain/query_compile.go index 5047f5a..6ec2d32 100644 --- a/pkg/rain/query_compile.go +++ b/pkg/rain/query_compile.go @@ -27,37 +27,33 @@ type compiledQuery struct { sql string argPlan []compiledArg hasNames bool + args []any } func (q compiledQuery) literalArgs() ([]any, error) { if q.hasNames { return nil, ErrPreparedArgsRequired } - - args := make([]any, 0, len(q.argPlan)) - for _, arg := range q.argPlan { - args = append(args, arg.value) - } - return args, nil + // OPTIMIZATION: Return a fresh copy of the pre-calculated arguments to avoid + // shared state footguns if the caller modifies the slice, while still + // avoiding the overhead of rebuilding the slice from argPlan. + return append([]any(nil), q.args...), nil } func (q compiledQuery) bind(args PreparedArgs) ([]any, error) { if !q.hasNames { - bound := make([]any, 0, len(q.argPlan)) - for _, arg := range q.argPlan { - bound = append(bound, arg.value) - } if len(args) > 0 { return nil, fmt.Errorf("rain: unexpected prepared args for query without placeholders") } - return bound, nil + // OPTIMIZATION: Return a fresh copy of the pre-calculated arguments to avoid + // shared state footguns if the caller modifies the slice. + return append([]any(nil), q.args...), nil } seen := make(map[string]struct{}, len(args)) - bound := make([]any, 0, len(q.argPlan)) - for _, arg := range q.argPlan { + bound := append([]any(nil), q.args...) + for i, arg := range q.argPlan { if arg.kind == compiledArgLiteral { - bound = append(bound, arg.value) continue } value, ok := args[arg.name] @@ -65,7 +61,7 @@ func (q compiledQuery) bind(args PreparedArgs) ([]any, error) { return nil, fmt.Errorf("rain: missing prepared arg %q", arg.name) } seen[arg.name] = struct{}{} - bound = append(bound, value) + bound[i] = value } for name := range args { if _, ok := seen[name]; ok { @@ -120,19 +116,32 @@ func (c *compileContext) String() string { } func (c *compileContext) compiledQuery() compiledQuery { - // OPTIMIZATION: Explicitly copy the argPlan slice so that the compileContext - // and its underlying array can be safely returned to the sync.Pool without - // causing data corruption for the caller of compiledQuery. - argPlan := append([]compiledArg(nil), c.argPlan...) + var hasNames bool + for _, arg := range c.argPlan { + if arg.kind == compiledArgNamedPlaceholder { + hasNames = true + break + } + } + + // OPTIMIZATION: Only copy the argPlan if the query has named placeholders. + // Queries with only literals can use the pre-calculated args slice. + var argPlan []compiledArg + if hasNames { + argPlan = append([]compiledArg(nil), c.argPlan...) + } compiled := compiledQuery{ - sql: c.String(), - argPlan: argPlan, + sql: c.String(), + argPlan: argPlan, + hasNames: hasNames, } - for _, arg := range compiled.argPlan { - if arg.kind == compiledArgNamedPlaceholder { - compiled.hasNames = true - break + if len(c.argPlan) > 0 { + compiled.args = make([]any, len(c.argPlan)) + for i, arg := range c.argPlan { + if arg.kind == compiledArgLiteral { + compiled.args[i] = arg.value + } } } return compiled diff --git a/pkg/rain/query_select.go b/pkg/rain/query_select.go index 23ab5a7..bea63b2 100644 --- a/pkg/rain/query_select.go +++ b/pkg/rain/query_select.go @@ -975,11 +975,10 @@ func (q *SelectQuery) compileExists() (compiledQuery, error) { } func wrapExistsCompiled(compiled compiledQuery) (compiledQuery, error) { - existsQuery := compiledQuery{ - sql: "SELECT EXISTS(" + compiled.sql + ")", - argPlan: make([]compiledArg, len(compiled.argPlan)), - hasNames: compiled.hasNames, - } - copy(existsQuery.argPlan, compiled.argPlan) - return existsQuery, nil + // NOTE: This shallow copies the input compiledQuery and wraps the SQL. + // The argPlan and args slices are shared with the original. This is safe + // because compileExists (the only caller) does not use the original after + // this call. + compiled.sql = "SELECT EXISTS(" + compiled.sql + ")" + return compiled, nil }