Re: Pluggable Storage - Andres's take - Mailing list pgsql-hackers

I reviewed new docs for $SUBJECT.

Find attached proposed changes.

There's one XXX item I'm unsure what it's intended to say.

Justin

From a3d290bf67af2a34e44cd6c160daf552b56a13b5 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 4 Apr 2019 00:48:09 -0500
Subject: [PATCH v1] Fine tune documentation for tableam

Added at commit b73c3a11963c8bb783993cfffabb09f558f86e37
---
 doc/src/sgml/catalogs.sgml        |  2 +-
 doc/src/sgml/config.sgml          |  4 ++--
 doc/src/sgml/ref/select_into.sgml |  6 +++---
 doc/src/sgml/storage.sgml         | 17 ++++++++-------
 doc/src/sgml/tableam.sgml         | 44 ++++++++++++++++++++-------------------
 5 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 58c8c96..40ddec4 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -587,7 +587,7 @@
    The catalog <structname>pg_am</structname> stores information about
    relation access methods.  There is one row for each access method supported
    by the system.
-   Currently, only table and indexes have access methods. The requirements for table
+   Currently, only tables and indexes have access methods. The requirements for table
    and index access methods are discussed in detail in <xref linkend="tableam"/> and
    <xref linkend="indexam"/> respectively.
   </para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4a9a1e8..90b478d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7306,8 +7306,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         This parameter specifies the default table access method to use when
         creating tables or materialized views if the <command>CREATE</command>
         command does not explicitly specify an access method, or when
-        <command>SELECT ... INTO</command> is used, which does not allow to
-        specify a table access method. The default is <literal>heap</literal>.
+        <command>SELECT ... INTO</command> is used, which does not allow
+        specification of a table access method. The default is <literal>heap</literal>.
        </para>
       </listitem>
      </varlistentry>
diff --git a/doc/src/sgml/ref/select_into.sgml b/doc/src/sgml/ref/select_into.sgml
index 17bed24..1443d79 100644
--- a/doc/src/sgml/ref/select_into.sgml
+++ b/doc/src/sgml/ref/select_into.sgml
@@ -106,11 +106,11 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
   </para>
 
   <para>
-   In contrast to <command>CREATE TABLE AS</command> <command>SELECT
-   INTO</command> does not allow to specify properties like a table's access
+   In contrast to <command>CREATE TABLE AS</command>, <command>SELECT
+   INTO</command> does not allow specification of properties like a table's access
    method with <xref linkend="sql-createtable-method" /> or the table's
    tablespace with <xref linkend="sql-createtable-tablespace" />. Use <xref
-   linkend="sql-createtableas"/> if necessary.  Therefore the default table
+   linkend="sql-createtableas"/> if necessary.  Therefore, the default table
    access method is chosen for the new table. See <xref
    linkend="guc-default-table-access-method"/> for more information.
   </para>
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 62333e3..5dfca1b 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -189,11 +189,11 @@ there.
 </para>
 
 <para>
- Note that the following sections describe the way the builtin
+ Note that the following sections describe the behavior of the builtin
  <literal>heap</literal> <link linkend="tableam">table access method</link>,
- and the builtin <link linkend="indexam">index access methods</link> work. Due
- to the extensible nature of <productname>PostgreSQL</productname> other types
- of access method might work similar or not.
+ and the builtin <link linkend="indexam">index access methods</link>. Due
+ to the extensible nature of <productname>PostgreSQL</productname>, other
+ access methods might work differently.
 </para>
 
 <para>
@@ -703,11 +703,12 @@ erased (they will be recreated automatically as needed).
 This section provides an overview of the page format used within
 <productname>PostgreSQL</productname> tables and indexes.<footnote>
   <para>
-    Actually, neither table nor index access methods need not use this page
-    format.  All the existing index methods do use this basic format, but the
+    Actually, use of this page format is not required for either table or index
+    access methods.
+    The <literal>heap</literal> table access method always uses this format.
+    All the existing index methods also use the basic format, but the
     data kept on index metapages usually doesn't follow the item layout
-    rules. The <literal>heap</literal> table access method also always uses
-    this format.
+    rules.
   </para>
 </footnote>
 Sequences and <acronym>TOAST</acronym> tables are formatted just like a regular table.
diff --git a/doc/src/sgml/tableam.sgml b/doc/src/sgml/tableam.sgml
index 8d9bfd8..0a89935 100644
--- a/doc/src/sgml/tableam.sgml
+++ b/doc/src/sgml/tableam.sgml
@@ -48,54 +48,56 @@
   callbacks and their behavior is defined in the
   <structname>TableAmRoutine</structname> structure (with comments inside the
   struct defining the requirements for callbacks). Most callbacks have
-  wrapper functions, which are documented for the point of view of a user,
-  rather than an implementor, of the table access method.  For details,
+  wrapper functions, which are documented from the point of view of a user
+  (rather than an implementor) of the table access method.  For details,
   please refer to the <ulink
url="https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/access/tableam.h;hb=HEAD">
   <filename>src/include/access/tableam.h</filename></ulink> file.
  </para>
 
  <para>
-  To implement a access method, an implementor will typically need to
-  implement a AM specific type of tuple table slot (see
+  To implement an access method, an implementor will typically need to
+  implement an AM-specific type of tuple table slot (see
   <ulink url="https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/executor/tuptable.h;hb=HEAD">
-   <filename>src/include/executor/tuptable.h</filename></ulink>) which allows
+   <filename>src/include/executor/tuptable.h</filename></ulink>), which allows
    code outside the access method to hold references to tuples of the AM, and
    to access the columns of the tuple.
  </para>
 
  <para>
-  Currently the the way an AM actually stores data is fairly
-  unconstrained. It is e.g. possible to use postgres' shared buffer cache,
-  but not required. In case shared buffers are used, it likely makes to
-  postgres' standard page layout described in <xref
+  Currently, the way an AM actually stores data is fairly
+  unconstrained. For example, it's possible but not required to use postgres'
+  shared buffer cache.  In case it is used, it likely makes
+XXX something missing here ?
+  to postgres' standard page layout described in <xref
   linkend="storage-page-layout"/>.
  </para>
 
  <para>
   One fairly large constraint of the table access method API is that,
   currently, if the AM wants to support modifications and/or indexes, it is
-  necessary that each tuple has a tuple identifier (<acronym>TID</acronym>)
+  necessary for each tuple to have a tuple identifier (<acronym>TID</acronym>)
   consisting of a block number and an item number (see also <xref
   linkend="storage-page-layout"/>).  It is not strictly necessary that the
-  sub-parts of <acronym>TIDs</acronym> have the same meaning they e.g. have
+  sub-parts of <acronym>TIDs</acronym> have the same meaning as used
   for <literal>heap</literal>, but if bitmap scan support is desired (it is
   optional), the block number needs to provide locality.
  </para>
 
  <para>
-  For crash safety an AM can use postgres' <link
-  linkend="wal"><acronym>WAL</acronym></link>, or a custom approach can be
-  implemented.  If <acronym>WAL</acronym> is chosen, either <link
-  linkend="generic-wal">Generic WAL Records</link> can be used — which
-  implies higher WAL volume but is easy, or a new type of
-  <acronym>WAL</acronym> records can be implemented — but that
-  currently requires modifications of core code (namely modifying
+  For crash safety, an AM can use postgres' <link
+  linkend="wal"><acronym>WAL</acronym></link>, or a custom implementation.
+  If <acronym>WAL</acronym> is chosen, either <link
+  linkend="generic-wal">Generic WAL Records</link> can be used,
+  or a new type of <acronym>WAL</acronym> records can be implemented.
+  Generic WAL Records are easy, but imply higher WAL volume.
+  Implementation of a new type of WAL record
+  currently requires modifications to core code (specifically,
   <filename>src/include/access/rmgrlist.h</filename>).
  </para>
 
  <para>
   To implement transactional support in a manner that allows different table
-  access methods be accessed within a single transaction, it likely is
+  access methods be accessed within a single transaction, it's likely
   necessary to closely integrate with the machinery in
   <filename>src/backend/access/transam/xlog.c</filename>.
  </para>
@@ -103,8 +105,8 @@
  <para>
   Any developer of a new <literal>table access method</literal> can refer to
   the existing <literal>heap</literal> implementation present in
-  <filename>src/backend/heap/heapam_handler.c</filename> for more details of
-  how it is implemented.
+  <filename>src/backend/heap/heapam_handler.c</filename> for details of
+  its implementation.
  </para>
 
 </chapter>
-- 
2.1.4