On Fri, Jun 28, 2019 at 1:47 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Mon, Jun 24, 2019 at 11:26 PM Ashwin Agrawal <aagra...@pivotal.io>
> wrote:
> >
> > On Mon, Jun 3, 2019 at 5:24 PM Ashwin Agrawal <aagra...@pivotal.io>
> wrote:
> >>
> >> There were few more minor typos I had collected for table am, passing
> them along here.
> >>
> >> Some of the required callback functions are missing Assert checking
> (minor thing), adding them in separate patch.
> >
> >
> > Curious to know if need to register such small typo fixing and assertion
> adding patchs to commit-fest as well ?
> >
>
> Normally, such things are handled out of CF, but to avoid forgetting,
> we can register it. However, this particular item suits more to 'Open
> Items'[1]. You can remove the objectionable part of the comment,
> other things in your patch look good to me. If nobody else picks it
> up, I will take care of it.
>
Thank you, I thought Committer would remove the objectionable part of
comment change and commit the patch if seems fine. I don't mind changing,
just feel adds extra back and forth cycle.
Please find attached v2 of patch 1 without objectionable comment change. v1
of patch 2 attaching here just for convenience, no modifications made to it.
From 5c75807a56101a07685ed1f435eabcc43cd22fb7 Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal <aagra...@pivotal.io>
Date: Fri, 24 May 2019 16:30:38 -0700
Subject: [PATCH v2 1/2] Fix typos in few tableam comments.
---
src/include/access/tableam.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index fd07773b74f..1e45908c78a 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -434,8 +434,8 @@ typedef struct TableAmRoutine
*
* Note that only the subset of the relcache filled by
* RelationBuildLocalRelation() can be relied upon and that the relation's
- * catalog entries either will either not yet exist (new relation), or
- * will still reference the old relfilenode.
+ * catalog entries will either not yet exist (new relation), or will still
+ * reference the old relfilenode.
*
* As output *freezeXid, *minmulti must be set to the values appropriate
* for pg_class.{relfrozenxid, relminmxid}. For AMs that don't need those
@@ -591,7 +591,7 @@ typedef struct TableAmRoutine
* See table_relation_estimate_size().
*
* While block oriented, it shouldn't be too hard for an AM that doesn't
- * doesn't internally use blocks to convert into a usable representation.
+ * internally use blocks to convert into a usable representation.
*
* This differs from the relation_size callback by returning size
* estimates (both relation size and tuple count) for planning purposes,
@@ -967,7 +967,7 @@ table_index_fetch_end(struct IndexFetchTableData *scan)
*
* *all_dead, if all_dead is not NULL, will be set to true by
* table_index_fetch_tuple() iff it is guaranteed that no backend needs to see
- * that tuple. Index AMs can use that do avoid returning that tid in future
+ * that tuple. Index AMs can use that to avoid returning that tid in future
* searches.
*
* The difference between this function and table_fetch_row_version is that
@@ -1014,8 +1014,8 @@ extern bool table_index_fetch_tuple_check(Relation rel,
* true, false otherwise.
*
* See table_index_fetch_tuple's comment about what the difference between
- * these functions is. This function is the correct to use outside of
- * index entry->table tuple lookups.
+ * these functions is. This function is correct to use outside of index
+ * entry->table tuple lookups.
*/
static inline bool
table_tuple_fetch_row_version(Relation rel,
--
2.19.1
From 4ed947590f2f61182356a7fa4bc429c679e7619f Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal <aagra...@pivotal.io>
Date: Mon, 3 Jun 2019 17:07:05 -0700
Subject: [PATCH v2 2/2] Add assertions for required table am callbacks.
---
src/backend/access/table/tableamapi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c
index b2f58768107..98b8ea559d8 100644
--- a/src/backend/access/table/tableamapi.c
+++ b/src/backend/access/table/tableamapi.c
@@ -51,6 +51,7 @@ GetTableAmRoutine(Oid amhandler)
Assert(routine->scan_begin != NULL);
Assert(routine->scan_end != NULL);
Assert(routine->scan_rescan != NULL);
+ Assert(routine->scan_getnextslot != NULL);
Assert(routine->parallelscan_estimate != NULL);
Assert(routine->parallelscan_initialize != NULL);
@@ -62,8 +63,12 @@ GetTableAmRoutine(Oid amhandler)
Assert(routine->index_fetch_tuple != NULL);
Assert(routine->tuple_fetch_row_version != NULL);
+ Assert(routine->tuple_tid_valid != NULL);
+ Assert(routine->tuple_get_latest_tid != NULL);
Assert(routine->tuple_satisfies_snapshot != NULL);
+ Assert(routine->compute_xid_horizon_for_tuples != NULL);
+
Assert(routine->tuple_insert != NULL);
/*
@@ -89,6 +94,7 @@ GetTableAmRoutine(Oid amhandler)
Assert(routine->index_validate_scan != NULL);
Assert(routine->relation_size != NULL);
+ Assert(routine->relation_needs_toast_table != NULL);
Assert(routine->relation_estimate_size != NULL);
--
2.19.1
