Skip to content

BUG (P2): @QueryStorePriority OLTP_LIGHT preset dead code — IS NULL gate inert with non-NULL default #335

@nanoDBA

Description

@nanoDBA

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 = NULL get 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 (declared N'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
END

T-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 default

This 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
END

This 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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-qualityRefactoring, dead code removal, maintainability, or internal correctnessp2-importantSemantic gap, missing warning, or undocumented behavior that affects correctness

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions