-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Summary
The @QueryStorePriority parameter is declared with a default of N'N':
@QueryStorePriority NVARCHAR(20) = N'N'The OLTP_LIGHT preset block attempts to enable Query Store integration via:
IF @QueryStorePriority IS NULL
SET @QueryStorePriority = N'Y';A NVARCHAR default of N'N' is never NULL at execution time unless the caller explicitly passes NULL. The OLTP_LIGHT preset's Query Store integration has never fired for any caller who invokes @Preset = N'OLTP_LIGHT' without also explicitly passing @QueryStorePriority = NULL.
Impact
- OLTP_LIGHT preset silently ignores Query Store. Users who read the documentation, select the OLTP_LIGHT preset expecting integrated QS behavior, and don't know to also pass
@QueryStorePriority = NULLget a preset that omits one of its documented behaviors. - Documentation gap compounds the defect. The IS NULL sentinel contract is undocumented — callers have no mechanism to express "use the preset's value" vs. "explicitly disable QS" vs. "use the default" using NULL alone.
- All other preset parameters with NULL defaults (lines 363–376) are NOT affected. The four NULL-default parameters implement a valid "caller wins with non-NULL" sentinel pattern — those IS NULL gates are semantically correct. Only
@QueryStorePriority(declaredN'N', non-NULL) is broken.
Root Cause
-- DECLARATION (line ~363)
@QueryStorePriority NVARCHAR(20) = N'N' -- ← non-NULL default; IS NULL gate below is dead
-- OLTP_LIGHT preset block (~line 377)
IF @Preset = N'OLTP_LIGHT'
BEGIN
-- ... other preset assignments ...
IF @QueryStorePriority IS NULL -- ← never TRUE when caller uses default N'N'
SET @QueryStorePriority = N'Y';
-- @QueryStorePriority remains N'N' — QS integration never activates
ENDT-SQL has no mechanism to distinguish an omitted parameter (uses default) from an explicitly passed NULL. The IS NULL sentinel is the least-bad available pattern for a 55-parameter tool — it is correct for NULL-default parameters. For @QueryStorePriority, the fix is to change the default, not the pattern.
Fix
Option A (Preferred): Change declared default from N'N' to NULL
-- Line ~363
@QueryStorePriority NVARCHAR(20) = NULL -- was: N'N'The existing IS NULL gate in the preset block then fires correctly. Add a downstream NULL-to-default assignment before QS logic executes:
-- After all preset blocks, before QS branch:
IF @QueryStorePriority IS NULL
SET @QueryStorePriority = N'N'; -- explicit runtime defaultThis preserves existing behavior for non-preset callers (NULL → N'N' = disabled) while enabling the preset to override correctly.
Option B: Change the preset gate to unconditional SET
IF @Preset = N'OLTP_LIGHT'
BEGIN
SET @QueryStorePriority = N'Y'; -- unconditional; no IS NULL guard
ENDThis makes the preset authoritative but removes the caller's ability to override a preset value by passing a parameter. Option A is preferred as it preserves the caller-wins contract established by the other parameters.
Documentation Fix (Both Options)
At line ~348 (parameter block header) and lines 576–577 (QS section), document the IS NULL sentinel contract:
-- PRESET OVERRIDE CONTRACT:
-- Parameters with NULL default: caller passes non-NULL to override preset value.
-- Caller passes NULL (or omits) → preset value applies.
-- @QueryStorePriority: declared default NULL (after this fix).
-- Caller omits → preset value applies (or N'N' outside preset context).
-- Caller passes N'Y' or N'N' → caller value applies, overrides preset.
Scope Clarification
Initial analysis flagged all 5 IS NULL conditionals in the @Preset block as broken. This issue is narrower: only @QueryStorePriority is defective. The 4 NULL-default parameters (@MaxDOP, @LockTimeout, @StatisticsSample, @MaxSecondsPerStat) implement correct sentinel-null behavior and should not be changed.
Testing Requirements
- Validate
EXEC sp_StatUpdate @Preset = N'OLTP_LIGHT'results in@QueryStorePriority = N'Y'(currently fails — use this as regression test) - Validate caller override:
EXEC sp_StatUpdate @Preset = N'OLTP_LIGHT', @QueryStorePriority = N'N'→ QS disabled (caller wins) - Validate non-preset callers omitting
@QueryStorePriority→ default N'N' behavior unchanged - Validate explicit NULL passthrough:
EXEC sp_StatUpdate @QueryStorePriority = NULL→ N'N' (matches old default behavior) - Multi-version matrix: no version gate (parameter declaration change only)
References
- Internal analysis: Preset IS NULL vs. unconditional SET — scope adjudication
- Claim-062: @QueryStorePriority IS NULL gate is dead code (❌ Bug Fix confirmed)
- Claim-063: Four NULL-default params are NOT broken (✅ Correct implementation)
- Claim-212: IS NULL sentinel creates undocumented one-directional caller constraint
- Claim-213: T-SQL cannot distinguish omit from explicit-NULL — sentinel-null is least-bad pattern
- Claim-214/215: Documentation fix locations (line 348, 576–577)
- Related:
issue-p3f-preset-help-visibility.md(@preset as first-run entry point)