Re: Wrong unsafe-flag test in check_output_expressions()

Richard Guo <guofenglinux(at)gmail(dot)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