Re: WIP: Rework access method interface - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: WIP: Rework access method interface
Date
Msg-id CAA4eK1Jfa5-yhqPjPAkYZb6s3LgRGYmy5f24cQZRRVL42o6Qdw@mail.gmail.com
Whole thread Raw
In response to Re: WIP: Rework access method interface  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: WIP: Rework access method interface  (Petr Jelinek <petr@2ndquadrant.com>)
List pgsql-hackers
On Fri, Oct 2, 2015 at 8:14 PM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
>
>
> I agree about staying with one SQL-visible function.
>
> Other changes:
>  * Documentation reflects interface changes.
>  * IndexAmRoutine moved from CacheMemoryContext to indexcxt.
>  * Various minor formatting improvements.
>  * Error messages are corrected.
>

Few assorted comments:

1.
+  * Get IndexAmRoutine structure from access method oid.
+  */
+ IndexAmRoutine *
+ GetIndexAmRoutine(Oid 
amoid)
+ {
+ IndexAmRoutine *result;
+ HeapTuple tuple;
+ regproc
amhandler;
+ tuple = SearchSysCache1(AMOID, ObjectIdGetDatum(amoid));
+ if (!HeapTupleIsValid
(tuple))
+ elog(ERROR, "cache lookup failed for access method %u",
+  
amoid);
+ amhandler = ((Form_pg_am)GETSTRUCT(tuple))->amhandler;
+ if (!RegProcedureIsValid
(amhandler))
+ elog(ERROR, "invalid %u regproc", amhandler);


I have noticed that currently, the above kind of error is reported slightly
differently as in below code:
if (!RegProcedureIsValid(procOid)) \
elog(ERROR, "invalid %s regproc", CppAsString
(pname)); \

If you feel it is better to do the way as it is in current code, then you
can change accordingly.

2.
<para>
         Access methods that always return entries in the natural ordering
         of their data (such 
as btree) should set
!        <structname>pg_am</>.<structfield>amcanorder</> to true.
         Currently, such 
access methods must use btree-compatible strategy
         numbers for their equality and ordering operators.
  
      </para>
--- 545,551 ----
        <para>
         Access methods that always return entries in the natural 
ordering
         of their data (such as btree) should set
!        <structfield>amcanorder</> to true.
         
Currently, such access methods must use btree-compatible strategy
         numbers for their equality and 
ordering operators.


Isn't it better to use structure while referencing the field of it?

3.
!    Handler function must be written in a compiled language such as C, using
!    the version-1 interface.

Here, it is not completely clear, what do you refer to as version-1 interface.

4.
xindex.sgml
<title>Index Methods and Operator Classes</title>
..
It is possible to add a
   new index method by defining the required interface routines and
   then creating a row in <classname>pg_am</classname> &mdash; but that is
   beyond the scope of this chapter (see <xref linkend="indexam">).
  </para>

I think changing above to indicate something about handler function
could be useful.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: pgbench stats per script & other stuff
Next
From: Andres Freund
Date:
Subject: Re: ON CONFLICT issues around whole row vars,