Re: WIP: Rework access method interface

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: WIP: Rework access method interface
Date: 2015-11-12 11:49:43
Message-ID: CAPpHfdu1ww2wYnugi2RO9zwKZuQMYX28DzZYGDzLbgh+7BeCbQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 3, 2015 at 3:16 PM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> On Tue, Nov 3, 2015 at 2:36 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
>> > Tom Lane wrote:
>> >> I'm kind of inclined to just let the verifiers read the catalogs for
>> >> themselves. AFAICS, a loop around the results of SearchSysCacheList
>> >> is not going to be significantly more code than what this patch does,
>> >> and it avoids presuming that we know what the verifiers will wish to
>> >> look at.
>>
>> > Hmm, so this amounts to saying the verifier can only run after the
>> > catalog rows are written. Is that okay?
>>
>> Why not? Surely we are not interested in performance-optimizing for
>> the case of a detectably incorrect CREATE OP CLASS command.
>>
>> I don't actually care that much about running the verifiers during
>> CREATE/etc OP CLASS at all. It's at least as important to be able
>> to run them across the built-in opclasses, considering all the chances
>> for human error in constructing those things. Even in the ALTER OP CLASS
>> case, the verifier would likely need to look at existing catalog rows as
>> well as the new ones. So basically, I see zero value in exposing CREATE/
>> ALTER OP CLASS's internal working representation to the verifiers.
>>
>
> I'm OK with validating opclass directly by system catalog, i.e. looping
> over SearchSysCacheList results. Teodor was telling me something similar
> personally.
> I'll also try to rearrange header according to the comments upthread.
>

The next revision of patch is attached.
The changes are following:

1. Definitions of Selectivity, Cost and declarations
of IndexAmRoutine, PlannerInfo, IndexPath, IndexInfo are moved into
separate header reldecls.h. That allows to get rid of #include
"nodes/relation.h" in amapi.h.
2. amvalidate method now gets opclass oid as parameter. Internally,
amvalidate implementations do catalog lookups. opfam_internal.h isn't
included from inappropriate places anymore.
3. I removed amvalidate calls from opclasscmds.c. Validating
user-defined opclasses is behavior change which can have issues with
backward compatibility. I think if we want to introduce this, it should be
considered separately of API rework.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
aminterface-10.patch application/octet-stream 267.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2015-11-12 12:16:16 Re: Proposal: "Causal reads" mode for load balancing reads without stale data
Previous Message Etsuro Fujita 2015-11-12 10:33:33 Re: Foreign join pushdown vs EvalPlanQual