From 60a10f1609820462259eadd83e822e9709d038e6 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tv@fuzzy.cz>
Date: Wed, 10 Jul 2024 15:09:45 +0200
Subject: [PATCH v28-review 02/13] review

---
 contrib/amcheck/amcheck.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/contrib/amcheck/amcheck.c b/contrib/amcheck/amcheck.c
index bf3427e375d..41cdb5e35a4 100644
--- a/contrib/amcheck/amcheck.c
+++ b/contrib/amcheck/amcheck.c
@@ -8,6 +8,8 @@
  * IDENTIFICATION
  *	  contrib/amcheck/amcheck.c
  *
+ *
+ * XXX I'd probably call this verify_common.c or something like that.
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
@@ -29,6 +31,8 @@ static bool amcheck_index_mainfork_expected(Relation rel);
  * where there is simply nothing to verify.
  *
  * NB: Caller should call index_checkable() before calling here.
+ *
+ * XXX Wouldn't it be more natural to have this check in index_checkable?
  */
 static bool
 amcheck_index_mainfork_expected(Relation rel)
@@ -45,6 +49,9 @@ amcheck_index_mainfork_expected(Relation rel)
 	return false;
 }
 
+/*
+ * XXX missing comment, and it's the longest/most important function in the file
+ */
 void
 amcheck_lock_relation_and_check(Oid indrelid,
 								Oid am_id,
@@ -153,7 +160,7 @@ index_checkable(Relation rel, Oid am_id)
 		rel->rd_rel->relam != am_id)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("only B-Tree indexes are supported as targets for verification"), //TODO name AM
+				 errmsg("only B-Tree indexes are supported as targets for verification"), //FIXME name AM, shouldn't be hhard to lookup in AMOID syscache
 				 errdetail("Relation \"%s\" is not a B-Tree index.",
 						   RelationGetRelationName(rel))));
 
-- 
2.45.2

