Thank you to all involved.
> On Friday, March 15, 2013 12:52 AM Tom Lane wrote:
> > I wrote:
> > > ... So I think this patch is missing a bet by not
> > > accepting equal() expressions.
> >
> > I've committed this with that logic, a comment explaining exactly why
> > this is the way to do it, and
On Friday, March 15, 2013 12:52 AM Tom Lane wrote:
> I wrote:
> > ... So I think this patch is missing a bet by not
> > accepting equal() expressions.
>
> I've committed this with that logic, a comment explaining exactly why
> this is the way to do it, and some other cosmetic improvements.
Thank
On Thursday, March 14, 2013 8:35 PM Tom Lane wrote:
> I'm starting to review this patch now, and I'm having a hard time with
> this particular design decision:
>
> Amit Kapila writes:
> > We cannot directly compare expressions in target list as even if
> expressions
> > are equal, below node (ex.
I wrote:
> ... So I think this patch is missing a bet by not
> accepting equal() expressions.
I've committed this with that logic, a comment explaining exactly why
this is the way to do it, and some other cosmetic improvements.
regards, tom lane
--
Sent via pgsql-hacker
I'm starting to review this patch now, and I'm having a hard time with
this particular design decision:
Amit Kapila writes:
> We cannot directly compare expressions in target list as even if expressions
> are equal, below node (ex. APPEND) will
> not do projection, and hence expr will not be eval
On Friday, March 08, 2013 11:21 PM Heikki Linnakangas wrote:
On 12.02.2013 11:03, Amit Kapila wrote:
>> + /*
>> + * equivalent_tlists
>> + *returns whether two traget lists are equivalent
>> + *
>> + * We consider two target lists equivalent if both have
>> + * only Var entries and resjunk
On 12.02.2013 11:03, Amit Kapila wrote:
+ /*
+ * equivalent_tlists
+ * returns whether two traget lists are equivalent
+ *
+ * We consider two target lists equivalent if both have
+ * only Var entries and resjunk of each target entry is same.
+ *
+ * This function is used to decide wh
> > > I have updated the patch as per comments from Tom and Heikki.
> > > If you can verify it, then IMO it can be marked as 'Ready For
> > Committer'
> >
> > Would you please do that?
>
> Done.
Thank you.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hack
> On Friday, February 15, 2013 2:03 PM Kyotaro HORIGUCHI wrote:
> Hello,
>
> I've read the discussion held so far and am satisfied that apply
> this patch only for Result node. I applied the patch and found
> that it worked pretty fine for me.
>
> Thank you and I also think that we may send this
Hello,
I've read the discussion held so far and am satisfied that apply
this patch only for Result node. I applied the patch and found
that it worked pretty fine for me.
Thank you and I also think that we may send this to committers.
# It makes me fee ill at ease that the roles of us look invert
On Wednesday, February 13, 2013 8:12 AM Kyotaro HORIGUCHI wrote:
> Hello. Sorry for long absence.
>
> # I've lost my health and am not fully recovered..
>
> The direction of the discussion now taken place is just what I've
> wanted. The patch I proposed simply came from my poor
> understanding ab
Hello. Sorry for long absence.
# I've lost my health and am not fully recovered..
The direction of the discussion now taken place is just what I've
wanted. The patch I proposed simply came from my poor
understanding about exact how to detect identity projection by
comparing tlists, and I couldn't
On Saturday, February 09, 2013 9:03 AM Tom Lane wrote:
> Amit kapila writes:
> >> if (!is_projection_capable_plan(result_plan) &&
> compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
>
> > Sorry, the check I suggested in last mail should be as below:
>
> > if (!is_projection_capable_plan(
On Saturday, February 09, 2013 9:03 AM Tom Lane wrote:
Amit kapila writes:
>>> if (!is_projection_capable_plan(result_plan) &&
>>> compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
>> Sorry, the check I suggested in last mail should be as below:
>> if (!is_projection_capable_plan(result
Amit kapila writes:
>> if (!is_projection_capable_plan(result_plan) &&
>> compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
> Sorry, the check I suggested in last mail should be as below:
> if (!is_projection_capable_plan(result_plan) &&
> !compare_tlist_exprs(sub_tlist, result_plan->t
Saturday, February 09, 2013 6:56 AM Amit kapila wrote:
>Friday, February 08, 2013 11:06 PM Tom Lane wrote:
> >Amit Kapila writes:
>>> On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
>>> As per my understanding, currently in code wherever Result node can be
>>> avoided,
>> Hm. Really there'
Friday, February 08, 2013 11:06 PM Tom Lane wrote:
> Amit Kapila writes:
>> On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
>> As per my understanding, currently in code wherever Result node can be
>> avoided,
>> it calls function is_projection_capable_plan(), so we can even enhance
>> is_p
Amit Kapila writes:
> On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
> As per my understanding, currently in code wherever Result node can be
> avoided,
> it calls function is_projection_capable_plan(), so we can even enhance
> is_projection_capable_plan()
> so that it can also verify the
On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
> Amit Kapila writes:
> > There can be 2 ways to remove result node
> > a. Remove the Result plan node in case it is not required - This is
> same as
> > currently it does for SubqueryScan.
> >We can check if the result plan is trivial (wit
Amit Kapila writes:
> There can be 2 ways to remove result node
> a. Remove the Result plan node in case it is not required - This is same as
> currently it does for SubqueryScan.
>We can check if the result plan is trivial (with logic similar to
> trivial_subqueryscan()), then remove result
On Friday, December 14, 2012 5:11 PM Heikki Linnakangas wrote:
> On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
> > Hello, This is new version of identity projection patch.
> >
> > Reverted projectionInfo and ExecBuildProjectionInfo. Identity
> > projection is recognized directly in ExecGroup, ExecR
Kyotaro,
Are you planning to update this patch based on Heikki's comments? The
patch is listed in the commitfest and we're trying to make some
progress through all of those patches.
Thanks,
Stephen
* Heikki Linnakangas (hlinn...@iki.fi) wrote:
> On 12.11.2012 12:0
On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane.
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane..
The following is the result of performance te
Ah. It's too late. I'll re-submit updated versions of my patches
left alone in the last CF.
> Hi, I've marked this patch as Returned with Feedback (thanks Tom).
> Please submit an updated version to the upcoming commitfest. Thanks.
I'm sorry and thank you.
--
Kyotaro Horiguchi
NTT Open Source
Kyotaro HORIGUCHI wrote:
> Hello, sorry for long absense,
>
> # I had unexpected and urgent time-consuming tasks... :-(
>
> I have had a bit more precise inspection by two aspects, and they
> seemd showing that the difference should be the execution time of
> ExecProject.
>
> I'll be able to ba
Hello, sorry for long absense,
# I had unexpected and urgent time-consuming tasks... :-(
I have had a bit more precise inspection by two aspects, and they
seemd showing that the difference should be the execution time of
ExecProject.
I'll be able to back fully next week with reviesed patch, and
Kyotaro HORIGUCHI writes:
>> This patch seems quite unsafe to me: it's not generally okay for
>> a plan node to return a slot that doesn't belong to it, because of
>> tuple-lifespan issues. It's possible that Result in particular
>> could do that safely, but if so we ought to hack nodeResult.c fo
Although I said as following, the gain seems a bit larger... I'll
recheck the testing conditions...
> > Another point here is that the projection code already special-cases
> > simple projections, so it's a bit hard to believe that it's as slow as
> > you suggest above. I wonder if your test case
Hello, Thank you for suggestions.
> > This patch reduces run time of such queries by 45% when result
> > recored has 30 columns and seems to have no harm for performance.
>
> This patch seems quite unsafe to me: it's not generally okay for
> a plan node to return a slot that doesn't belong to it,
Kyotaro HORIGUCHI writes:
> This patch reduces run time of such queries by 45% when result
> recored has 30 columns and seems to have no harm for performance.
This patch seems quite unsafe to me: it's not generally okay for
a plan node to return a slot that doesn't belong to it, because of
tuple-
31 matches
Mail list logo
