On Wed, Mar 20, 2024 at 2:57 AM Tom Lane wrote:
> Richard Guo writes:
> > Here is the patch for HEAD. I simply re-posted v10. Nothing has
> > changed.
>
> I got back to this finally, and pushed it with some minor cosmetic
> adjustments.
Thanks for pushing!
Thanks
Richard
Richard Guo writes:
> Here is the patch for HEAD. I simply re-posted v10. Nothing has
> changed.
I got back to this finally, and pushed it with some minor cosmetic
adjustments.
regards, tom lane
On Fri, Feb 2, 2024 at 1:45 AM Tom Lane wrote:
> I pushed v12 (with some cosmetic adjustments) into the back branches,
> since we're getting close to the February release freeze. We still
> need to deal with the larger fix for HEAD. Please re-post that
> one so that the cfbot knows which is the
Richard Guo writes:
> How about the attached v12 patch? But I'm a little concerned about
> omitting PVC_RECURSE_AGGREGATES and PVC_RECURSE_WINDOWFUNCS in this
> case. Is it possible that aggregates or window functions appear in the
> tablesample expression?
No, they can't appear there; it'd mak
On Wed, Jan 31, 2024 at 11:21 PM Tom Lane wrote:
> Richard Guo writes:
> > On Wed, Jan 31, 2024 at 5:12 AM Tom Lane wrote:
> >> * Why is it okay to just use pull_varnos on a tablesample expression,
> >> when contain_references_to() does something different?
>
> > Hmm, the main reason is that th
Richard Guo writes:
> On Wed, Jan 31, 2024 at 5:12 AM Tom Lane wrote:
>> * Why is it okay to just use pull_varnos on a tablesample expression,
>> when contain_references_to() does something different?
> Hmm, the main reason is that the expression_tree_walker() function does
> not handle T_Restri
On Wed, Jan 31, 2024 at 5:12 AM Tom Lane wrote:
> Richard Guo writes:
> > On Wed, Jan 17, 2024 at 5:01 PM Richard Guo
> wrote:
> >> Sure, here it is:
> >> v10-0001-Avoid-reparameterizing-Paths-when-it-s-not-suitable.patch
>
> > I forgot to mention that this patch applies on v16 not master.
>
>
Richard Guo writes:
> On Wed, Jan 17, 2024 at 5:01 PM Richard Guo wrote:
>> Sure, here it is:
>> v10-0001-Avoid-reparameterizing-Paths-when-it-s-not-suitable.patch
> I forgot to mention that this patch applies on v16 not master.
I spent some time looking at this patch (which seems more urgent t
On Wed, Jan 17, 2024 at 5:01 PM Richard Guo wrote:
> On Tue, Jan 16, 2024 at 2:30 AM Robert Haas wrote:
>
>> On Mon, Jan 8, 2024 at 3:32 AM Richard Guo
>> wrote:
>
> > Fair point. I think we can back-patch a more minimal fix, as Tom
>> > proposed in [1], which disallows the reparameterization
On Tue, Jan 16, 2024 at 2:30 AM Robert Haas wrote:
> On Mon, Jan 8, 2024 at 3:32 AM Richard Guo wrote:
> > Thanks for the suggestion. Attached is an updated patch which is added
> > with a commit message that tries to explain the problem and the fix.
>
> This is great. The references to "the sa
Robert Haas writes:
> I know this is on Tom's to-do list which makes me a bit reluctant to
> get too involved here, because certainly he knows this code better
> than I do, maybe better than anyone does, but on the other hand, we
> shouldn't leave server crashes unfixed for too long, so maybe I ca
On Mon, Jan 8, 2024 at 3:32 AM Richard Guo wrote:
> Thanks for the suggestion. Attached is an updated patch which is added
> with a commit message that tries to explain the problem and the fix.
This is great. The references to "the sampling infos" are a little bit
confusing to me. I'm not sure i
Thanks for the review!
On Sat, Jan 6, 2024 at 2:36 AM Robert Haas wrote:
> Richard, I think it could be useful to put a better commit message
> into the patch file, describing both what problem is being fixed and
> what the design of the fix is. I gather that the problem is that we
> crash if th
Robert Haas writes:
> OK, so a few people like the current form of this patch but we haven't
> heard from Tom since August. Tom, any thoughts on the current
> incarnation?
Not yet, but it is on my to-look-at list. In the meantime I concur
with your comments here.
regards
On Tue, Dec 12, 2023 at 9:55 PM Andrei Lepikhov
wrote:
> By and large, this patch is in a good state and may be committed.
OK, so a few people like the current form of this patch but we haven't
heard from Tom since August. Tom, any thoughts on the current
incarnation?
Richard, I think it could b
On 6/12/2023 14:30, Richard Guo wrote:
I've self-reviewed this patch again and I think it's now in a
committable state. I'm wondering if we can mark it as 'Ready for
Committer' now, or we need more review comments/feedbacks.
To recap, this patch postpones reparameterization of paths until
creat
On Fri, Dec 8, 2023 at 5:39 PM Alena Rybakina
wrote:
> On 06.12.2023 10:30, Richard Guo wrote:
> > I've self-reviewed this patch again and I think it's now in a
> > committable state. I'm wondering if we can mark it as 'Ready for
> > Committer' now, or we need more review comments/feedbacks.
> >
On 06.12.2023 10:30, Richard Guo wrote:
I've self-reviewed this patch again and I think it's now in a
committable state. I'm wondering if we can mark it as 'Ready for
Committer' now, or we need more review comments/feedbacks.
To recap, this patch postpones reparameterization of paths until
crea
I've self-reviewed this patch again and I think it's now in a
committable state. I'm wondering if we can mark it as 'Ready for
Committer' now, or we need more review comments/feedbacks.
To recap, this patch postpones reparameterization of paths until
createplan.c, which would help avoid building
On Fri, Oct 20, 2023 at 2:52 AM Alena Rybakina
wrote:
> Thank you for your work on the subject.
>
Thanks for taking an interest in this patch.
> During review your patch I didn't understand why are you checking that the
> variable is path and not new_path of type T_SamplePath (I highlighted it)
Hi!
Thank you for your work on the subject.
During review your patch I didn't understand why are you checking that
the variable is path and not new_path of type T_SamplePath (I
highlighted it)?
Path *
reparameterize_path_by_child(PlannerInfo *root, Path *path,
RelOptInfo *child_rel)
...
s
On 18/10/2023 13:39, Richard Guo wrote:
On Fri, Oct 13, 2023 at 6:18 PM Andrei Lepikhov
mailto:a.lepik...@postgrespro.ru>> wrote:
On 23/8/2023 12:37, Richard Guo wrote:
> To fix it we may need to modify RelOptInfos for Path, BitmapHeapPath,
> ForeignPath and CustomPath, and modi
On Fri, Oct 13, 2023 at 6:18 PM Andrei Lepikhov
wrote:
> On 23/8/2023 12:37, Richard Guo wrote:
> > To fix it we may need to modify RelOptInfos for Path, BitmapHeapPath,
> > ForeignPath and CustomPath, and modify IndexOptInfos for IndexPath. It
> > seems that that is not easily done without post
On 23/8/2023 12:37, Richard Guo wrote:
To fix it we may need to modify RelOptInfos for Path, BitmapHeapPath,
ForeignPath and CustomPath, and modify IndexOptInfos for IndexPath. It
seems that that is not easily done without postponing reparameterization
of paths until createplan.c.
Attached is a
On 23/8/2023 12:37, Richard Guo wrote:
If we go with the "tablesample scans can't be reparameterized" approach
in the back branches, I'm a little concerned that what if we find more
cases in the futrue where we need modify RTEs for reparameterization.
So I spent some time seeking and have managed
On Fri, Sep 8, 2023 at 3:04 PM Ashutosh Bapat
wrote:
> When the clause s.t1b = s.a is presented to distribute_qual_to_rels()
> it has form PHV(t1.b) = t2.b. The PHV's ph_eval_at is 4, which is what
> is returned as varno to pull_varnos(). The other Var in the caluse has
> varno = 4 already so pu
On Thu, Aug 24, 2023 at 8:17 AM Richard Guo wrote:
>
>
> On Thu, Aug 24, 2023 at 1:44 AM Tom Lane wrote:
>>
>> Richard Guo writes:
>> > If we go with the "tablesample scans can't be reparameterized" approach
>> > in the back branches, I'm a little concerned that what if we find more
>> > cases i
On Thu, Aug 24, 2023 at 1:44 AM Tom Lane wrote:
> Richard Guo writes:
> > If we go with the "tablesample scans can't be reparameterized" approach
> > in the back branches, I'm a little concerned that what if we find more
> > cases in the futrue where we need modify RTEs for reparameterization.
>
Richard Guo writes:
> If we go with the "tablesample scans can't be reparameterized" approach
> in the back branches, I'm a little concerned that what if we find more
> cases in the futrue where we need modify RTEs for reparameterization.
> So I spent some time seeking and have managed to find one
On Wed, Aug 23, 2023 at 11:07 AM Richard Guo wrote:
>
>
> On Tue, Aug 22, 2023 at 10:39 PM Tom Lane wrote:
>>
>> Richard Guo writes:
>> > I'm wondering if we can instead adjust the 'inner_req_outer' in
>> > create_nestloop_path before we perform the check to work around this
>> > issue for the b
On Tue, Aug 22, 2023 at 10:39 PM Tom Lane wrote:
> Richard Guo writes:
> > I'm wondering if we can instead adjust the 'inner_req_outer' in
> > create_nestloop_path before we perform the check to work around this
> > issue for the back branches, something like
> > + /*
> > +* Adjust the par
Hi Tom,
On Tue, Aug 22, 2023 at 2:18 AM Tom Lane wrote:
>
> (BTW, I did look at Ashutosh's idea of merging the
> reparameterize_path_by_child and path_is_reparameterizable_by_child
> functions, but I didn't think that would be an improvement,
> because we'd have to clutter reparameterize_path_by
I wrote:
> ... I think the risk/reward ratio for messing with this in the
> back branches is unattractive in any case: to fix a corner case that
> apparently nobody uses in the field, we risk breaking any number of
> mainstream parameterized-path cases. I'm content to commit the v5 patch
> (or a s
Richard Guo writes:
> On Tue, Aug 22, 2023 at 4:48 AM Tom Lane wrote:
>> So that led me to the attached v5, which seemed committable to me so I
>> set about back-patching it ... and it fell over immediately in v15, as
>> shown in the attached regression diffs from v15. It looks to me like
>> we
On Tue, Aug 22, 2023 at 4:48 AM Tom Lane wrote:
> I spent some time reviewing the v4 patch. I noted that
> path_is_reparameterizable_by_child still wasn't modeling the pass/fail
> behavior of reparameterize_path_by_child very well, because that
> function checks this at every recursion level:
>
I spent some time reviewing the v4 patch. I noted that
path_is_reparameterizable_by_child still wasn't modeling the pass/fail
behavior of reparameterize_path_by_child very well, because that
function checks this at every recursion level:
/*
* If the path is not parameterized by the paren
On Mon, Aug 21, 2023 at 4:09 PM Richard Guo wrote:
>
>
> On Sun, Aug 20, 2023 at 11:48 PM Tom Lane wrote:
>>
>> I looked over the v3 patch. I think it's going in generally
>> the right direction, but there is a huge oversight:
>> path_is_reparameterizable_by_child does not come close to modeling
On Sun, Aug 20, 2023 at 11:48 PM Tom Lane wrote:
> I looked over the v3 patch. I think it's going in generally
> the right direction, but there is a huge oversight:
> path_is_reparameterizable_by_child does not come close to modeling
> the success/failure conditions of reparameterize_path_by_chi
I looked over the v3 patch. I think it's going in generally
the right direction, but there is a huge oversight:
path_is_reparameterizable_by_child does not come close to modeling
the success/failure conditions of reparameterize_path_by_child.
In all the cases where reparameterize_path_by_child can
On Wed, Aug 9, 2023 at 8:14 AM Richard Guo wrote:
>
> With this patch, the reparameterize_path_by_child work is postponed
> until createplan time, so in create_nestloop_path() the inner path is
> still parameterized by top parent. So we have to check against the top
> parent of outer rel.
>
Your
On Tue, Aug 8, 2023 at 7:34 PM Ashutosh Bapat
wrote:
> On Tue, Aug 8, 2023 at 12:44 PM Richard Guo
> wrote:
> > This is not an existing bug. Our current code (without this patch)
> > would always call create_nestloop_path() after the reparameterization
> > work has been done for the inner path.
On Tue, Aug 8, 2023 at 12:44 PM Richard Guo wrote:
>
>
> I agree that we should mention in the function's comment that
> reparameterize_path_by_child can only be used after the best path has
> been selected because the RTE might be modified by this function. I'm
> not sure if it's a good idea to
On Mon, Aug 7, 2023 at 9:29 PM Ashutosh Bapat
wrote:
> Thanks. The patch looks good overall. I like it because of its potential
> to reduce memory consumption in reparameterization. That's cake on top of
> cream :)
>
Thanks for reviewing this patch!
> Some comments here.
>
> + {
> + FLAT_COPY_
On Fri, Aug 4, 2023 at 6:08 AM Richard Guo wrote:
>
> On Thu, Aug 3, 2023 at 7:20 PM Ashutosh Bapat <
> ashutosh.bapat@gmail.com> wrote:
>
>> However, if reparameterization can *not* happen at the planning time, we
>> have chosen a plan which can not be realised meeting a dead end. So as long
On Thu, Aug 3, 2023 at 7:20 PM Ashutosh Bapat
wrote:
> However, if reparameterization can *not* happen at the planning time, we
> have chosen a plan which can not be realised meeting a dead end. So as long
> as we can ensure that the reparameterization is possible we can delay
> actual translatio
On Thu, Aug 3, 2023 at 4:14 PM Richard Guo wrote:
>
> On Thu, Aug 3, 2023 at 12:38 PM Ashutosh Bapat <
> ashutosh.bapat@gmail.com> wrote:
>
> Sometimes it would help to avoid the translation at all if we postpone
> the reparameterization until createplan.c, such as in the case that a
> non-
On Thu, Aug 3, 2023 at 12:38 PM Ashutosh Bapat
wrote:
> On Tue, Aug 1, 2023 at 6:50 PM Tom Lane wrote:
> > Alternatively, could we postpone the reparameterization until
> > createplan.c? Having to build reparameterized expressions we might
> > not use seems expensive, and it would also contribu
On Tue, Aug 1, 2023 at 6:50 PM Tom Lane wrote:
>
> Alternatively, could we postpone the reparameterization until
> createplan.c? Having to build reparameterized expressions we might
> not use seems expensive, and it would also contribute to the memory
> bloat being complained of in nearby thread
On Tue, Aug 1, 2023 at 9:20 PM Tom Lane wrote:
> Richard Guo writes:
> > So what I'm thinking is that maybe we can add a new type of path, named
> > SampleScanPath, to have the TableSampleClause per path. Then we can
> > safely reparameterize the TableSampleClause as needed for each
> > SampleS
Richard Guo writes:
> In this case what we need to do is to adjust the TableSampleClause to
> refer to the correct child relations. We can do that with the help of
> adjust_appendrel_attrs_multilevel(). One problem is that the
> TableSampleClause is stored in RangeTblEntry, and it does not seem
For paths of type 'T_Path', reparameterize_path_by_child just does the
flat-copy but does not adjust the expressions that have lateral
references. This would have problems for partitionwise-join. As an
example, consider
regression=# explain (costs off)
select * from prt1 t1 join lateral
(sel
51 matches
Mail list logo
