Re: Wrong unsafe-flag test in check_output_expressions()

Richard Guo <guofenglinux@gmail.com> 于2026年6月1日周一 16:27写道：
>
> I happened to notice $subject when working on a bug-fix near-by.
>
>    /* If subquery uses window functions, check point 4 */
>    if (subquery->hasWindowFuncs &&
>        (safetyInfo->unsafeFlags[tle->resno] &
>         UNSAFE_NOTIN_DISTINCTON_CLAUSE) == 0 &&
>        !targetIsInAllPartitionLists(tle, subquery))
>    {
>        /* not present in all PARTITION BY clauses, so mark it unsafe */
>        safetyInfo->unsafeFlags[tle->resno] |= UNSAFE_NOTIN_PARTITIONBY_CLAUSE;
>        continue;
>    }
>
> So point 4 tests UNSAFE_NOTIN_DISTINCTON_CLAUSE while setting
> UNSAFE_NOTIN_PARTITIONBY_CLAUSE.  This does not seem correct to me.
> Each check in this loop should guard on the same bit it is about to
> set, as an idempotency optimization.

I don't know too much about this code, but per the nearby comments,
it seems UNSAFE_NOTIN_PARTITIONBY_CLAUSE is the correct flag to use.

>
> Fortunately, this does not lead to wrong plans.  When
> UNSAFE_NOTIN_PARTITIONBY_CLAUSE is already set but
> UNSAFE_NOTIN_DISTINCTON_CLAUSE is not, the guard fails to skip
> targetIsInAllPartitionLists() and recomputes it, but setting the same
> bit again changes nothing.  When UNSAFE_NOTIN_DISTINCTON_CLAUSE is
> already set, point 4 is skipped and UNSAFE_NOTIN_PARTITIONBY_CLAUSE is
> left unset; but such a column is already unsafe for pushdown via
> UNSAFE_NOTIN_DISTINCTON_CLAUSE, so the outcome is unchanged.
>
> Attached is the one-line fix.

Yes, your analysis is correct.
+1

-- 
Thanks,
Tender Wang