On Wed, May 1, 2019 at 4:08 AM Tom Lane wrote:
> OK, I tweaked that a bit and pushed both versions.
Thank you.
Regards,
Amit
Amit Langote writes:
> On Tue, Apr 30, 2019 at 1:26 PM Amit Langote wrote:
>> It would be nice if at least we fix the bug that directly accessed
>> partitions are not excluded with constraint_exclusion = on, without
>> removing PG 11's contortions in relation_excluded_by_constraints to
>> work ar
Amit Langote writes:
> Here is the patch. I've also included the patch to update the text in
> ddl.sgml regarding constraint exclusion and partition pruning.
I thought this was a bit messy. In particular, IMV the reason to
have a split between get_relation_constraints and its only caller
relati
On Sun, Apr 28, 2019 at 8:10 AM Tom Lane wrote:
> Amit Langote writes:
> > Not sure if you'll like it but maybe we could ignore even regular
> > inheritance child targets that are proven to be empty (is_dummy_rel()) for
> > a given query during the initial SELECT planning. That way, we can avoid
Amit Langote writes:
> On 2019/04/23 7:08, Tom Lane wrote:
>> [ a bunch of stuff ]
> Not sure if you'll like it but maybe we could ignore even regular
> inheritance child targets that are proven to be empty (is_dummy_rel()) for
> a given query during the initial SELECT planning. That way, we can
On 2019/04/23 7:08, Tom Lane wrote:
> Amit Langote writes:
>> On 2019/04/02 2:34, Tom Lane wrote:
>>> I'm not at all clear on what we think the interaction between
>>> enable_partition_pruning and constraint_exclusion ought to be,
>>> so I'm not sure what the appropriate resolution is here. Thoug
Amit Langote writes:
> On 2019/04/02 2:34, Tom Lane wrote:
>> I'm not at all clear on what we think the interaction between
>> enable_partition_pruning and constraint_exclusion ought to be,
>> so I'm not sure what the appropriate resolution is here. Thoughts?
> Prior to 428b260f87 (that is, in P
On 2019/04/11 14:03, David Rowley wrote:
> On Fri, 5 Apr 2019 at 19:50, Amit Langote
> wrote:
>> While we're on the topic of the relation between constraint exclusion and
>> partition pruning, I'd like to (re-) propose this documentation update
>> patch. The partitioning chapter in ddl.sgml says
On Fri, 5 Apr 2019 at 19:50, Amit Langote wrote:
> While we're on the topic of the relation between constraint exclusion and
> partition pruning, I'd like to (re-) propose this documentation update
> patch. The partitioning chapter in ddl.sgml says update/delete of
> partitioned tables uses const
On Fri, 5 Apr 2019 at 23:07, Amit Langote wrote:
>
> Hi,
>
> On 2019/04/05 18:13, Floris Van Nee wrote:
> > One unrelated thing I noticed (but I'm not sure if it's worth a separate
> > email thread) is that the changed default of jit=on in v12 doesn't work
> > very well with a large number of pa
Hi,
On 2019/04/05 18:13, Floris Van Nee wrote:
> One unrelated thing I noticed (but I'm not sure if it's worth a separate
> email thread) is that the changed default of jit=on in v12 doesn't work very
> well with a large number of partitions at run-time, for which a large number
> get excluded
Thanks for the details! Indeed the versions with now()/current_date use the
runtime pruning rather than planning time. I wasn't aware of the use of 'today'
though - that could be useful in case we're sure statements won't be prepared.
Previously (v10/ partly v11) it was necessary to make sure th
On 2019/04/02 14:50, Amit Langote wrote:
> Attached patch is only for HEAD this time. I'll post one for PG 11 (if
> you'd like) once we reach consensus on the best thing to do here is.
While we're on the topic of the relation between constraint exclusion and
partition pruning, I'd like to (re-) p
On 2019/04/05 12:18, David Rowley wrote:
> On Fri, 5 Apr 2019 at 16:09, Amit Langote
> wrote:
>> Although, we still have ways
>> to go in terms of scaling generic plan execution to larger partition
>> counts, solution(s) for which have been proposed by David but haven't made
>> it into master yet
On Fri, 5 Apr 2019 at 16:09, Amit Langote wrote:
> Although, we still have ways
> to go in terms of scaling generic plan execution to larger partition
> counts, solution(s) for which have been proposed by David but haven't made
> it into master yet.
Is that a reference to the last paragraph in [1
On 2019/04/05 6:59, David Rowley wrote:
> On Fri, 5 Apr 2019 at 07:33, Floris Van Nee wrote:
>> I had a question about the performance of pruning of functions like now()
>> and current_date. I know these are handled differently, as they cannot be
>> excluded during the first phases of planning.
On Fri, 5 Apr 2019 at 07:33, Floris Van Nee wrote:
> I had a question about the performance of pruning of functions like now() and
> current_date. I know these are handled differently, as they cannot be
> excluded during the first phases of planning. However, curerntly, this new
> patch makes t
;
Amit Langote; Alvaro Herrera; Robert Haas; Justin Pryzby; Pg Hackers
Subject: Re: speeding up planning with partitions [External]
Thanks for taking a look.
On 2019/04/02 2:34, Tom Lane wrote:
> Amit Langote writes:
>> On 2019/03/30 0:29, Tom Lane wrote:
>>> That seems like
Thanks for taking a look.
On 2019/04/02 2:34, Tom Lane wrote:
> Amit Langote writes:
>> On 2019/03/30 0:29, Tom Lane wrote:
>>> That seems like probably an independent patch --- do you want to write it?
>
>> Here is that patch.
>> It revises get_relation_constraints() such that the partition con
Amit Langote writes:
> On 2019/03/30 0:29, Tom Lane wrote:
>> That seems like probably an independent patch --- do you want to write it?
> Here is that patch.
> It revises get_relation_constraints() such that the partition constraint
> is loaded in only the intended cases.
So I see the problem y
Amit Langote writes:
> On 2019/04/01 3:46, Tom Lane wrote:
>> One thing that I intentionally left out of the committed patch was changes
>> to stop short of scanning the whole simple_rel_array when looking only for
>> baserels. I thought that had been done in a rather piecemeal fashion
>> and it'
On 2019/03/30 0:29, Tom Lane wrote:
> Amit Langote writes:
>> Finally, it's not in the patch, but how about visiting
>> get_relation_constraints() for revising this block of code:
>
> That seems like probably an independent patch --- do you want to write it?
Here is that patch.
It revises get_r
On 2019/04/01 3:46, Tom Lane wrote:
> One thing that I intentionally left out of the committed patch was changes
> to stop short of scanning the whole simple_rel_array when looking only for
> baserels. I thought that had been done in a rather piecemeal fashion
> and it'd be better to address it ho
(I've closed the CF entry: https://commitfest.postgresql.org/22/1778/)
On 2019/04/01 2:04, Tom Lane wrote:
> Amit Langote writes:
>> On Sun, Mar 31, 2019 at 11:45 AM Imai Yoshikazu
>> wrote:
>>> Certainly, using bitmapset contributes to the performance when scanning
>>> one partition(few partit
On Sun, 31 Mar 2019 at 05:50, Robert Haas wrote:
>
> On Sat, Mar 30, 2019 at 12:16 PM Amit Langote wrote:
> > Fwiw, I had complained when reviewing the run-time pruning patch that
> > creating those maps in the planner and putting them in
> > PartitionPruneInfo might not be a good idea, but David
One thing that I intentionally left out of the committed patch was changes
to stop short of scanning the whole simple_rel_array when looking only for
baserels. I thought that had been done in a rather piecemeal fashion
and it'd be better to address it holistically, which I've now done in the
attac
Amit Langote writes:
> On Sun, Mar 31, 2019 at 11:45 AM Imai Yoshikazu
> wrote:
>> Certainly, using bitmapset contributes to the performance when scanning
>> one partition(few partitions) from large partitions.
> Thanks Imai-san for testing.
I tried to replicate these numbers with the code as-
On Sun, Mar 31, 2019 at 11:45 AM Imai Yoshikazu wrote:
> On 2019/03/31 1:06, Amit Langote wrote:
> > On Sun, Mar 31, 2019 at 12:11 AM Tom Lane wrote:
> >> I am curious as to why there seems to be more degradation
> >> for hash cases, as per Yoshikazu-san's results in
> >> <0F97FA9ABBDBE54F917
On 2019/03/31 1:06, Amit Langote wrote:
> On Sun, Mar 31, 2019 at 12:11 AM Tom Lane wrote:
>> Amit Langote writes:
>>> I think the performance results did prove that degradation due to
>>> those loops over part_rels becomes significant for very large
>>> partition counts. Is there a better
On Sat, Mar 30, 2019 at 12:16 PM Amit Langote wrote:
> Fwiw, I had complained when reviewing the run-time pruning patch that
> creating those maps in the planner and putting them in
> PartitionPruneInfo might not be a good idea, but David insisted that
> it'd be good for performance (in the contex
On Sun, Mar 31, 2019 at 12:59 AM Robert Haas wrote:
>
> On Sat, Mar 30, 2019 at 11:46 AM Tom Lane wrote:
> > > The only problem with PartitionPruneInfo structures of which I am
> > > aware is that they rely on PartitionDesc offsets not changing. But I
> > > added code in that commit in ExecCreate
On Sun, Mar 31, 2019 at 12:11 AM Tom Lane wrote:
> Amit Langote writes:
> > I think the performance results did prove that degradation due to
> > those loops over part_rels becomes significant for very large
> > partition counts. Is there a better solution than the bitmapset that
> > you have in
On Sat, Mar 30, 2019 at 11:46 AM Tom Lane wrote:
> > The only problem with PartitionPruneInfo structures of which I am
> > aware is that they rely on PartitionDesc offsets not changing. But I
> > added code in that commit in ExecCreatePartitionPruneState to handle
> > that exact problem. See also
Robert Haas writes:
> On Sat, Mar 30, 2019 at 11:11 AM Tom Lane wrote:
>> Before that, though, I remain concerned that the PartitionPruneInfo
>> data structure the planner is transmitting to the executor is unsafe
>> against concurrent ATTACH PARTITION operations. The comment for
>> PartitionedR
On Sat, Mar 30, 2019 at 11:11 AM Tom Lane wrote:
> Before that, though, I remain concerned that the PartitionPruneInfo
> data structure the planner is transmitting to the executor is unsafe
> against concurrent ATTACH PARTITION operations. The comment for
> PartitionedRelPruneInfo says in so many
Amit Langote writes:
> On Sat, Mar 30, 2019 at 9:17 AM Tom Lane wrote:
>> What I propose we do about the GEQO problem is shown in 0001 attached
>> (which would need to be back-patched into v11).
>> ...
>> That's just dumb. What we *ought* to be doing in such degenerate
>> outer-join cases is jus
Thanks for the new patches.
On Sat, Mar 30, 2019 at 9:17 AM Tom Lane wrote:
>
> Amit Langote writes:
> > On 2019/03/29 7:38, Tom Lane wrote:
> >> 2. I seriously dislike what's been done in joinrels.c, too. That
> >> really seems like a kluge (and I haven't had time to study it
> >> closely).
>
Amit Langote writes:
> On 2019/03/29 7:38, Tom Lane wrote:
>> 2. I seriously dislike what's been done in joinrels.c, too. That
>> really seems like a kluge (and I haven't had time to study it
>> closely).
> Those hunks account for the fact that pruned partitions, for which we no
> longer create
Amit Langote writes:
> On 2019/03/29 7:38, Tom Lane wrote:
>> 2. I seriously dislike what's been done in joinrels.c, too. That
>> really seems like a kluge (and I haven't had time to study it
>> closely).
> Those hunks account for the fact that pruned partitions, for which we no
> longer create
I wrote:
> Amit Langote writes:
>> About the XXX: I think resetting inh flag is unnecessary, so we should
>> just remove the line.
> Possibly. I hadn't had time to follow up the XXX annotation.
Now I have ...
Yeah, it seems we can just drop that and leave the flag alone. We'll
end up running
Amit Langote writes:
> Here are some comments on v38.
Thanks for looking it over! I'll just reply to points worth discussing:
> -Assert(rte->rtekind == RTE_RELATION ||
> - rte->rtekind == RTE_SUBQUERY);
> -add_appendrel_other_rels(root, rel, rti);
> +
Here are some comments on v38.
On 2019/03/29 12:44, Amit Langote wrote:
> Thanks again for the new patch. I'm reading it now and will send comments
> later today if I find something.
-Assert(rte->rtekind == RTE_RELATION ||
- rte->rtekind == RTE_SUBQUERY);
-
On Fri, Mar 29, 2019 at 3:45 PM, Amit Langote wrote:
> Thanks a lot for hacking on the patch. I'm really happy with the direction
> you took for inheritance_planner, as it allows UPDATE/DELETE to use
> partition pruning.
I was astonished by Tom's awesome works and really thanks him.
> Certainly.
Thanks a lot for hacking on the patch. I'm really happy with the
direction you took for inheritance_planner, as it allows UPDATE/DELETE to
use partition pruning.
On 2019/03/29 7:38, Tom Lane wrote:
> I've been hacking on this pretty hard over the last couple days,
> because I really didn't like t
I've been hacking on this pretty hard over the last couple days,
because I really didn't like the contortions you'd made to allow
inheritance_planner to call expand_inherited_rtentry in a completely
different context than the regular code path did. I eventually got
rid of that by having inheritanc
On 2019/03/28 14:03, Tom Lane wrote:
> Amit Langote writes:
>> On 2019/03/27 23:57, Tom Lane wrote:
>>> Yeah, there's something to be said for having plancat.c open each table
>>> *and store the Relation pointer in the RelOptInfo*, and then close that
>>> again at the end of planning rather than i
Amit Langote writes:
> On 2019/03/27 23:57, Tom Lane wrote:
>> Yeah, there's something to be said for having plancat.c open each table
>> *and store the Relation pointer in the RelOptInfo*, and then close that
>> again at the end of planning rather than immediately. If we can't avoid
>> these ret
On 2019/03/27 23:57, Tom Lane wrote:
> David Rowley writes:
>> On Wed, 27 Mar 2019 at 18:39, Amit Langote
>> wrote:
>>> On 2019/03/27 14:26, David Rowley wrote:
Perhaps the way to make this work, at least in the long term is to do
in the planner what we did in the executor in d73f4c74dd
David Rowley writes:
> On Wed, 27 Mar 2019 at 18:39, Amit Langote
> wrote:
>> On 2019/03/27 14:26, David Rowley wrote:
>>> Perhaps the way to make this work, at least in the long term is to do
>>> in the planner what we did in the executor in d73f4c74dd34.
>> Maybe you meant 9ddef36278a9?
> Pro
On Wed, 27 Mar 2019 at 18:39, Amit Langote
wrote:
>
> On 2019/03/27 14:26, David Rowley wrote:
> > Perhaps the way to make this work, at least in the long term is to do
> > in the planner what we did in the executor in d73f4c74dd34.
>
> Maybe you meant 9ddef36278a9?
Probably.
> What would be nic
On 2019/03/27 14:26, David Rowley wrote:
> On Wed, 27 Mar 2019 at 18:13, Amit Langote
> wrote:
>>
>> On 2019/03/27 13:50, Amit Langote wrote:
>>> On 2019/03/27 12:06, Amit Langote wrote:
I wonder if I should rework inherit.c so that its internal interfaces
don't pass around parent Relati
On Wed, 27 Mar 2019 at 18:13, Amit Langote
wrote:
>
> On 2019/03/27 13:50, Amit Langote wrote:
> > On 2019/03/27 12:06, Amit Langote wrote:
> >> I wonder if I should rework inherit.c so that its internal interfaces
> >> don't pass around parent Relation, but make do with the RelOptInfo? I'll
> >>
On 2019/03/27 13:50, Amit Langote wrote:
> On 2019/03/27 12:06, Amit Langote wrote:
>> I wonder if I should rework inherit.c so that its internal interfaces
>> don't pass around parent Relation, but make do with the RelOptInfo? I'll
>> need to add tupdesc and reltype fields to RelOptInfo to go ahe
On 2019/03/27 12:06, Amit Langote wrote:
> I wonder if I should rework inherit.c so that its internal interfaces
> don't pass around parent Relation, but make do with the RelOptInfo? I'll
> need to add tupdesc and reltype fields to RelOptInfo to go ahead with that
> though.
To give more context o
On 2019/03/27 1:06, Tom Lane wrote:
> Amit Langote writes:
>> 0002 is a new patch to get rid of the duplicate RTE and PlanRowMark that's
>> created for partitioned parent table, as it's pointless. I was planning
>> to propose it later, but decided to post it now if we're modifying the
>> nearby c
Amit Langote writes:
> On 2019/03/23 6:07, Tom Lane wrote:
>> I also feel like you used a dartboard while deciding where to insert the
>> call in query_planner(); dropping it into the middle of a sequence of
>> equivalence-class-related operations seems quite random. Maybe we could
>> delay that
Amit Langote writes:
> 0002 is a new patch to get rid of the duplicate RTE and PlanRowMark that's
> created for partitioned parent table, as it's pointless. I was planning
> to propose it later, but decided to post it now if we're modifying the
> nearby code anyway.
Good idea, but it needs a bit
Amit-san,
On Tue, Mar 26, 2019 at 7:17 AM, Amit Langote wrote:
> Rebased patches attached.
I could only do the code review of v36-0001.
On Mon, Mar 25, 2019 at 11:35 AM, Amit Langote wrote:
> On 2019/03/23 6:07, Tom Lane wrote:
> > Amit Langote writes:
> >> [ v34 patch set ]
> >
> > I had a bi
On Fri, Mar 22, 2019 at 9:07 PM, Tom Lane wrote:
> BTW, it strikes me that we could take advantage of the fact that baserels
> must all appear before otherrels in the rel array, by having loops over
> that array stop early if they're only interested in baserels. We could
> either save the index of
I wrote:
> ... I also
> don't like the undocumented way that you've got build_base_rel_tlists
> working on something that's not the final tlist, and then going back
> and doing more work of the same sort later. I wonder whether we can
> postpone build_base_rel_tlists until after the other stuff ha
Amit Langote writes:
> [ v34 patch set ]
I had a bit of a look through this. I went ahead and pushed 0008 and
0009, as they seem straightforward and independent of the rest. (BTW,
0009 makes 0003's dubious optimization in set_relation_partition_info
quite unnecessary.) As for the rest:
0001:
On Fri, 22 Mar 2019 at 20:39, Amit Langote
wrote:
> The problem is that make_partitionedrel_pruneinfo() does some work which
> involves allocating arrays containing nparts elements and then looping
> over the part_rels array to fill values in those arrays that will be used
> by run-time pruning.
Hi Amit,
On 3/22/19 3:39 AM, Amit Langote wrote:
I took a stab at this. I think rearranging the code in
make_partitionedrel_pruneinfo() slightly will take care of this.
The problem is that make_partitionedrel_pruneinfo() does some work which
involves allocating arrays containing nparts element
Jesper, Imai-san,
Thanks for testing and reporting your findings.
On 2019/03/21 23:10, Imai Yoshikazu wrote:
> On 2019/03/20 23:25, Jesper Pedersen wrote:> Hi,
> > My tests - using hash partitions - shows that the extra time is spent in
> > make_partition_pruneinfo() for the relid_subplan_map v
Hi Jesper,
On 2019/03/20 23:25, Jesper Pedersen wrote:> Hi,
>
> On 3/19/19 11:15 PM, Imai, Yoshikazu wrote:
>> Here the details.
>>
>> [creating partitioned tables (with 1024 partitions)]
>> drop table if exists rt;
>> create table rt (a int, b int, c int) partition by range (a);
>> \o /de
Hi Amit,
On 3/19/19 8:41 PM, Amit Langote wrote:
I have fixed all. Attached updated patches.
The comments in the thread has been addressed, so I have put the CF
entry into Ready for Committer.
Best regards,
Jesper
Hi,
On 3/19/19 11:15 PM, Imai, Yoshikazu wrote:
Here the details.
[creating partitioned tables (with 1024 partitions)]
drop table if exists rt;
create table rt (a int, b int, c int) partition by range (a);
\o /dev/null
select 'create table rt' || x::text || ' partition of rt for values from ('
Amit-san,
On Wed, Mar 20, 2019 at 9:07 AM, Amit Langote wrote:
> On 2019/03/20 17:36, Imai, Yoshikazu wrote:
> > On Wed, Mar 20, 2019 at 8:21 AM, Amit Langote wrote:
> >> On 2019/03/20 12:15, Imai, Yoshikazu wrote:
> >>> [select1024.sql]
> >>> \set a random (1, 1024)
> >>> select * from rt where a
Imai-san,
On 2019/03/20 17:36, Imai, Yoshikazu wrote:
> On Wed, Mar 20, 2019 at 8:21 AM, Amit Langote wrote:
>> On 2019/03/20 12:15, Imai, Yoshikazu wrote:
>>> [select1024.sql]
>>> \set a random (1, 1024)
>>> select * from rt where a = :a;
>>>
>>> [pgbench]
>>> pgbench -n -f select1024.sql -T 60
>
Amit-san,
On Wed, Mar 20, 2019 at 8:21 AM, Amit Langote wrote:
> On 2019/03/20 12:15, Imai, Yoshikazu wrote:
> > Here the details.
> >
> > [creating partitioned tables (with 1024 partitions)] drop table if
> > exists rt; create table rt (a int, b int, c int) partition by range
> > (a); \o /dev/nul
Imai-san,
On 2019/03/20 12:15, Imai, Yoshikazu wrote:
> Here the details.
>
> [creating partitioned tables (with 1024 partitions)]
> drop table if exists rt;
> create table rt (a int, b int, c int) partition by range (a);
> \o /dev/null
> select 'create table rt' || x::text || ' partition of rt f
Amit-san,
On Wed, Mar 20, 2019 at 3:01 PM, Amit Langote wrote:
> > On Wed, Mar 20, 2019 at 2:34 AM, Amit Langote wrote:
> >> On 2019/03/20 11:21, Imai, Yoshikazu wrote:
> >>> (4)
> >>> We expect the performance does not depend on the number of
> >>> partitions
> >> after applying all patches, if p
On 2019/03/20 11:51, Imai, Yoshikazu wrote:
> Amit-san,
>
> On Wed, Mar 20, 2019 at 2:34 AM, Amit Langote wrote:
>> On 2019/03/20 11:21, Imai, Yoshikazu wrote:
>>> (4)
>>> We expect the performance does not depend on the number of partitions
>> after applying all patches, if possible.
>>>
>>> num
Amit-san,
On Wed, Mar 20, 2019 at 2:34 AM, Amit Langote wrote:
> On 2019/03/20 11:21, Imai, Yoshikazu wrote:
> > (4)
> > We expect the performance does not depend on the number of partitions
> after applying all patches, if possible.
> >
> > num of partTPS
> > --- -
> > 1024
Imai-san,
On 2019/03/20 11:21, Imai, Yoshikazu wrote:
> (4)
> We expect the performance does not depend on the number of partitions after
> applying all patches, if possible.
>
> num of partTPS
> --- -
> 1024 7,257 (7274, 7246, 7252)
> 2048 6,718 (6627, 6780, 674
Amit-san,
On Wed, Mar 20, 2019 at 0:42 AM, Amit Langote wrote:
> On 2019/03/19 20:13, Imai, Yoshikazu wrote:
> > Thanks for new patches.
> > I looked over them and there are little comments.
> >
> > ...
> >
> > I have no more comments about codes other than above :)
>
> I have fixed all. Attach
On 2019/03/20 9:49, Robert Haas wrote:
> On Fri, Mar 8, 2019 at 4:18 AM Amit Langote
> wrote:
>> Maybe you know that range_table_mutator() spends quite a long time if
>> there are many target children, but I realized there's no need for
>> range_table_mutator() to copy/mutate child target RTEs. F
On Fri, Mar 8, 2019 at 4:18 AM Amit Langote
wrote:
> Maybe you know that range_table_mutator() spends quite a long time if
> there are many target children, but I realized there's no need for
> range_table_mutator() to copy/mutate child target RTEs. First, there's
> nothing to translate in their
Amit-san,
On Tue, Mar 19, 2019 at 6:53 AM, Amit Langote wrote:
> On 2019/03/15 9:33, Imai, Yoshikazu wrote:
> > On Thu, Mar 14, 2019 at 9:04 AM, Amit Langote wrote:
> >>> * In inheritance_planner(), we do ChangeVarNodes() only for
> >>> orig_rtable,
> >> so the codes concatenating lists of append_
Amit-san,
On Mon, Mar 18, 2019 at 9:56 AM, Amit Langote wrote:
> On 2019/03/15 14:40, Imai, Yoshikazu wrote:
> > Amit-san,
> >
> > I have another little comments about v31-patches.
> >
> > * We don't need is_first_child in inheritance_planner().
> >
> > On Fri, Mar 8, 2019 at 9:18 AM, Amit Langote
Imai-san,
On 2019/03/15 14:40, Imai, Yoshikazu wrote:
> Amit-san,
>
> I have another little comments about v31-patches.
>
> * We don't need is_first_child in inheritance_planner().
>
> On Fri, Mar 8, 2019 at 9:18 AM, Amit Langote wrote:
>> On 2019/03/08 16:16, Imai, Yoshikazu wrote:
>>> I attac
Amit-san,
I have another little comments about v31-patches.
* We don't need is_first_child in inheritance_planner().
On Fri, Mar 8, 2019 at 9:18 AM, Amit Langote wrote:
> On 2019/03/08 16:16, Imai, Yoshikazu wrote:
> > I attached the diff of modification for v26-0003 patch which also
> contains
Hi, David
On Thu, Mar 14, 2019 at 9:04 AM, David Rowley wrote:
> On Thu, 14 Mar 2019 at 21:35, Imai, Yoshikazu
> wrote:
> > 0007:
> > * This changes some processes using "for loop" to using
> "while(bms_next_member())" which speeds up processing when we scan few
> partitions in one statement, but
Amit-san,
On Thu, Mar 14, 2019 at 9:04 AM, Amit Langote wrote:
> > 0002:
> > * I don't really know that delaying adding resjunk output columns to
> the plan doesn't affect any process in the planner. From the comments
> of PlanRowMark, those columns are used in only the executor so I think
> delay
On Thu, 14 Mar 2019 at 21:35, Imai, Yoshikazu
wrote:
> 0007:
> * This changes some processes using "for loop" to using
> "while(bms_next_member())" which speeds up processing when we scan few
> partitions in one statement, but when we scan a lot of partitions in one
> statement, its performance
Imai-san,
On 2019/03/13 19:34, Imai, Yoshikazu wrote:
> I have done code review of v31 patches from 0001 to 0004.
> I described below what I confirmed or thoughts.
Thanks for checking.
> 0001: This seems ok.
>
> 0002:
> * I don't really know that delaying adding resjunk output columns to the pl
Amit-san,
I have done code review of v31 patches from 0004 to 0008.
0004:
* s/childern/children
0005:
* This seems reasonable for not using a lot of memory in specific case,
although it needs special looking of planner experts.
0006:
* The codes initializing/setting RelOptInfo's part_rels look
Amit-san,
On Tue, Mar 12, 2019 at 2:34 PM, Amit Langote wrote:
> Thanks for the heads up.
>
> On Tue, Mar 12, 2019 at 10:23 PM Jesper Pedersen
> wrote:
> > After applying 0004 check-world fails with the attached. CFBot agrees
> [1].
>
> Fixed. I had forgotten to re-run postgres_fdw tests after
Hi Amit,
On 3/12/19 4:22 AM, Amit Langote wrote:
I wrestled with this idea a bit and concluded that we don't have to
postpone *all* of preprocess_targetlist() processing to query_planner,
only the part that adds row mark "junk" Vars, because only those matter
for the problem being solved. To re
Amit-san,
On Fri, Mar 8, 2019 at 9:18 AM, Amit Langote wrote:
> On 2019/03/08 16:16, Imai, Yoshikazu wrote:
> > So I modified the code and did test to confirm memory increasement don't
> happen. The test and results are below.
> >
> > [test]
> > * Create partitioned table with 1536 partitions.
> >
Amit-san,
On Wed, Mar 6, 2019 at 5:38 AM, Amit Langote wrote:
...
> > I didn't investigate that problem, but there is another memory
> > increase
> issue, which is because of 0003 patch I think. I'll try to solve the latter
> issue.
>
> Interested in details as it seems to be a separate problem.
Hi,
On 3/5/19 5:24 AM, Amit Langote wrote:
Attached an updated version.
Need a rebase due to 898e5e3290.
Best regards,
Jesper
Amit-san,
On Wed, Mar 6, 2019 at 5:38 AM, Amit Langote wrote:
> On 2019/03/06 11:09, Imai, Yoshikazu wrote:
> > [0004 or 0005]
> > There are redundant process in add_appendrel_other_rels (or
> expand_xxx_rtentry()?).
> > I modified add_appendrel_other_rels like below and it actually worked.
> >
>
Imai-san,
Thanks for the review.
On 2019/03/06 11:09, Imai, Yoshikazu wrote:
> Here is the code review for previous v26 patches.
>
> [0002]
> In expand_inherited_rtentry():
>
> expand_inherited_rtentry()
> {
> ...
> + RelOptInfo *rel = NULL;
>
> can be declared at more later:
>
> if (ol
On Wed, Mar 6, 2019 at 2:10 AM, Imai, Yoshikazu wrote:
> > and Imai-san's review. I haven't been able to pin down the bug (or
> > whatever) that makes throughput go down as the partition count
> > increases, when tested with a --enable-cassert build.
>
> I didn't investigate that problem, but the
Amit-san,
On Tue, Mar 5, 2019 at 10:24 AM, Amit Langote wrote:
> On 2019/03/05 9:50, Amit Langote wrote:
> > I'll post the updated patches after diagnosing what I'm suspecting a
> > memory over-allocation bug in one of the patches. If you configure
> > build with --enable-cassert, you'll see that
On 2019/03/06 0:57, Jesper Pedersen wrote:
> On 3/5/19 5:24 AM, Amit Langote wrote:
>> Attached an updated version. This incorporates fixes for both Jesper's
>> and Imai-san's review. I haven't been able to pin down the bug (or
>> whatever) that makes throughput go down as the partition count inc
On 3/5/19 5:24 AM, Amit Langote wrote:
Attached an updated version. This incorporates fixes for both Jesper's
and Imai-san's review. I haven't been able to pin down the bug (or
whatever) that makes throughput go down as the partition count increases,
when tested with a --enable-cassert build.
On 2019/03/04 19:38, Amit Langote wrote:
> 2. Defer inheritance expansion to add_other_rels_to_query(). Although the
> purpose of doing this is to perform partition pruning before adding the
> children, this patch doesn't change when the pruning occurs. It deals
> with other issues that must be t
On 2019/03/05 9:50, Amit Langote wrote:
> I'll post the updated patches after diagnosing what
> I'm suspecting a memory over-allocation bug in one of the patches. If you
> configure build with --enable-cassert, you'll see that throughput
> decreases as number of partitions run into many thousands,
1 - 100 of 233 matches
Mail list logo
