On Mon, Oct 16, 2017 at 5:03 PM, Ashutosh Bapat
wrote:
> set_append_rel_size() crashes when it encounters a partitioned table
> with a dropped column. Dropped columns do not have any translations
> saved in AppendInfo::translated_vars; the corresponding entry is NULL
> per make_inh_translation_lis
On Sat, Oct 7, 2017 at 1:04 AM, Robert Haas wrote:
>>
>> The fix is to copy the relevant partitioning information from relcache
>> into PartitionSchemeData and RelOptInfo. Here's a quick patch with
>> that fix.
>
> Committed. I hope that makes things less red rather than more,
> because I'm goin
On Thu, Oct 12, 2017 at 10:49 PM, Robert Haas wrote:
> On Wed, Oct 11, 2017 at 10:43 PM, Ashutosh Bapat
> wrote:
>> You are suggesting that a dummy partitioned table be treated as an
>> un-partitioned table and apply above suggested optimization. A join
>> between a partitioned and unpartitioned
On Wed, Oct 11, 2017 at 10:43 PM, Ashutosh Bapat
wrote:
> You are suggesting that a dummy partitioned table be treated as an
> un-partitioned table and apply above suggested optimization. A join
> between a partitioned and unpartitioned table is partitioned by the
> keys of only partitioned table.
On Wed, Oct 11, 2017 at 7:47 PM, Robert Haas wrote:
> On Mon, Oct 9, 2017 at 2:05 AM, Ashutosh Bapat
> wrote:
>> On Sat, Oct 7, 2017 at 1:04 AM, Robert Haas wrote:
>>> Committed. I hope that makes things less red rather than more,
>>> because I'm going to be AFK for a few hours anyway.
>>
>> He
On Mon, Oct 9, 2017 at 2:05 AM, Ashutosh Bapat
wrote:
> On Sat, Oct 7, 2017 at 1:04 AM, Robert Haas wrote:
>> Committed. I hope that makes things less red rather than more,
>> because I'm going to be AFK for a few hours anyway.
>
> Here's the last patch, dealing with the dummy relations, rebased
On Sat, Oct 7, 2017 at 1:04 AM, Robert Haas wrote:
>
> Committed. I hope that makes things less red rather than more,
> because I'm going to be AFK for a few hours anyway.
>
Here's the last patch, dealing with the dummy relations, rebased. With
this fix every join order of a partitioned join can
On Fri, Oct 6, 2017 at 3:07 PM, Ashutosh Bapat
wrote:
> On Fri, Oct 6, 2017 at 8:45 PM, Robert Haas wrote:
>> I think this is very good work and I'm excited about the feature. Now
>> I'll wait to see whether the buildfarm, or Tom, yell at me for
>> whatever problems this may still have...
>
> Bu
On Fri, Oct 6, 2017 at 8:45 PM, Robert Haas wrote:
>
> I think this is very good work and I'm excited about the feature. Now
> I'll wait to see whether the buildfarm, or Tom, yell at me for
> whatever problems this may still have...
>
Buildfarm animal prion turned red. Before going into that fai
On Fri, Oct 6, 2017 at 8:45 PM, Robert Haas wrote:
> On Fri, Oct 6, 2017 at 8:40 AM, Ashutosh Bapat
> wrote:
>> Sorry. I sent a wrong file. Here's the real v37.
>
> Committed 0001-0006. I made some assorted comment and formatting
> changes and two small substantive changes:
>
> - In try_nestloop
On Fri, Oct 6, 2017 at 8:40 AM, Ashutosh Bapat
wrote:
> Sorry. I sent a wrong file. Here's the real v37.
Committed 0001-0006. I made some assorted comment and formatting
changes and two small substantive changes:
- In try_nestloop_path, bms_free(outerrelids) before returning if we
can't reparam
On Wed, Oct 4, 2017 at 9:04 PM, Robert Haas wrote:
> On Tue, Oct 3, 2017 at 3:27 PM, Robert Haas wrote:
>> I decided to skip over 0001 for today and spend some time looking at
>> 0002-0006.
>
> Back to 0001.
>
> +Enables or disables the query planner's use of partition-wise join
> +
On Thu, Oct 5, 2017 at 7:18 PM, Alvaro Herrera wrote:
> Robert Haas wrote:
>
>> Regarding nomenclature and my previous griping about wisdom, I was
>> wondering about just calling this a "partition join" like you have in
>> the regression test. So the GUC would be enable_partition_join, you'd
>> h
On Thu, Oct 5, 2017 at 9:48 AM, Alvaro Herrera wrote:
> Robert Haas wrote:
>> Regarding nomenclature and my previous griping about wisdom, I was
>> wondering about just calling this a "partition join" like you have in
>> the regression test. So the GUC would be enable_partition_join, you'd
>> hav
Robert Haas wrote:
> Regarding nomenclature and my previous griping about wisdom, I was
> wondering about just calling this a "partition join" like you have in
> the regression test. So the GUC would be enable_partition_join, you'd
> have generate_partition_join_paths(), etc. Basically just dele
On Wed, Oct 4, 2017 at 9:01 PM, Robert Haas wrote:
> On Thu, Sep 21, 2017 at 8:52 AM, Robert Haas wrote:
>> On Thu, Sep 21, 2017 at 8:21 AM, Ashutosh Bapat
>> wrote:
>>> About your earlier comment of making build_joinrel_partition_info()
>>> simpler. Right now, the code assumes that partexprs or
On Wed, Oct 4, 2017 at 8:23 AM, Ashutosh Bapat
wrote:
> I am not sure whether your assumption that expression with no Vars
> would have em_relids empty is correct. I wonder whether we will add
> any em_is_child members with empty em_relids; looking at
> process_equivalence() those come from Restri
On Wed, Oct 4, 2017 at 11:34 AM, Robert Haas wrote:
> +Enables or disables the query planner's use of partition-wise join
> +plans. When enabled, it spends time in creating paths for joins
> between
> +partitions and consumes memory to construct expression nodes to be
> u
On Tue, Oct 3, 2017 at 3:27 PM, Robert Haas wrote:
> I decided to skip over 0001 for today and spend some time looking at
> 0002-0006.
Back to 0001.
+Enables or disables the query planner's use of partition-wise join
+plans. When enabled, it spends time in creating paths for join
On Thu, Sep 21, 2017 at 8:52 AM, Robert Haas wrote:
> On Thu, Sep 21, 2017 at 8:21 AM, Ashutosh Bapat
> wrote:
>> About your earlier comment of making build_joinrel_partition_info()
>> simpler. Right now, the code assumes that partexprs or
>> nullable_partexpr can be NULL when either of them is n
On Wed, Oct 4, 2017 at 12:57 AM, Robert Haas wrote:
>
> 0003:
>
> The commit message mentions estimate_num_groups but the patch doesn't touch
> it.
This was fixed when we converted many rel->reloptkind ==
RELOPT_BASEREL to IS_SIMPLE_REL(). I have removed this section from
the commit message.
>
On 2017/10/04 4:27, Robert Haas wrote:
> On Tue, Oct 3, 2017 at 8:57 AM, Ashutosh Bapat
> wrote:
>>> Regarding nomenclature and my previous griping about wisdom, I was
>>> wondering about just calling this a "partition join" like you have in
>>> the regression test. So the GUC would be enable_par
On Tue, Oct 3, 2017 at 8:57 AM, Ashutosh Bapat
wrote:
>> Regarding nomenclature and my previous griping about wisdom, I was
>> wondering about just calling this a "partition join" like you have in
>> the regression test. So the GUC would be enable_partition_join, you'd
>> have generate_partition_
On Thu, Sep 21, 2017 at 8:21 AM, Ashutosh Bapat
wrote:
> Here's set of rebased patches. The patch with extra tests is not for
> committing. All other patches, except the last one, will need to be
> committed together. The last patch may be committed along with other
> patches or as a separate patc
On Fri, Sep 15, 2017 at 5:29 PM, Ashutosh Bapat
wrote:
>
>>
>> Apart from these there is a regression case on a custom table, on head
>> query completes in 20s and with this patch it takes 27s. Please find
>> the attached .out and .sql file for the output and schema for the test
>> case respective
On Fri, Sep 22, 2017 at 10:45 AM, Rafia Sabih
wrote:
>>
>> On completing the benchmark for all queries for the above mentioned
>> setup, following performance improvement can be seen,
>> Query | Patch | Head
>> 3 | 1455 | 1631
>> 4 | 499 | 4344
>> 5 | 1464 | 1606
>> 10 | 1475 | 1599
On Mon, Sep 18, 2017 at 10:18 AM, Rafia Sabih
wrote:
>>
>
> Limit (cost=83341943.28..83341943.35 rows=1 width=92) (actual
> time=1556989.996..1556989.997 rows=1 loops=1)
>-> Finalize GroupAggregate (cost=83341943.28..83342723.24
> rows=10064 width=92) (actual time=1556989.994..1556989.994
On Thu, Sep 21, 2017 at 8:21 AM, Ashutosh Bapat
wrote:
> About your earlier comment of making build_joinrel_partition_info()
> simpler. Right now, the code assumes that partexprs or
> nullable_partexpr can be NULL when either of them is not populated.
> That may be saves a sizeof(pointer) * (numbe
On Tue, Sep 19, 2017 at 5:47 AM, Ashutosh Bapat
wrote:
> Done.
Committed 0001 with extensive editorialization. I did not think it
was a good idea to include a partition.h a file in src/include/nodes,
so I worked around that. The include of pg_inherits_fn.h was
unneeded. I rewrote a lot of the
On Wed, Sep 20, 2017 at 3:13 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
> On Wed, Sep 20, 2017 at 9:44 AM, Thomas Munro
> wrote:
> > 2. What queries in the 0008 patch are hitting lines that 0007 doesn't
> hit?
> >
> > I thought about how to answer questions like this and came u
On Tue, Sep 19, 2017 at 3:17 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
> On Tue, Sep 19, 2017 at 2:35 AM, Robert Haas
> wrote:
> > On Mon, Sep 18, 2017 at 8:02 AM, Ashutosh Bapat
> > wrote:
> >> partition pruning might need partexprs look up relevant quals, but
> >> nullable_p
On Tue, Sep 19, 2017 at 3:17 PM, Ashutosh Bapat
wrote:
>>
- I'm not entirely sure whether maintaining partexprs and
nullable_partexprs is the right design. If I understand correctly,
whether or not a partexpr is nullable is really a per-RTI property,
not a per-expression prope
On Wed, Sep 20, 2017 at 9:44 AM, Thomas Munro
wrote:
>
> The main areas of uncovered lines are: code in
> get_wholerow_ref_from_convert_row_type() and code that calls it, and
> per node type cases in reparameterize_path_by_child(). It seems like
> the former could use a test case, and I wonder if
On Tue, Sep 19, 2017 at 3:50 PM, Alvaro Herrera wrote:
> Rafia Sabih wrote:
>
>> On completing the benchmark for all queries for the above mentioned
>> setup, following performance improvement can be seen,
>> Query | Patch | Head
>> 3 | 1455 | 1631
>> 4 | 499 | 4344
>> 5 | 1464 | 1606
>
Rafia Sabih wrote:
> On completing the benchmark for all queries for the above mentioned
> setup, following performance improvement can be seen,
> Query | Patch | Head
> 3 | 1455 | 1631
> 4 | 499 | 4344
> 5 | 1464 | 1606
> 10 | 1475 | 1599
> 12 | 1465 | 1790
>
> Note that all v
On Mon, Sep 18, 2017 at 8:02 AM, Ashutosh Bapat
wrote:
> partition pruning might need partexprs look up relevant quals, but
> nullable_partexprs doesn't have any use there. So may be we should add
> nullable_partexpr to RelOptInfo as part of 0002 (partition-wise join
> implementation) instead of 0
On Sat, Sep 16, 2017 at 2:53 AM, Robert Haas wrote:
> On Fri, Sep 15, 2017 at 6:11 AM, Ashutosh Bapat
> wrote:
>> Thanks a lot Robert.
>>
>> Here are rebased patches.
>
> I didn't get quite as much time to look at these today as I would have
> liked, but here's what I've got so far.
>
> Comments
On Sat, Sep 16, 2017 at 9:23 AM, Robert Haas wrote:
> On the overall patch set:
>
> - I am curious to know how this has been tested. How much of the new
> code is covered by the tests in 0007-Partition-wise-join-tests.patch?
> How much does coverage improve with
> 0008-Extra-testcases-for-partiti
On Fri, Sep 15, 2017 at 6:11 AM, Ashutosh Bapat
wrote:
> Thanks a lot Robert.
>
> Here are rebased patches.
I didn't get quite as much time to look at these today as I would have
liked, but here's what I've got so far.
Comments on 0001:
- In the RelOptInfo, part_oids is defined in a completely
On Fri, Sep 15, 2017 at 2:09 PM, Rafia Sabih
wrote:
> On TPC-H benchmarking of this patch, I found a regression in Q7. It
> was taking some 1500s with the patch and some 900s without the patch.
> Please find the attached pwd_reg.zip for the output of explain analyse
> on head and with patch.
>
> T
On 2017/09/15 4:43, Robert Haas wrote:
> On Thu, Sep 14, 2017 at 8:06 AM, Ashutosh Bapat
> wrote:
>> I have few changes to multi-level expansion patch as per discussion in
>> earlier mails
>
> OK, I have committed
> 0002-Multi-level-partitioned-table-expansion.patch with a few cosmetic
> changes.
On Thu, Sep 14, 2017 at 8:06 AM, Ashutosh Bapat
wrote:
> I have few changes to multi-level expansion patch as per discussion in
> earlier mails
OK, I have committed
0002-Multi-level-partitioned-table-expansion.patch with a few cosmetic
changes.
Phew, getting that sorted out has been an astonishi
On Wed, Sep 13, 2017 at 10:57 PM, Amit Langote
wrote:
> I very much like pcinfo-for-subquery.patch, although I'm not sure if we
> need to create PartitionedChildRelInfo for the sub-query parent RTE as the
> patch teaches add_paths_to_append_rel() to do. ISTM, nested UNION ALL
> subqueries are fla
On 2017/09/14 7:43, Robert Haas wrote:
> On Wed, Sep 13, 2017 at 12:56 PM, Ashutosh Bapat
> wrote:
>> I debugged what happens in case of query "select 1 from t1 union all
>> select 2 from t1;" with the current HEAD (without multi-level
>> expansion patch attached). It doesn't set partitioned_rels
On Wed, Sep 13, 2017 at 12:56 PM, Ashutosh Bapat
wrote:
> I debugged what happens in case of query "select 1 from t1 union all
> select 2 from t1;" with the current HEAD (without multi-level
> expansion patch attached). It doesn't set partitioned_rels in Append
> path that gets converted into Appe
On Wed, Sep 13, 2017 at 12:51 PM, Ashutosh Bapat
wrote:
> On Wed, Sep 13, 2017 at 12:39 AM, Robert Haas wrote:
>> On Tue, Sep 12, 2017 at 3:46 AM, Amit Langote
>> wrote:
>>> In this case, AcquireExecutorLocks will lock all the relations in
>>> PlannedStmt.rtable, which must include all partition
On 13 September 2017 at 13:05, Ashutosh Bapat
wrote:
> On Wed, Sep 13, 2017 at 12:32 PM, Amit Khandekar
> wrote:
>> Hi,
>>
>> Rafia had done some testing on TPCH queries using Partition-wise join
>> patch along with Parallel Append patch.
>>
>> There, we had observed that for query 4, even thoug
On 2017/09/13 16:21, Ashutosh Bapat wrote:
> On Wed, Sep 13, 2017 at 12:39 AM, Robert Haas wrote:
>> locks taken from the executor are worthless because plancache.c will
>> always do the job for us. I don't know of a case where we execute a
>> saved plan without going through the plan cache, but
On Wed, Sep 13, 2017 at 12:32 PM, Amit Khandekar wrote:
> Hi,
>
> Rafia had done some testing on TPCH queries using Partition-wise join
> patch along with Parallel Append patch.
>
> There, we had observed that for query 4, even though the partition
> wise joins are under a Parallel Append, the joi
On Wed, Sep 13, 2017 at 11:29 AM, Amit Langote
wrote:
> On 2017/09/12 19:56, Ashutosh Bapat wrote:
>> I think the code here expects the original parent_rte and not the one
>> we set around line 1169.
>>
>> This isn't a bug right now, since both the parent_rte s have same
>> content. But I am not s
On Wed, Sep 13, 2017 at 12:39 AM, Robert Haas wrote:
> On Tue, Sep 12, 2017 at 3:46 AM, Amit Langote
> wrote:
>> In this case, AcquireExecutorLocks will lock all the relations in
>> PlannedStmt.rtable, which must include all partitioned tables of all
>> partition trees involved in the query. Of
Hi,
Rafia had done some testing on TPCH queries using Partition-wise join
patch along with Parallel Append patch.
There, we had observed that for query 4, even though the partition
wise joins are under a Parallel Append, the join are all non-partial.
Specifically, the partition-wise join has non
On 2017/09/12 19:56, Ashutosh Bapat wrote:
> I think the code here expects the original parent_rte and not the one
> we set around line 1169.
>
> This isn't a bug right now, since both the parent_rte s have same
> content. But I am not sure if that will remain to be so. Here's patch
> to fix the t
On Tue, Sep 12, 2017 at 3:46 AM, Amit Langote
wrote:
> In this case, AcquireExecutorLocks will lock all the relations in
> PlannedStmt.rtable, which must include all partitioned tables of all
> partition trees involved in the query. Of those, it will lock the tables
> whose RT indexes appear in P
On Tue, Sep 12, 2017 at 2:35 PM, Amit Langote
wrote:
> On 2017/09/12 17:53, Ashutosh Bapat wrote:
>> On Tue, Sep 12, 2017 at 1:42 PM, Amit Langote wrote:
>>> So, we can remove partitioned_rels from (Merge)AppendPath and
>>> (Merge)Append nodes and remove ExecLockNonLeafAppendTables().
>>
>> Don't
On 2017/09/12 18:49, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 2:17 PM, Amit Langote
> wrote:
>>
>> That said, I noticed that we might need to be careful about what the value
>> of the root parent's PlanRowMark's allMarkType field gets set to. We need
>> to make sure that it reflects markTy
On Tue, Sep 12, 2017 at 2:17 PM, Amit Langote
wrote:
>
> That said, I noticed that we might need to be careful about what the value
> of the root parent's PlanRowMark's allMarkType field gets set to. We need
> to make sure that it reflects markType of all partitions in the tree,
> including those
On 2017/09/12 17:53, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 1:42 PM, Amit Langote wrote:
>> So, we can remove partitioned_rels from (Merge)AppendPath and
>> (Merge)Append nodes and remove ExecLockNonLeafAppendTables().
>
> Don't we need partitioned_rels from Append paths to be transferred
On Tue, Sep 12, 2017 at 1:42 PM, Amit Langote
wrote:
> On 2017/09/12 16:55, Ashutosh Bapat wrote:
>> On Tue, Sep 12, 2017 at 1:16 PM, Amit Langote wrote:
>>> So I looked at this a bit closely and came to the conclusion that we may
>>> not need to keep partitioned table RT indexes in the
>>> (Merge
On 2017/09/12 16:39, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 7:31 AM, Amit Langote
> wrote:
>> On 2017/09/11 19:45, Ashutosh Bapat wrote:
>>> On Mon, Sep 11, 2017 at 12:16 PM, Amit Langote wrote:
IMHO, we should make it the responsibility of the future patch to set a
child PlanRo
On 2017/09/12 16:55, Ashutosh Bapat wrote:
> On Tue, Sep 12, 2017 at 1:16 PM, Amit Langote wrote:
>> So I looked at this a bit closely and came to the conclusion that we may
>> not need to keep partitioned table RT indexes in the
>> (Merge)Append.partitioned_rels after all, as far as execution-time
On Tue, Sep 12, 2017 at 1:16 PM, Amit Langote
wrote:
> On 2017/09/11 21:07, Ashutosh Bapat wrote:
>> On Mon, Sep 11, 2017 at 5:19 PM, Robert Haas wrote:
>>> On Mon, Sep 11, 2017 at 6:45 AM, Ashutosh Bapat
>>> wrote:
So, all partitioned partitions are getting locked correctly. Am I
miss
On 2017/09/11 21:07, Ashutosh Bapat wrote:
> On Mon, Sep 11, 2017 at 5:19 PM, Robert Haas wrote:
>> On Mon, Sep 11, 2017 at 6:45 AM, Ashutosh Bapat
>> wrote:
>>> So, all partitioned partitions are getting locked correctly. Am I
>>> missing something?
>>
>> That's not a valid test. In that scenar
On Tue, Sep 12, 2017 at 7:31 AM, Amit Langote
wrote:
> On 2017/09/11 19:45, Ashutosh Bapat wrote:
>> On Mon, Sep 11, 2017 at 12:16 PM, Amit Langote wrote:
>>> IMHO, we should make it the responsibility of the future patch to set a
>>> child PlanRowMark's prti to the direct parent's RT index, when
On 2017/09/11 19:45, Ashutosh Bapat wrote:
> On Mon, Sep 11, 2017 at 12:16 PM, Amit Langote wrote:
>> IMHO, we should make it the responsibility of the future patch to set a
>> child PlanRowMark's prti to the direct parent's RT index, when we actually
>> know that it's needed for something. We cle
On Mon, Sep 11, 2017 at 8:07 AM, Ashutosh Bapat
wrote:
> I see the same thing when I use prepare and execute
Hmm. Well, that's good, but it doesn't prove there's no bug. We have
to understand where and why it's getting locked to know whether the
behavior will be correct in all cases. I haven't
On Mon, Sep 11, 2017 at 5:19 PM, Robert Haas wrote:
> On Mon, Sep 11, 2017 at 6:45 AM, Ashutosh Bapat
> wrote:
>> So, all partitioned partitions are getting locked correctly. Am I
>> missing something?
>
> That's not a valid test. In that scenario, you're going to hold all
> the locks acquired b
On Mon, Sep 11, 2017 at 6:45 AM, Ashutosh Bapat
wrote:
> So, all partitioned partitions are getting locked correctly. Am I
> missing something?
That's not a valid test. In that scenario, you're going to hold all
the locks acquired by the planner, all the locks acquired by the
rewriter, and all t
On Mon, Sep 11, 2017 at 2:16 PM, Amit Langote
wrote:
> On 2017/09/11 16:23, Ashutosh Bapat wrote:
>> On Sat, Sep 9, 2017 at 6:28 AM, Robert Haas wrote:
>>> I'm a bit suspicious about the fact that there are now executor
>>> changes related to the PlanRowMarks. If the rowmark's prti is now the
>>
On Mon, Sep 11, 2017 at 12:16 PM, Amit Langote
wrote:
> On 2017/09/09 2:38, Ashutosh Bapat wrote:
>> On Fri, Sep 8, 2017 at 11:17 AM, Amit Langote wrote:
>>> I updated the patch to include just those changes. I'm not sure about
>>> one of the Ashutosh's changes whereby the child PlanRowMark is al
On 2017/09/11 16:23, Ashutosh Bapat wrote:
> On Sat, Sep 9, 2017 at 6:28 AM, Robert Haas wrote:
>> I'm a bit suspicious about the fact that there are now executor
>> changes related to the PlanRowMarks. If the rowmark's prti is now the
>> intermediate parent's RT index rather than the top-parent'
On Sat, Sep 9, 2017 at 6:28 AM, Robert Haas wrote:
> On Fri, Sep 8, 2017 at 1:38 PM, Ashutosh Bapat
> wrote:
>>> I also confirmed that the partition-pruning patch set works fine with this
>>> patch instead of the patch on that thread with the same functionality,
>>> which I will now drop from tha
On 2017/09/09 9:58, Robert Haas wrote:
> I'm a bit suspicious about the fact that there are now executor
> changes related to the PlanRowMarks. If the rowmark's prti is now the
> intermediate parent's RT index rather than the top-parent's RT index,
> it'd seem like that'd matter somehow. Maybe it
On 2017/09/09 2:38, Ashutosh Bapat wrote:
> On Fri, Sep 8, 2017 at 11:17 AM, Amit Langote wrote:
>> I updated the patch to include just those changes. I'm not sure about
>> one of the Ashutosh's changes whereby the child PlanRowMark is also passed
>> to expand_partitioned_rtentry() to use as the p
On Fri, Sep 8, 2017 at 1:38 PM, Ashutosh Bapat
wrote:
>> I also confirmed that the partition-pruning patch set works fine with this
>> patch instead of the patch on that thread with the same functionality,
>> which I will now drop from that patch set. Sorry about the wasted time.
>
> Thanks a lot
On Fri, Sep 8, 2017 at 1:47 AM, Amit Langote
wrote:
> When I tried the attached patch, it doesn't seem to expand partitioning
> inheritance in step-wise manner as the patch's title says. I think the
> rewritten patch forgot to include Ashutosh's changes to
> expand_single_inheritance_child() wher
On Fri, Sep 8, 2017 at 12:34 AM, Robert Haas wrote:
> On Tue, Sep 5, 2017 at 7:01 AM, Ashutosh Bapat
> wrote:
>> accumulate_append_subpath() is executed for every path instead of
>> every relation, so changing it would collect the same list multiple
>> times. Instead, I found the old way of assoc
On 2017/09/08 14:47, Amit Langote wrote:
> When I tried the attached patch, it doesn't seem to expand partitioning
> inheritance in step-wise manner as the patch's title says.
Oops. By "attached patch", I had meant to say the Robert's patch, that
is, expand-stepwise-rmh.patch. Not expand-stepwis
On 2017/09/08 4:04, Robert Haas wrote:
> On Tue, Sep 5, 2017 at 7:01 AM, Ashutosh Bapat
> wrote:
>> accumulate_append_subpath() is executed for every path instead of
>> every relation, so changing it would collect the same list multiple
>> times. Instead, I found the old way of associating all int
On Tue, Sep 5, 2017 at 7:01 AM, Ashutosh Bapat
wrote:
> accumulate_append_subpath() is executed for every path instead of
> every relation, so changing it would collect the same list multiple
> times. Instead, I found the old way of associating all intermediate
> partitions with the root partition
On Thu, Sep 7, 2017 at 4:32 PM, Antonin Houska wrote:
> Ashutosh Bapat wrote:
>
>> On Fri, Sep 1, 2017 at 6:05 PM, Antonin Houska wrote:
>> >
>> >
>> >
>> > * get_partitioned_child_rels_for_join()
>> >
>> > I think the Assert() statement is easier to understand inside the loop, see
>> > the asse
Ashutosh Bapat wrote:
> On Fri, Sep 1, 2017 at 6:05 PM, Antonin Houska wrote:
> >
> >
> >
> > * get_partitioned_child_rels_for_join()
> >
> > I think the Assert() statement is easier to understand inside the loop, see
> > the assert.diff attachment.
> The assert at the end of function also chec
On Tue, Sep 5, 2017 at 1:16 PM, Amit Langote
wrote:
> On 2017/09/05 15:43, Ashutosh Bapat wrote:
>> Ok. Can you please answer my previous questions?
>>
>> AFAIU, the list contained RTIs of the relations, which didnt' have
>> corresponding AppendRelInfos to lock those relations. Now that we
>> crea
On 2017/09/05 15:43, Ashutosh Bapat wrote:
> Ok. Can you please answer my previous questions?
>
> AFAIU, the list contained RTIs of the relations, which didnt' have
> corresponding AppendRelInfos to lock those relations. Now that we
> create AppendRelInfos even for partitioned partitions with my 0
On Tue, Sep 5, 2017 at 12:06 PM, Amit Langote
wrote:
> On 2017/09/05 15:30, Ashutosh Bapat wrote:
>> Those changes are already part of my updated 0001 patch. Aren't they?
>> May be you should just review those and see if those are suitable for
>> you?
>
> Yeah, I think it's going to be the same pa
On 2017/09/05 15:30, Ashutosh Bapat wrote:
> Those changes are already part of my updated 0001 patch. Aren't they?
> May be you should just review those and see if those are suitable for
> you?
Yeah, I think it's going to be the same patch, functionality-wise.
And sorry, I didn't realize you were
On 2017/09/05 13:20, Amit Langote wrote:
On 2017/09/04 21:32, Ashutosh Bapat wrote:
+1. Will Fujita-san's patch also handle getting rid of partitioned_rels list?
As Fujita-san mentioned, his patch won't. Actually, I suppose he didn't
say that partitioned_rels itself is useless, just that it
On Tue, Sep 5, 2017 at 11:54 AM, Amit Langote
wrote:
> On 2017/09/05 13:20, Amit Langote wrote:
>> The later phase can
>> build that list from the AppendRelInfos that you mention we now [1] build.
>>
>> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=30833ba154
>
> Looking at th
On 2017/09/05 13:20, Amit Langote wrote:
> The later phase can
> build that list from the AppendRelInfos that you mention we now [1] build.
>
> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=30833ba154
Looking at that commit again, AppendRelInfos are still not created for
part
On 2017/09/04 21:32, Ashutosh Bapat wrote:
> On Mon, Sep 4, 2017 at 10:04 AM, Amit Langote wrote:
>> By the way, if you want to get rid of PartitionedChildRelInfo, you can do
>> that as long as you find some other way of putting together the
>> partitioned_rels list to add into the ModifyTable (App
On 2017/09/04 21:32, Ashutosh Bapat wrote:
On Mon, Sep 4, 2017 at 10:04 AM, Amit Langote
By the way, if you want to get rid of PartitionedChildRelInfo, you can do
that as long as you find some other way of putting together the
partitioned_rels list to add into the ModifyTable (Append/MergeAppend
On Mon, Sep 4, 2017 at 10:04 AM, Amit Langote
wrote:
> On 2017/09/04 12:38, Etsuro Fujita wrote:
>> On 2017/09/02 4:10, Ashutosh Bapat wrote:
>>> This rebase mainly changes patch 0001, which translates partition
>>> hierarchy into inheritance hierarchy creating AppendRelInfos and
>>> RelOptInfos f
On 2017/09/04 12:38, Etsuro Fujita wrote:
> On 2017/09/02 4:10, Ashutosh Bapat wrote:
>> This rebase mainly changes patch 0001, which translates partition
>> hierarchy into inheritance hierarchy creating AppendRelInfos and
>> RelOptInfos for partitioned partitions. Because of that, it's not
>> nece
On 2017/09/02 4:10, Ashutosh Bapat wrote:
This rebase mainly changes patch 0001, which translates partition
hierarchy into inheritance hierarchy creating AppendRelInfos and
RelOptInfos for partitioned partitions. Because of that, it's not
necessary to record the partitioned partitions in a
Partit
Ashutosh Bapat wrote:
> I originally thought to provide it along-with the changes to
> expand_inherited_rtentry(), but that thread is taking longer. Jeevan
> Chalke needs rebased patches for his work on aggregate pushdown and
> Thomas might need them for further review. So, here they are.
Since
On Wed, Aug 16, 2017 at 3:31 AM, Ashutosh Bapat
wrote:
> There are two ways we can do this
> 1. In expand_inherited_rtentry(), remember (childRTE and childRTIndex)
> or just childRTIndex (using this we can fetch childRTE calling
> rtfetch()) of intermediate partitioned tables. Once we are done
> e
On Tue, Aug 15, 2017 at 10:15 PM, Robert Haas wrote:
> On Thu, Aug 10, 2017 at 8:00 AM, Ashutosh Bapat
> wrote:
>> Attached patches with the comments addressed.
>
> I have committed 0001-0003 as 480f1f4329f1bf8bfbbcda8ed233851e1b110ad4
> and e139f1953f29db245f60a7acb72fccb1e05d2442.
Thanks a lot
On Thu, Aug 10, 2017 at 8:00 AM, Ashutosh Bapat
wrote:
> Attached patches with the comments addressed.
I have committed 0001-0003 as 480f1f4329f1bf8bfbbcda8ed233851e1b110ad4
and e139f1953f29db245f60a7acb72fccb1e05d2442.
0004 doesn't apply any more, probably due to commit
d57929afc7063431f80a0ac4
On Thu, Aug 10, 2017 at 5:43 AM, Thomas Munro
wrote:
>> Do you think we solving this problem is a prerequisite for
>> partition-wise join? Or should we propose that patch as a separate
>> enhancement?
>
> No, I'm not proposing anything yet. For now I just wanted to share
> this observation about
On Thu, Aug 10, 2017 at 3:13 PM, Thomas Munro
wrote:
> On Thu, Aug 10, 2017 at 6:23 PM, Ashutosh Bapat
> wrote:
>> Your patch didn't improve planning time without partition-wise join,
>> so it's something good to have along-with partition-wise join. Given
>> that Bitmapsets are used in other part
1 - 100 of 240 matches
Mail list logo
