Обсуждение: pgsql: Don't Memoize lateral joins with volatile join conditions
Don't Memoize lateral joins with volatile join conditions The use of Memoize was already disabled in normal joins when the join conditions had volatile functions per the code in match_opclause_to_indexcol(). Ordinarily, the parameterization for the inner side of a nested loop will be an Index Scan or at least eventually lead to an index scan (perhaps nested several joins deep). However, for lateral joins, that's not the case and seq scans can be parameterized too, so we can't rely on match_opclause_to_indexcol(). Here we explicitly check the parameterization for volatile functions and don't consider the generation of a Memoize path when such functions are present. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs49nHFnHbpepLsv_yF3qkpCS4BdB-v8HoJVv8_=Oat0u_w@mail.gmail.com Backpatch-through: 14, where Memoize was introduced Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/990c3650c535f6fc84d53abb0bd03ad0dd16c576 Modified Files -------------- src/backend/optimizer/path/joinpath.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
David Rowley <drowley@postgresql.org> writes:
> Don't Memoize lateral joins with volatile join conditions
Is this really something to be pushing into stable branches less than
12 hours before a release wrap? We don't normally take such risks
for anything but security patches, which this isn't. There will not,
for example, be time for full buildfarm coverage.
regards, tom lane
On Tue, 8 Aug 2023 at 01:02, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > David Rowley <drowley@postgresql.org> writes: > > Don't Memoize lateral joins with volatile join conditions > > Is this really something to be pushing into stable branches less than > 12 hours before a release wrap? We don't normally take such risks > for anything but security patches, which this isn't. There will not, > for example, be time for full buildfarm coverage. My apologies. I had been under the impression that the go-no window was between stamp and tag and *that* was the window that was used for a buildfarm cycle. It sounds like it's not quite as black and white as that. I'm not aware of any documentation which gives guidance on this. I'm only aware of: * https://wiki.postgresql.org/wiki/Release_process * https://wiki.postgresql.org/wiki/Committing_checklist Is there somewhere else I should look? or is this something that can maybe be improved? (I'm assuming backing the patch out now won't improve the situation, but please correct me if you think I'm wrong on that) Thanks David
David Rowley <dgrowleyml@gmail.com> writes: > On Tue, 8 Aug 2023 at 01:02, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Is this really something to be pushing into stable branches less than >> 12 hours before a release wrap? > My apologies. I had been under the impression that the go-no window > was between stamp and tag and *that* was the window that was used for > a buildfarm cycle. It sounds like it's not quite as black and white as > that. I'm not aware of any documentation which gives guidance on > this. Hmm. Customarily we avoid pushing stuff into due-for-release branches beginning 48-72 hours before the wrap, and then continuing until the tags are applied ~24 hours after wrap. The pause before wrap is to allow a complete buildfarm test cycle even on slower machines. The pause after wrap is to wait for feedback from packagers. > * https://wiki.postgresql.org/wiki/Committing_checklist I thought we had something written down there after the last discussion of this point, but maybe it was hanging fire because we hadn't chosen a specific definition of when to start the pause. > (I'm assuming backing the patch out now won't improve the situation, > but please correct me if you think I'm wrong on that) No, I've already made the tarballs. I did look closely at the patch and decided it was probably harmless and not in need of reversion. I'm just whining because this wasn't following process. regards, tom lane
On Tue, 8 Aug 2023 at 10:06, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > David Rowley <dgrowleyml@gmail.com> writes: > > * https://wiki.postgresql.org/wiki/Committing_checklist > > I thought we had something written down there after the last > discussion of this point, but maybe it was hanging fire because > we hadn't chosen a specific definition of when to start the pause. Probably something wider to provide some margin is likely better than it not being documented at all. > I'm just whining because this wasn't following process. I understand. I certainly don't want to be the cause of the destabilisation of a release. It's just frustrating when trying to follow the process but the lack of documentation about what that process is just leaves it to trial/error/someone complaining. David