On Tue, Mar 7, 2017 at 10:28 PM, Robert Haas wrote:
>> Apart from this, there was one problem in match_unsorted_outer (in
>> v10), Basically, if inner_cheapest_total was not parallel_safe then I
>> was always getting parallel safe inner. But, we should not do anything
>> if jointype was JOIN_UNIQU
On Tue, Mar 7, 2017 at 11:38 AM, Dilip Kumar wrote:
> On Tue, Mar 7, 2017 at 7:47 PM, Robert Haas wrote:
>> You're right to be confused, because that seems to be a bug in the
>> existing code. There seems to be no guarantee that the cheapest
>> parallel-safe path will be in the cheapest_paramete
On Tue, Mar 7, 2017 at 7:47 PM, Robert Haas wrote:
> You're right to be confused, because that seems to be a bug in the
> existing code. There seems to be no guarantee that the cheapest
> parallel-safe path will be in the cheapest_parameterized_paths list.
> I'll go fix that.
Okay, Done the simm
On Tue, Mar 7, 2017 at 5:16 AM, Dilip Kumar wrote:
> I am confused about whether to call
> "get_cheapest_parallel_safe_total_inner" with
> innerrel->cheapest_parameterized_paths like we do in case of
> hash_inner_and_outer or with
> innerrel->pathlist. The reason behind I am calling this with com
On Tue, Mar 7, 2017 at 5:21 AM, Robert Haas wrote:
> +/* Can't do anything else if inner path needs to be unique'd */
> +if (save_jointype == JOIN_UNIQUE_INNER)
> +return;
>
> Right after this, you should try_partial_mergejoin_path() with the
> result of get_cheapest_parallel_safe_
On Mon, Mar 6, 2017 at 8:15 AM, Dilip Kumar wrote:
> On Fri, Mar 3, 2017 at 3:57 PM, Robert Haas wrote:
>> I'm not happy with the way this patch can just happen to latch on to a
>> path that's not parallel-safe rather than one that is and then just
>> give up on a merge join in that case. I alre
On Fri, Mar 3, 2017 at 3:57 PM, Robert Haas wrote:
> I'm not happy with the way this patch can just happen to latch on to a
> path that's not parallel-safe rather than one that is and then just
> give up on a merge join in that case. I already made this argument in
> https://www.postgresql.org/me
On Fri, Mar 3, 2017 at 9:47 PM, Dilip Kumar wrote:
> On Fri, Mar 3, 2017 at 3:57 PM, Robert Haas wrote:
>> I'm not happy with the way this patch can just happen to latch on to a
>> path that's not parallel-safe rather than one that is and then just
>> give up on a merge join in that case. I alre
On Fri, Mar 3, 2017 at 3:57 PM, Robert Haas wrote:
> I'm not happy with the way this patch can just happen to latch on to a
> path that's not parallel-safe rather than one that is and then just
> give up on a merge join in that case. I already made this argument in
> https://www.postgresql.org/me
On Wed, Mar 1, 2017 at 12:01 PM, Amit Kapila wrote:
> On Wed, Mar 1, 2017 at 11:24 AM, Dilip Kumar wrote:
>> On Wed, Mar 1, 2017 at 11:13 AM, Amit Kapila wrote:
>>> I think for now we can keep the parallel safety check for cheapest
>>> inner path, though it will be of use only for the very first
On Wed, Mar 1, 2017 at 11:24 AM, Dilip Kumar wrote:
> On Wed, Mar 1, 2017 at 11:13 AM, Amit Kapila wrote:
>> I think for now we can keep the parallel safety check for cheapest
>> inner path, though it will be of use only for the very first time we
>> compare the paths in that loop. I am not sure
On Wed, Mar 1, 2017 at 11:13 AM, Amit Kapila wrote:
> I think for now we can keep the parallel safety check for cheapest
> inner path, though it will be of use only for the very first time we
> compare the paths in that loop. I am not sure if there is any other
> better way to handle the same.
D
On Tue, Feb 28, 2017 at 9:28 PM, Dilip Kumar wrote:
> On Mon, Feb 27, 2017 at 10:27 AM, Amit Kapila wrote:
>> Okay, but in that case don't you think it is better to consider the
>> parallel safety of cheapest_total_inner only when we don't find any
>> cheap parallel_safe innerpath by reducing the
On Mon, Feb 27, 2017 at 10:27 AM, Amit Kapila wrote:
> Okay, but in that case don't you think it is better to consider the
> parallel safety of cheapest_total_inner only when we don't find any
> cheap parallel_safe innerpath by reducing the sort keys?
Well, we can do that but suppose cheapest_to
On Sun, Feb 26, 2017 at 12:01 PM, Robert Haas wrote:
> On Fri, Feb 24, 2017 at 3:54 PM, Amit Kapila wrote:
>> I agree in some cases it could be better, but I think benefits are not
>> completely clear, so probably we can leave it as of now and if later
>> any one comes with a clear use case or ca
On Fri, Feb 24, 2017 at 3:54 PM, Amit Kapila wrote:
> I agree in some cases it could be better, but I think benefits are not
> completely clear, so probably we can leave it as of now and if later
> any one comes with a clear use case or can see the benefits of such
> path then we can include it.
On Sat, Feb 25, 2017 at 3:22 PM, Dilip Kumar wrote:
> On Sat, Feb 25, 2017 at 11:29 AM, Amit Kapila wrote:
>> It seems you have forgotten to change in the below check:
>>
>> + if (innerpath != NULL &&
>> + innerpath->parallel_safe &&
>> + (cheapest_startup_inner == NULL ||
>> + cheapest_startup_i
On Sat, Feb 25, 2017 at 11:29 AM, Amit Kapila wrote:
> It seems you have forgotten to change in the below check:
>
> + if (innerpath != NULL &&
> + innerpath->parallel_safe &&
> + (cheapest_startup_inner == NULL ||
> + cheapest_startup_inner->parallel_safe == false ||
> + compare_path_costs(innerp
On Fri, Feb 24, 2017 at 4:49 PM, Dilip Kumar wrote:
> On Fri, Feb 24, 2017 at 3:04 PM, Amit Kapila wrote:
>> What advantage do you see for considering such a path when the cost of
>> innerpath is more than cheapest_total_inner? Remember the more paths
>> we try to consider, the more time we spen
On Fri, Feb 24, 2017 at 3:04 PM, Amit Kapila wrote:
> What advantage do you see for considering such a path when the cost of
> innerpath is more than cheapest_total_inner? Remember the more paths
> we try to consider, the more time we spend in the planner. By any
> chance are you able to generat
On Fri, Feb 24, 2017 at 3:42 PM, Dilip Kumar wrote:
> On Fri, Feb 24, 2017 at 3:04 PM, Amit Kapila wrote:
>
> Thanks for the comments.
>
>> What advantage do you see for considering such a path when the cost of
>> innerpath is more than cheapest_total_inner? Remember the more paths
>> we try to
On Fri, Feb 24, 2017 at 3:04 PM, Amit Kapila wrote:
Thanks for the comments.
> What advantage do you see for considering such a path when the cost of
> innerpath is more than cheapest_total_inner? Remember the more paths
> we try to consider, the more time we spend in the planner. By any
> cha
On Tue, Feb 14, 2017 at 5:22 PM, Dilip Kumar wrote:
> On Tue, Feb 14, 2017 at 12:25 PM, Amit Kapila wrote:
> Apart from this, there was one small problem in an earlier version
> which I have corrected in this.
>
> + /* Consider only parallel safe inner path */
> + if (innerpath != NULL &&
> + inn
On Tue, Feb 14, 2017 at 12:25 PM, Amit Kapila wrote:
> Few review comments on the latest version of the patch:
> 1.
> - if (joinrel->consider_parallel && nestjoinOK &&
> - save_jointype != JOIN_UNIQUE_OUTER &&
> - bms_is_empty(joinrel->lateral_relids))
> + if (outerrel->partial_pathlist == NIL ||
On Wed, Jan 4, 2017 at 9:27 PM, Dilip Kumar wrote:
> On Wed, Jan 4, 2017 at 12:02 PM, Amit Kapila wrote:
>> 3.
>> + /*
>> + * See comments in try_partial_nestloop_path().
>> + */
>>
>> This same code exists in try_partial_nestloop_path() and
>> try_partial_hashjoin_path() with minor difference o
On Thu, Jan 5, 2017 at 12:57 AM, Dilip Kumar wrote:
> On Wed, Jan 4, 2017 at 12:02 PM, Amit Kapila wrote:
>> Review comments:
>> 1.
>> + bool is_partial);
>> +
>>
>> Seems additional new line is not required.
> Fixed
This patch has a patch, no new reviews. Moved to CF 2017-03.
--
Michael
--
On Wed, Jan 4, 2017 at 12:02 PM, Amit Kapila wrote:
> Review comments:
> 1.
> + bool is_partial);
> +
>
> Seems additional new line is not required.
Fixed
>
> 2.
> + * try_partial_mergejoin_path
> + * Consider a partial merge join path; if it appears useful,
> push it into
> + * the joinrel's pa
On Wed, Dec 21, 2016 at 9:23 PM, Dilip Kumar wrote:
> On Wed, Dec 21, 2016 at 8:39 PM, Robert Haas wrote:
>> Committed the refactoring patch with some mild cosmetic adjustments.
>
> Thanks..
>>
>> As to the second patch:
>>
>> +if (jointype == JOIN_UNIQUE_INNER)
>> +jointype =
On Thu, Dec 29, 2016 at 3:15 AM, Tomas Vondra
wrote:
> FWIW, I've done quite a bit of testing on this patch, and also on the other
> patches adding parallel index scans and bitmap heap scan. I've been running
> TPC-H and TPC-DS on 16GB data sets with each patch, looking for regressions
> or crashe
Hi,
On 12/21/2016 04:53 PM, Dilip Kumar wrote:
On Wed, Dec 21, 2016 at 8:39 PM, Robert Haas wrote:
Committed the refactoring patch with some mild cosmetic adjustments.
Thanks..
As to the second patch:
+if (jointype == JOIN_UNIQUE_INNER)
+jointype = JOIN_INNER;
Isn't t
On Wed, Dec 21, 2016 at 8:39 PM, Robert Haas wrote:
> Committed the refactoring patch with some mild cosmetic adjustments.
Thanks..
>
> As to the second patch:
>
> +if (jointype == JOIN_UNIQUE_INNER)
> +jointype = JOIN_INNER;
>
> Isn't this dead code? save_jointype might that
On Tue, Dec 13, 2016 at 10:04 AM, Dilip Kumar wrote:
> On Tue, Dec 13, 2016 at 6:42 AM, Dilip Kumar wrote:
>>> This patch is hard to read because it is reorganizing a bunch of code
>>> as well as adding new functionality. Perhaps you could separate it
>>> into two patches, one with the refactori
On Tue, Dec 13, 2016 at 8:34 PM, Dilip Kumar wrote:
> I have created two patches as per the suggestion.
>
> 1. mergejoin_refactoring_v2.patch --> Move functionality of
> considering various merge join path into new function.
> 2. parallel_mergejoin_v2.patch -> This adds the functionality of
> supp
On Tue, Dec 13, 2016 at 6:42 AM, Dilip Kumar wrote:
>> This patch is hard to read because it is reorganizing a bunch of code
>> as well as adding new functionality. Perhaps you could separate it
>> into two patches, one with the refactoring and then the other with the
>> new functionality.
>
> Ok
On Tue, Dec 13, 2016 at 12:11 AM, Robert Haas wrote:
>
> Hmm, so it seems my initial guess that we didn't need to bother
> generating such paths was wrong. Oops.
>
> This patch is hard to read because it is reorganizing a bunch of code
> as well as adding new functionality. Perhaps you could sep
On Sat, Dec 10, 2016 at 7:59 AM, Dilip Kumar wrote:
> I would like to propose a patch for parallelizing merge join path.
> This idea is derived by analyzing TPCH results.
>
> I have done this analysis along with my colleagues Rafia sabih and Amit
> kaplia.
>
> Currently we already have infrastruc
On Sun, Dec 11, 2016 at 8:14 AM, Peter Geoghegan wrote:
> I noticed that the partially parallelized Merge Join EXPLAIN ANALYZE
> that you attach has a big misestimation:
>
> -> Merge Join (cost=3405052.45..3676948.66 rows=320 width=32)
> (actual time=21165.849..37814.551 rows=1357812 loops=4)
>
On Sat, Dec 10, 2016 at 4:59 AM, Dilip Kumar wrote:
> 3. 20_patch.out : Explain analyze output with patch (median of 3 runs)
I noticed that the partially parallelized Merge Join EXPLAIN ANALYZE
that you attach has a big misestimation:
-> Merge Join (cost=3405052.45..3676948.66 rows=320 width=
I would like to propose a patch for parallelizing merge join path.
This idea is derived by analyzing TPCH results.
I have done this analysis along with my colleagues Rafia sabih and Amit kaplia.
Currently we already have infrastructure for executing parallel join,
so we don't need any change at e
39 matches
Mail list logo
