Hello. I had a look on the patch set. I cannot see the thread structure due to the depth and cannot get the picture on the all patches, but I have some comments. I apologize in advance for possible duplicate with upthread.
0001-Reduce-the... This doesn't apply master. > TupleTableSlot * > ExecStoreHeapTuple(HeapTuple tuple, > TupleTableSlot *slot, > + Oid relid, > bool shouldFree) The comment for ExecStoreHeapTuple is missing the description on "relid" parameter. > - if (HeapTupleSatisfiesVisibility(tuple, &SnapshotDirty, > hscan->rs_cbuf)) > + if (HeapTupleSatisfiesVisibility(tuple, > RelationGetRelid(hscan->rs_scan.rs_rd), > + &SnapshotDirty, hscan->rs_cbuf)) The second parameter seems to be always RelationGetRelid(...). Actually only relid is required but isn't it better to take Relation instead of Oid as the second parameter? 0005-Reorganize-am-as... > + catalog. The contents of an table are entirely under the control of its "of an table" => "of a table" 0006-Doc-update-of-Create-access.. > + be <type>index_am_handler</type> and for <literal>TABLE</literal> > + access methods, it must be <type>table_am_handler</type>. > + The C-level API that the handler function must implement varies > + depending on the type of access method. The index access method API > + is described in <xref linkend="index-access-methods"/> and the table > access method > + API is described in <xref linkend="table-access-methods"/>. If table is the primary object, talbe-am should precede index-am? 0007-Doc-update-of-create-materi.. > + This clause specifies optional access method for the new materialize view; "materialize view" => "materialized view"? > + If this option is not specified, then the default table access method I'm not sure the 'then' is needed. > + is chosen for the new materialized view. see <xref > linkend="guc-default-table-access-method"/> "see" => "See" 0008-Doc-update-of-CREATE_TABLE.. > +[ USING <replaceable class="parameter">method</replaceable> ] *I* prefer access_method to just method. > + If this option is not specified, then the default table access method Same to 0007. "I'm not sure the 'then' is needed.". > + is chosen for the new table. see <xref > linkend="guc-default-table-access-method"/> Same to 0007. " "see" => "See" " " 0009-Doc-of-CREATE-TABLE-AS Same as 0008. 0010-Table-access-method-API- > + Any new <literal>TABLE ACCSESS METHOD</literal> developers can refer the > exisitng <literal>HEAP</literal> > + There are differnt type of API's that are defined and those details are > below. "differnt" => "different" > + by the AM, in case if it support parallel scan. "support" => "supports" > + This API to return the total size that is required for the AM to perform Total size of what? Shared memory chunk? Or parallel scan descriptor? > + the parallel table scan. The minimum size that is required is > + <structname>ParallelBlockTableScanDescData</structname>. I don't get what the "The minimum size" tells. Just reading this I would always return the minimum size... > + This API to perform the initialization of the > <literal>parallel_scan</literal> > + that is required for the parallel scan to be performed by the AM and > also return > + the total size that is required for the AM to perform the parallel > table scan. (Note: I'm not good at English..) Similar to the above. I cannot read what the "size" is for. In the code it is used as: > Size snapshot_off = rel->rd_tableam->parallelscan_initialize(rel, pscan); (The varialbe name should be snapshot_offset) It is the offset from the beginning of parallel scan descriptor but it should be described in other representation, which I'm not sure of.. Something like this? > This API to initialize a parallel scan by the AM and also > return the consumed size so far of parallel scan descriptor. (Sorry for not finishing. Time's up today.) regards. -- Kyotaro Horiguchi NTT Open Source Software Center