Wrong unsafe-flag test in check_output_expressions()

Technical Problem

This thread addresses a subtle bug in PostgreSQL's query optimizer, specifically in the check_output_expressions() function which is part of the subquery pullup / qual pushdown machinery. The bug involves a wrong flag being tested in a guard condition, though fortunately it doesn't produce incorrect query plans.

Context: Subquery Pullup and Safety Flags

When PostgreSQL's optimizer considers pulling up a subquery or pushing down qual conditions past certain query constructs (window functions, DISTINCT ON, GROUP BY), it must determine whether each output expression of the subquery is "safe" for such transformations. The safetyInfo->unsafeFlags[] array tracks per-target-list-entry reasons why an expression might be unsafe to push down.

The relevant flags include:

• UNSAFE_NOTIN_DISTINCTON_CLAUSE — the expression doesn't appear in a DISTINCT ON clause
• UNSAFE_NOTIN_PARTITIONBY_CLAUSE — the expression doesn't appear in all PARTITION BY clauses of window functions

The Bug

In the code that checks "point 4" (window function partition clause membership), the guard condition tests the wrong flag:

if (subquery->hasWindowFuncs &&
    (safetyInfo->unsafeFlags[tle->resno] &
     UNSAFE_NOTIN_DISTINCTON_CLAUSE) == 0 &&   /* BUG: wrong flag */
    !targetIsInAllPartitionLists(tle, subquery))
{
    safetyInfo->unsafeFlags[tle->resno] |= UNSAFE_NOTIN_PARTITIONBY_CLAUSE;
    continue;
}

The guard should test UNSAFE_NOTIN_PARTITIONBY_CLAUSE (the same flag it sets), serving as an idempotency optimization — if the flag is already set, there's no need to call targetIsInAllPartitionLists() again.

Why It Doesn't Produce Wrong Plans

Richard Guo's analysis identifies two scenarios:

1. UNSAFE_NOTIN_PARTITIONBY_CLAUSE already set, UNSAFE_NOTIN_DISTINCTON_CLAUSE not set: The guard fails to short-circuit, targetIsInAllPartitionLists() is called unnecessarily, but re-setting the same bit is a no-op. Result: wasted computation, correct outcome.

2. UNSAFE_NOTIN_DISTINCTON_CLAUSE already set: Point 4 is skipped entirely, so UNSAFE_NOTIN_PARTITIONBY_CLAUSE is never set. However, the column is already marked unsafe via UNSAFE_NOTIN_DISTINCTON_CLAUSE, which is a stronger restriction that already prevents pushdown. Result: correct outcome (the union of unsafe flags still blocks the pushdown).

This is a classic example of a "latent bug" — code that is logically wrong but doesn't manifest due to the broader invariants of the system.

The Fix

The fix is a one-line change: replace the tested flag from UNSAFE_NOTIN_DISTINCTON_CLAUSE to UNSAFE_NOTIN_PARTITIONBY_CLAUSE in the guard condition. This makes the pattern consistent with other checks in the same loop, where each check guards on the same bit it intends to set.

Design Pattern

The loop in check_output_expressions() follows a standard pattern for each safety check:

if (condition_applies &&
    (unsafeFlags[resno] & THIS_SPECIFIC_FLAG) == 0 &&  /* skip if already set */
    !expression_passes_safety_check())
{
    unsafeFlags[resno] |= THIS_SPECIFIC_FLAG;
    continue;
}

The guard (unsafeFlags[resno] & FLAG) == 0 is an optimization to avoid expensive checks (like targetIsInAllPartitionLists()) when the flag is already known to be set from a previous iteration or earlier logic. Each check should guard on its own flag for this pattern to work correctly.

Historical Context

The buggy check was introduced in PostgreSQL v15, likely as part of the work that added or refactored the unsafe-flags infrastructure for qual pushdown past window functions. Richard Guo discovered it while working on a related patch fixing qual pushdown past window functions with mismatched opfamily/collation — a more substantive bug in the same area of code.

Resolution

The fix was pushed and back-patched through v15 (where the bug originated). Despite not being a correctness bug, David Rowley advocated for backpatching to keep the code clean across all supported versions, avoiding potential confusion for future developers reading the code in older branches.