From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Proposal: scan key push down to heap [WIP] |
Date: | 2016-11-29 04:17:29 |
Message-ID: | CAFiTN-vcdkamSW1i5wQeXGLi+uzti3tVXpzyKPWG4x8PBkb0-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 28, 2016 at 8:25 PM, Robert Haas <robertmhaas(at)gmail(dot)com> 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 is still quite good.
I agree.
However, there's a
> fair amount of this patch that doesn't look right:
>
> - The changes to heapam.c shouldn't be needed any more. Ditto
> valid.h, relscan.h, catcache.c and maybe some other stuff.
Actually we want to call slot_getattr instead heap_getattr, because of
problem mentioned by Andres upthread and we also saw in test results.
Should we make a copy of HeapKeyTest lets say ExecKeyTest and keep it
under executor ?
ExecKeyTest will be same as HeapKeyTest but it will call slot_getattr
instead of heap_getattr.
>
> - get_scankey_from_qual() should be done at plan time, not execution
> time. Just as index scans already divide up quals between "Index
> Cond" and "Filter" (see ExplainNode), I think Seq Scans are now going
> to need to something similar. Obviously, "Index Cond" isn't an
> appropriate name for things that we test via HeapKeyTest, but maybe
> "Heap Cond" would be suitable. That's going to be a fair amount of
> refactoring, since the "typedef Scan SeqScan" in plannodes.h is going
> to need to be replaced by an actual new structure definition.
>
Okay.
> - get_scankey_from_qual()'s prohibition on variable-width columns is
> presumably no longer necessary with this approach, right?
Correct.
>
> - Anything tested in SeqNext() will also need to be retested in
> SeqRecheck(); otherwise, the test will be erroneously skipped during
> EPQ rechecks.
Okay..
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-11-29 04:22:33 | Re: Time to up bgwriter_lru_maxpages? |
Previous Message | Nico Williams | 2016-11-29 03:25:29 | Re: Tackling JsonPath support |