| From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
|---|---|
| To: | Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Wrong unsafe-flag test in check_output_expressions() |
| Date: | 2026-06-01 08:27:06 |
| Message-ID: | CAMbWs49Q_xnF_P2QSUyDzJ34MnrO7dh-cUAaK2HJPgSgh88NcA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-wrong-unsafe-flag-test-in-check_output_expres.patch | application/octet-stream | 2.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ewan Young | 2026-06-01 08:41:35 | Re: autovacuum launcher crash: assert in pgstat_count_io_op (IOOP_EXTEND on pg_database's VM) |
| Previous Message | ZizhuanLiu X-MAN | 2026-06-01 08:01:58 | Re: [PATCH]Refactor and unify expression construction functions in makefuncs.c |