From: | Surafel Temesgen <surafel3000(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>, andrew(at)tao11(dot)riddles(dot)org(dot)uk, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: FETCH FIRST clause WITH TIES option |
Date: | 2019-04-08 08:25:42 |
Message-ID: | CALAY4q8kK6XNBi2uD=9_gw1--H+4GjVODX2g_kMfhUQROAtZ8w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Apr 7, 2019 at 1:39 AM Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:
>
> 1) I've removed the costing changes. As Tom mentions elsewhere in this
> thread, that's probably not needed for v1 and it's true those estimates
> are probably somewhat sketchy anyway.
>
>
> 2) v8 did this (per my comment):
>
> - ExecSetTupleBound(compute_tuples_needed(node),
> outerPlanState(node));
> + if(node->limitOption == EXACT_NUMBER)
> + ExecSetTupleBound(compute_tuples_needed(node),
> outerPlanState(node));
>
> but I noticed the comment immediately above that says
>
> * Notify child node about limit. Note: think not to "optimize" by
> * skipping ExecSetTupleBound if compute_tuples_needed returns < 0. We
> * must update the child node anyway, in case this is a rescan and the
> * previous time we got a different result.
>
> which applies to WITH_TIES too, no? So I've modified compute_tuples_needed
> to return -1, and reverted this change. Not sure, though.
>
>
>
I agree that passing -1 to ExecSetTupleBound is more appropriate
implementation
3) I'm a bit confused by the initialization added to ExecInitLimit. It
> first gets the tuple descriptor from the limitstate (it should not do so
> directly but use ExecGetResultType). But when it creates the extra slot,
> it uses ops extracted from the outer plan. That's strange, I guess ...
>
> And then it extracts the descriptor from the outer plan and uses it when
> calling execTuplesMatchPrepare. But AFAIK it's going to be compared to
> the last_slot, which is using a descriptor from the limitstate.
>
> IMHO all of this should use descriptor/ops from the outer plan, no? It
> probably does not change anything because limit does not project, but it
> seems confusing.
>
agree
regards
Surafel
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2019-04-08 08:26:15 | Re: reloption to prevent VACUUM from truncating empty pages at the end of relation |
Previous Message | Fujii Masao | 2019-04-08 08:23:59 | Re: reloption to prevent VACUUM from truncating empty pages at the end of relation |