Re: Index-only quals

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Index-only quals
Date: 2009-08-22 15:14:27
Message-ID: 9475.1250954067@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Barring objections, I'm going to apply the indexam API changes part,
> since that simplifies the code in question regardless of the rest of the
> work. I'm pretty happy now with the indexfilter patch as well, but want
> to do some more testing on that before committing. Some more eyeballs
> would be appreciated as well.

The first patch definitely needs another editorial pass, eg you have
complicated the behavior of heap_hot_search_buffer's all_dead parameter
without adjusting its documentation. The patch as-submitted is really
quite hard to review, though. It's hard to convince myself that all the
logic you removed from index_getnext is all accounted for elsewhere.
Could you run through the reasoning on that?

I think the second patch needs a fair amount of work. The fact that
_bt_getindextuple can fail to get the right tuple seems quite bogus;
what I think it means is that you've attached the functionality in the
wrong place. nbtree certainly had its hands on the right tuple at some
point, and what you should have done was saved the tuple aside at that
point. I feel it's important that an indexam either be able to give
back tuples or not; this "maybe I can" semantics will prevent us from
doing many interesting things with the capability. (More about that
in an upcoming message.)

ExecStoreIndexTuple seems rather confused as well, as it's applying what
apparently is a heap tupdesc to an index tuple. That might accidentally
fail to fail at the moment, but I think it would be better to keep the
two concepts well separated. Moreover attr-by-attr extraction is not
what you want, for efficiency reasons. Use index_deform_tuple(), now
that that's there. (The internal implementation of that is no better,
but now that there is actually a performance reason to improve it, we
could do that.)

I concur with the objection to "regurgitate" terminology, it seems a
bit yucky.

I haven't had time to go through the planner part in any detail...

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2009-08-22 15:17:00 Re: DELETE syntax on JOINS
Previous Message Tom Lane 2009-08-22 14:03:22 Re: [PATCH] plpythonu datatype conversion improvements