planner: Missing initialization for EnableSemiJoinRewrite in NewSessionVars (release-8.5)
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
The \`EnableSemiJoinRewrite\` field in \`SessionVars\` struct is not initialized in the \`NewSessionVars()\` function, while it has a corresponding default constant \`DefOptEnableSemiJoinRewrite\` defined.
## Location
**File**: \`pkg/sessionctx/variable/session.go\`
**Branch**: \`release-8.5\`
**Function**: \`NewSessionVars()\`
**Affected Field**: \`EnableSemiJoinRewrite\`
## Current State
\`\`\`go
// Field declaration exists (line ~1115)
type SessionVars struct {
// EnableSemiJoinRewrite enables the SEMI_JOIN_REWRITE hint for subqueries in the where clause.
EnableSemiJoinRewrite bool
}
// Default constant exists (tidb_vars.go)
const DefOptEnableSemiJoinRewrite = false
// System variable registration exists (sysvar.go)
{Name: TiDBOptEnableSemiJoinRewrite, Value: BoolToOnOff(DefOptEnableSemiJoinRewrite), ...}
// ❌ BUT: Missing initialization in NewSessionVars()
func NewSessionVars(hctx HookContext) *SessionVars {
vars := &SessionVars{
EnableOuterJoinReorder: DefTiDBEnableOuterJoinReorder,
EnableNoDecorrelateInSelect: DefOptEnableNoDecorrelateInSelect,
// EnableSemiJoinRewrite: DefOptEnableSemiJoinRewrite, // ← MISSING!
RetryLimit: DefTiDBRetryLimit,
DisableTxnAutoRetry: DefTiDBDisableTxnAutoRetry,
...
}
}
\`\`\`
## Why This is a Bug
While the field currently works because:
1. Go's zero value for \`bool\` is \`false\`
2. \`DefOptEnableSemiJoinRewrite\` is currently \`false\`
3. So the zero value happens to match the expected default
This is still a bug because:
1. **Code Inconsistency**: All other optimizer variables with \`Def*\` constants are explicitly initialized
2. **Violates Code Convention**: The codebase pattern is to explicitly initialize all fields that have default constants
3. **Potential Future Bug**: If someone changes \`DefOptEnableSemiJoinRewrite\` to \`true\` in the future, new sessions will still get \`false\` (zero value), causing unexpected behavior
4. **Maintainability**: It's unclear whether the missing initialization is intentional or an oversight
## Impact
### Current Impact: Low
- Functionality works correctly because zero value matches default value
- No runtime errors or incorrect behavior observed
### Future Risk: High
If \`DefOptEnableSemiJoinRewrite\` is changed to \`true\`:
\`\`\`go
const DefOptEnableSemiJoinRewrite = true // Changed from false
\`\`\`
Then:
- **Global variable** would show: \`true\` ✅
- **New sessions** would get: \`false\` (uninitialized) ❌
- **Result**: Inconsistent behavior between global setting and new session default
## Proposed Fix
Add the missing initialization in \`NewSessionVars()\`:
\`\`\`diff
func NewSessionVars(hctx HookContext) *SessionVars {
vars := &SessionVars{
EnableOuterJoinReorder: DefTiDBEnableOuterJoinReorder,
EnableNoDecorrelateInSelect: DefOptEnableNoDecorrelateInSelect,
+ EnableSemiJoinRewrite: DefOptEnableSemiJoinRewrite,
RetryLimit: DefTiDBRetryLimit,
DisableTxnAutoRetry: DefTiDBDisableTxnAutoRetry,
...
}
}
\`\`\`
## Related PRs
- PR #63540: Added \`EnableSemiJoinRewrite\` field and its initialization
- PR #63541: Cherry-pick that accidentally removed the initialization
### 4. What is your TiDB version? (Required)
v8.5.4-pre
Contributor guide
Assessment
This issue has not been assessed yet.