Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-12-05 Thread Robert Haas
On Sat, Dec 3, 2016 at 10:30 AM, Dilip Kumar wrote: >> I'll post my new expression evaluation stuff - which doesn't do this >> atm, but makes ExecQual faster in other ways - later this week. If we >> could get the planner (or parse-analysis?) to set an OpExpr flag that >> signals that the express

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-12-03 Thread Dilip Kumar
On Fri, Dec 2, 2016 at 3:11 AM, Andres Freund wrote: > Hm. I'm more than a bit doubful about this approach. Shouldn't we just > *always* do this as part of expression evaluation, instead of > special-casing for seqscans? That make sense, we can actually do this as part of expression evaluation an

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-12-02 Thread Haribabu Kommi
On Fri, Dec 2, 2016 at 8:41 AM, Andres Freund wrote: > On 2016-11-30 16:11:23 +0530, Dilip Kumar wrote: > > On Tue, Nov 29, 2016 at 11:21 PM, Robert Haas > wrote: > > > On Mon, Nov 28, 2016 at 11:17 PM, Dilip Kumar > wrote: > > >> Actually we want to call slot_getattr instead heap_getattr, beca

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-12-01 Thread Andres Freund
On 2016-11-30 16:11:23 +0530, Dilip Kumar wrote: > On Tue, Nov 29, 2016 at 11:21 PM, Robert Haas wrote: > > On Mon, Nov 28, 2016 at 11:17 PM, Dilip Kumar wrote: > >> Actually we want to call slot_getattr instead heap_getattr, because of > >> problem mentioned by Andres upthread and we also saw in

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-12-01 Thread Robert Haas
On Wed, Nov 30, 2016 at 5:41 AM, Dilip Kumar wrote: > 1. As we decided to separate scankey and qual during planning time. So > I am doing it at create_seqscan_path. My question is currently we > don't have path node for seqscan, so where should we store scankey ? > In Path node, or create new SeqS

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-30 Thread Dilip Kumar
On Tue, Nov 29, 2016 at 11:21 PM, Robert Haas wrote: > On Mon, Nov 28, 2016 at 11:17 PM, Dilip Kumar wrote: >> Actually we want to call slot_getattr instead heap_getattr, because of >> problem mentioned by Andres upthread and we also saw in test results. > > Ah, right. > >> Should we make a copy

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-29 Thread Robert Haas
On Mon, Nov 28, 2016 at 11:17 PM, Dilip Kumar wrote: > Actually we want to call slot_getattr instead heap_getattr, because of > problem mentioned by Andres upthread and we also saw in test results. Ah, right. > Should we make a copy of HeapKeyTest lets say ExecKeyTest and keep it > under executo

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-29 Thread Robert Haas
On Mon, Nov 28, 2016 at 11:25 PM, Andres Freund wrote: > On 2016-11-28 09:55:00 -0500, Robert Haas wrote: >> I think we should go with this approach. I don't think it's a good >> idea to have the heapam layer know about executor slots. > > I agree that that's not pretty. > >> Even though >> it's

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-28 Thread Andres Freund
On 2016-11-28 09:55:00 -0500, Robert Haas wrote: > I think we should go with this approach. I don't think it's a good > idea to have the heapam layer know about executor slots. I agree that that's not pretty. > Even though > it's a little sad to pass up an opportunity for a larger performance >

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-28 Thread Dilip Kumar
On Mon, Nov 28, 2016 at 8:25 PM, Robert Haas wrote: > I think we should go with this approach. I don't think it's a good > idea to have the heapam layer know about executor slots. Even though > it's a little sad to pass up an opportunity for a larger performance > improvement, this improvement i

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-28 Thread Robert Haas
On Mon, Nov 28, 2016 at 4:30 AM, Dilip Kumar wrote: > On Sat, Nov 19, 2016 at 6:48 PM, Dilip Kumar wrote: >> patch1: Original patch (heap_scankey_pushdown_v1.patch), only >> supported for fixed length datatype and use heap_getattr. >> >> patch2: Switches memory context in HeapKeyTest + Store tupl

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-28 Thread Dilip Kumar
On Mon, Nov 28, 2016 at 3:00 PM, Dilip Kumar wrote: > As promised, I have taken the performance with TPCH benchmark and > still result are quite good. However this are less compared to older > version (which was exposing expr ctx and slot to heap). > > Query Head [1] Patch3

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-28 Thread Dilip Kumar
On Sat, Nov 19, 2016 at 6:48 PM, Dilip Kumar wrote: > patch1: Original patch (heap_scankey_pushdown_v1.patch), only > supported for fixed length datatype and use heap_getattr. > > patch2: Switches memory context in HeapKeyTest + Store tuple in slot > and use slot_getattr instead of heap_getattr. >

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-19 Thread Dilip Kumar
On Mon, Nov 14, 2016 at 9:44 PM, Dilip Kumar wrote: >> Also, what if we abandoned the idea of pushing qual evaluation all the >> way down into the heap and just tried to do HeapKeyTest in SeqNext >> itself? Would that be almost as fast, or would it give up most of the >> benefits? > This we can d

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-14 Thread Dilip Kumar
On Mon, Nov 14, 2016 at 9:30 PM, Robert Haas wrote: > Couldn't we just change the current memory context before calling > heap_getnext()? And then change back? Right, seems like it will not have any problem.. > > Also, what if we abandoned the idea of pushing qual evaluation all the > way down i

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-14 Thread Robert Haas
On Sun, Nov 13, 2016 at 12:16 AM, Dilip Kumar wrote: > Problem1: As Andres has mentioned, HeapKeyTest uses heap_getattr, > whereas ExecQual use slot_getattr().So we can have worst case > performance problem when very less tuple are getting filter out and we > have table with many columns with qua

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-12 Thread Dilip Kumar
I have done performance analysis for TPCH queries, I saw visible gain in 5 queries (10-25%). Performance Data: Benchmark : TPCH (S.F. 10) shared_buffer : 20GB work_mem : 50MB Machine : POWER Results are median of three run (explain analyze results for both head/patch are attached in T

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-03 Thread Robert Haas
On Tue, Nov 1, 2016 at 8:31 PM, Kouhei Kaigai wrote: > By the way, I'm a bit skeptical whether this enhancement is really beneficial > than works for this enhancement, because we can now easily increase the number > of processor cores to run seq-scan with qualifier, especially, when it has > high

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-02 Thread Dilip Kumar
On Sat, Oct 29, 2016 at 12:17 PM, Dilip Kumar wrote: > What about putting slot reference inside HeapScanDesc ?. I know it > will make ,heap layer use executor structure but just a thought. > > I have quickly hacked this way where we use slot reference in > HeapScanDesc and directly use > slot_get

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-11-01 Thread Kouhei Kaigai
S] Proposal: scan key push down to heap [WIP] > > On Wed, Oct 26, 2016 at 12:01 PM, Andres Freund wrote: > > The gains are quite noticeable in some cases. So if we can make it work > > without noticeable downsides... > > > > What I'm worried about though is that

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-31 Thread Andres Freund
On 2016-10-31 09:28:00 -0400, Robert Haas wrote: > On Fri, Oct 28, 2016 at 2:46 AM, Andres Freund wrote: > > Well, that'll also make the feature not particularly useful :(. My > > suspicion is that the way to suceed here isn't to rely more on testing > > as part of the scan, but create a more gen

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-31 Thread Robert Haas
On Fri, Oct 28, 2016 at 2:46 AM, Andres Freund wrote: > Well, that'll also make the feature not particularly useful :(. My > suspicion is that the way to suceed here isn't to rely more on testing > as part of the scan, but create a more general fastpath for qual > evaluation, which atm is a *LOT*

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-28 Thread Dilip Kumar
On Wed, Oct 26, 2016 at 12:01 PM, Andres Freund wrote: > The gains are quite noticeable in some cases. So if we can make it work > without noticeable downsides... > > What I'm worried about though is that this, afaics, will quite > noticeably *increase* total cost in cases with a noticeable number

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-28 Thread Amit Kapila
On Fri, Oct 28, 2016 at 12:16 PM, Andres Freund wrote: > On 2016-10-28 11:23:22 +0530, Amit Kapila wrote: > >> I think if we decide to form the scan key from a qual only when qual >> refers to fixed length column and that column is before any varlen >> column, the increased cost will be alleviated

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-27 Thread Andres Freund
On 2016-10-28 11:23:22 +0530, Amit Kapila wrote: > On Wed, Oct 26, 2016 at 12:01 PM, Andres Freund wrote: > > What I'm worried about though is that this, afaics, will quite > > noticeably *increase* total cost in cases with a noticeable number of > > columns and a not that selective qual. The reas

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-27 Thread Amit Kapila
On Wed, Oct 26, 2016 at 12:01 PM, Andres Freund wrote: > On 2016-10-25 13:18:47 -0400, Tom Lane wrote: >> (Frankly, I'm pretty skeptical of this entire patch being worth the >> trouble...) > > The gains are quite noticeable in some cases. So if we can make it work > without noticeable downsides...

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-25 Thread Andres Freund
On 2016-10-25 13:18:47 -0400, Tom Lane wrote: > (Frankly, I'm pretty skeptical of this entire patch being worth the > trouble...) The gains are quite noticeable in some cases. So if we can make it work without noticeable downsides... What I'm worried about though is that this, afaics, will quite

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-25 Thread Dilip Kumar
On Tue, Oct 25, 2016 at 10:35 PM, Alvaro Herrera wrote: > We don't promise order of execution (which is why we can afford to sort > on cost), but I think it makes sense to keep a rough ordering based on > cost, and not let this push-down affect those ordering decisions too > much. Ok, Thanks for

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-25 Thread Tom Lane
Alvaro Herrera writes: > BTW, should we cost push-down-able quals differently, say discount some > fraction of the cost, to reflect the fact that they are cheaper to run? > However, since the decision of which ones to push down depends on the > cost, and the cost would depend on which ones we push

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-25 Thread Alvaro Herrera
Dilip Kumar wrote: > #2. Currently quals are ordered based on cost (refer > order_qual_clauses), But once we pushdown some of the quals, then > those quals will always be executed first. Can this create problem ? We don't promise order of execution (which is why we can afford to sort on cost), bu

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-25 Thread Dilip Kumar
On Fri, Oct 14, 2016 at 11:54 AM, Andres Freund wrote: > I don't think it's a good idea to do this under the content lock in any > case. But luckily I don't think we have to do so at all. > > Due to pagemode - which is used by pretty much everything iterating over > heaps, and definitely seqscans

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-13 Thread Andres Freund
Hi, On 2016-10-11 17:27:56 +0530, Dilip Kumar wrote: > I would like to propose a patch for pushing down the scan key to heap. I think that makes sense. Both because scankey eval is faster than generic expression eval, and because it saves a lot of function calls in heavily filtered cases. > Howe

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-13 Thread Tom Lane
Dilip Kumar writes: > On Thu, Oct 13, 2016 at 6:05 AM, Robert Haas wrote: >> I seriously doubt that this should EVER be supported for anything >> other than "var op const", and even then only for very simple >> operators. > Yes, with existing key push down infrastructure only "var op const", > b

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-12 Thread Dilip Kumar
On Thu, Oct 13, 2016 at 6:05 AM, Robert Haas wrote: > I seriously doubt that this should EVER be supported for anything > other than "var op const", and even then only for very simple > operators. Yes, with existing key push down infrastructure only "var op const", but If we extend that I think we

Re: [HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-12 Thread Robert Haas
On Tue, Oct 11, 2016 at 4:57 AM, Dilip Kumar wrote: > This patch will extract the expression from qual and prepare the scan > keys. Currently in POC version I have only supported "var OP const" > type of qual, because these type of quals can be pushed down using > existing framework. > > Purpose

[HACKERS] Proposal: scan key push down to heap [WIP]

2016-10-11 Thread Dilip Kumar
Hi Hackers, I would like to propose a patch for pushing down the scan key to heap. Currently only in case of system table scan keys are pushed down. I have implemented the POC patch to do the same for normal table scan. This patch will extract the expression from qual and prepare the scan keys.