Adding agg_spill_mem parameter.

[fengttt]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #3433

What this PR does / why we need it:

Try to add a spill mem session var to config spill threshold. right now it is NOT auto tuned, but this can be done at
either plan time (we know global resource) or run time (we know real time resource). But later.

---

PR Type

Enhancement

---

Description

• Add agg_spill_mem session variable to configure aggregate spill threshold

• Propagate agg_spill_mem from query builder through plan nodes to runtime operators

• Define memory size constants (KiB, MiB, GiB, TiB, PiB) in common package

• Update protobuf definitions to include spill_mem field in Group and Node messages

---

Diagram Walkthrough

flowchart LR
  A["Session Variable: agg_spill_mem"] --> B["QueryBuilder"]
  B --> C["Plan Nodes"]
  C --> D["Protobuf Messages"]
  D --> E["Runtime Operators"]
  E --> F["Group Operator"]

Loading

File Walkthrough

	Relevant files
Configuration changes	1 files variables.go Register new `agg_spill_mem` session variable with default 1GiB +9/-0
Enhancement	14 files const.go Add memory size constants (KiB through PiB)                            +23/-0    types.go Add `aggSpillMem` field to QueryBuilder struct                      +3/-0      query_builder.go Initialize `aggSpillMem` from session variable in QueryBuilder +29/-0    build_dml_util.go Set `SpillMem` on aggregate nodes in DML operations            +3/-0      distinct_agg.go Set `SpillMem` on aggregate nodes for distinct optimization +1/-0      agg_pushdown_pullup.go Set `SpillMem` on aggregate nodes during pushdown optimization +1/-0      opt_misc.go Set `SpillMem` when rewriting DISTINCT to aggregate            +1/-0      build_insert.go Set `SpillMem` on aggregate nodes in INSERT plans                +1/-0      build_constraint_util.go Set `SpillMem` on aggregate nodes in constraint plans        +1/-0      plan.proto Add `spill_mem` field to Node message definition                  +3/-0      pipeline.proto Add `spill_mem` field to Group message definition                +1/-0      types.go Add `SpillMem` field to Group operator struct                        +3/-2      operator.go Copy `SpillMem` when duplicating and constructing Group operators +2/-0      remoterun.go Convert `SpillMem` between pipeline and VM operator representations +2/-0
Bug fix	1 files compiler_context.go Fix return statement in `GetLowerCaseTableNames` method    +1/-1
Additional files	2 files pipeline.pb.go +292/-256 plan.pb.go +834/-795

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Type assertion risk Description: Changed defaulting logic returns literal 1 instead of assigning and returning the session variable value, which can cause a panic if a non-int64 value is present (type assertion on line 82) or logic errors due to unexpected defaulting. compiler_context.go [77-82] Referred Code defer tcc.mu.Unlock() val, err := tcc.execCtx.ses.GetSessionSysVar("lower_case_table_names") if err != nil || val == nil { return 1 } return val.(int64)
Ticket Compliance
🟡	🎫 #3433 🟢 Provide a way to configure the memory budget/threshold. Ensure the configured threshold is available from planning through to runtime so multi-CN execution can honor it. 🔴 Hash join and hash aggregate must track memory usage. When memory usage exceeds a configured budget/threshold, the operator must spill.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider a more generic spill parameter Instead of the aggregation-specific agg_spill_mem parameter, consider a more generic one like operator_spill_mem. This would be more extensible for future memory-intensive operators like hash joins. Examples: pkg/frontend/variables.go [3805-3812] "agg_spill_mem": { Name: "agg_spill_mem", Scope: ScopeBoth, Dynamic: true, SetVarHintApplies: false, Type: InitSystemVariableIntType("agg_spill_mem", 0, common.TiB, false), Default: int64(common.GiB), }, pkg/sql/plan/query_builder.go [71-77] var aggSpillMem int64 aggSpillMemInt, err := ctx.ResolveVariable("agg_spill_mem", true, false) if err == nil { if aggSpillMemVal, ok := aggSpillMemInt.(int64); ok { aggSpillMem = aggSpillMemVal } } Solution Walkthrough: Before: // pkg/frontend/variables.go var gSysVarsDefs = map[string]SystemVariable{ "agg_spill_mem": { Name: "agg_spill_mem", // ... specific to aggregation }, } // pkg/sql/plan/query_builder.go type QueryBuilder struct { aggSpillMem int64 } func (builder *QueryBuilder) appendAggNode(...) { aggNode := &plan.Node{ SpillMem: builder.aggSpillMem, } } After: // pkg/frontend/variables.go var gSysVarsDefs = map[string]SystemVariable{ "operator_spill_mem": { // or a more generic name Name: "operator_spill_mem", // ... generic for all spillable operators }, } // pkg/sql/plan/query_builder.go type QueryBuilder struct { operatorSpillMem int64 } func (builder *QueryBuilder) appendAggNode(...) { aggNode := &plan.Node{ SpillMem: builder.operatorSpillMem, } } // In the future, for other operators: func (builder *QueryBuilder) appendJoinNode(...) { joinNode := &plan.Node{ SpillMem: builder.operatorSpillMem, // Reuse the same parameter } } Suggestion importance[1-10]: 8 __ Why: This is a significant design suggestion that improves extensibility, as the related ticket mentions both hash aggregation and hash join spilling, but the PR only implements a specific agg_spill_mem parameter.	Medium
Possible issue	Handle variable resolution errors gracefully Explicitly set aggSpillMem to its default value of 1 GiB if resolving the variable fails or the type assertion is unsuccessful, instead of silently defaulting to 0. pkg/sql/plan/query_builder.go [71-77] var aggSpillMem int64 aggSpillMemInt, err := ctx.ResolveVariable("agg_spill_mem", true, false) -if err == nil { +if err != nil { + aggSpillMem = int64(common.GiB) +} else { if aggSpillMemVal, ok := aggSpillMemInt.(int64); ok { aggSpillMem = aggSpillMemVal + } else { + aggSpillMem = int64(common.GiB) } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that failing to resolve the agg_spill_mem variable results in a zero value, which contradicts the defined default of 1 GiB and could lead to unexpected behavior or performance issues.	Medium
Update