On Fri, Jun 18, 2021 at 11:24:00AM -0400, Tom Lane wrote:
> Julien Rouhaud writes:
> > On Fri, Jun 18, 2021 at 10:24:20AM -0400, Tom Lane wrote:
> >> Maybe "if true, pstmt's node tree must not be modified" ?
>
> > Thanks, I find it way better!
>
> OK, pushed that way, and with a couple other com
Julien Rouhaud writes:
> On Fri, Jun 18, 2021 at 10:24:20AM -0400, Tom Lane wrote:
>> Maybe "if true, pstmt's node tree must not be modified" ?
> Thanks, I find it way better!
OK, pushed that way, and with a couple other comment tweaks from
an additional re-reading.
rega
On Fri, Jun 18, 2021 at 10:24:20AM -0400, Tom Lane wrote:
> Julien Rouhaud writes:
> > On Thu, Jun 17, 2021 at 01:03:29PM -0400, Tom Lane wrote:
> > + * readOnlyTree: treat pstmt's node tree as read-only
>
> > Maybe it's because I'm not a native english speaker, or because it's quite
> > late her
Julien Rouhaud writes:
> On Thu, Jun 17, 2021 at 01:03:29PM -0400, Tom Lane wrote:
> + * readOnlyTree: treat pstmt's node tree as read-only
> Maybe it's because I'm not a native english speaker, or because it's quite
> late here, but I don't find "treat as read-only" really clear. I don't have
Michael Paquier writes:
> The DECLARE CURSOR case in ExplainOneUtility() does a copy of a Query.
> Perhaps a comment should be added to explain why a copy is still
> required?
I did add a comment about that in the v2 patch --- the issue is the
call path for EXPLAIN EXECUTE.
On Thu, Jun 17, 2021 at 02:08:34PM -0700, Andres Freund wrote:
> Unfortunately github is annoying to search through - it doesn't seem to
> have any logic to prevent duplicates across repositories :(. Which means
> there's dozens of copies of postgres code included...
I agree with the position of d
I wrote:
> (In any case, if someone does get excited about this, they
> could rearrange things to push the copyObject calls into the
> individual arms of the switch in ProcessUtility. Personally
> though I doubt it could be worth the code bloat.)
It occurred to me to try making the copying code l
Hi,
On 2021-06-17 16:50:57 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2021-06-17 15:53:22 -0400, Tom Lane wrote:
> >> Uh, nobody ever promised that server-internal APIs are frozen as of beta1;
> >> that would be a horrid crimp on our ability to fix bugs during beta.
>
> > Sure, there'
Andres Freund writes:
> On 2021-06-17 15:53:22 -0400, Tom Lane wrote:
>> Uh, nobody ever promised that server-internal APIs are frozen as of beta1;
>> that would be a horrid crimp on our ability to fix bugs during beta.
> Sure, there's no promise. But I still think it's worth taking the amount
>
Andres Freund writes:
> On 2021-06-16 21:39:49 -0400, Tom Lane wrote:
>> Although this adds some overhead in the form of copying of
>> utility node trees that won't actually mutate during execution,
>> I think that won't be too bad because those trees tend to be
>> small and hence cheap to copy.
Hi,
On 2021-06-17 15:53:22 -0400, Tom Lane wrote:
> Andres Freund writes:
> > Phew. Do we really want to break a quite significant number of
> > extensions this long after feature freeze? Since we already need to find
> > a backpatchable way to deal with the issue it seems like deferring the
> >
Andres Freund writes:
> Phew. Do we really want to break a quite significant number of
> extensions this long after feature freeze? Since we already need to find
> a backpatchable way to deal with the issue it seems like deferring the
> API change to 15 might be prudent?
Uh, nobody ever promised
Hi,
On 2021-06-17 13:03:29 -0400, Tom Lane wrote:
> Here's a v2 that does it like that. In this formulation, we're
> basically hoisting the responsibility for doing copyObject up into
> ProcessUtility from its direct children, which seems like a clearer
> way of thinking about what has to change.
Hi,
On 2021-06-16 21:39:49 -0400, Tom Lane wrote:
> Although this adds some overhead in the form of copying of
> utility node trees that won't actually mutate during execution,
> I think that won't be too bad because those trees tend to be
> small and hence cheap to copy. The statements that can
On Thu, Jun 17, 2021 at 01:03:29PM -0400, Tom Lane wrote:
>
> Here's a v2 that does it like that. In this formulation, we're
> basically hoisting the responsibility for doing copyObject up into
> ProcessUtility from its direct children, which seems like a clearer
> way of thinking about what has
I wrote:
> What seems like a much safer answer is to make the API change noisy.
> That is, move the responsibility for actually calling copyObject
> into ProcessUtility itself, and add a bool parameter saying whether
> that needs to be done. That forces all callers to consider the
> issue, and giv
I wrote:
> What I found is that there are only two places that call
> ProcessUtility with a statement that might be coming from the plan
> cache. _SPI_execute_plan is always doing so, so it can just
> unconditionally copy the statement. The other one is
> PortalRunUtility, which can examine the P
On Wed, Jun 16, 2021 at 6:40 PM Tom Lane wrote:
> I haven't pushed my quick-hack fix for bug #17053 ([1]) because
> I wasn't really satisfied with band-aiding that problem in one
> more place. I took a look around to see if we could protect
> against the whole class of scribble-on-a-utility-stat
18 matches
Mail list logo
