On Mon, Nov 28, 2016 at 8:25 PM, Robert Haas <robertmh...@gmail.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.
> - get_scankey_from_qual()'s prohibition on variable-width columns is
> presumably no longer necessary with this approach, right?

> - Anything tested in SeqNext() will also need to be retested in
> SeqRecheck(); otherwise, the test will be erroneously skipped during
> EPQ rechecks.

Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to