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. 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. - Richard
v1-0001-Fix-wrong-unsafe-flag-test-in-check_output_expres.patch
Description: Binary data
