On Fri, Oct 2, 2015 at 8:14 PM, Alexander Korotkov <
a.korot...@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

Reply via email to