outdated comment in table_tuple_update definition
Hello
I noticed that the comment for the table_tuple_update function (src/include/access/tableam.h) describes the update_indexes parameter as boolean:
* update_indexes - in success cases this is set to true if new index entries
* are required for this tuple
Although this parameter is an enum of three values.
I found that the parameter type was changed by commit 19d8e23 (Ignore BRIN indexes when checking for HOT updates), but the comment was not updated. Looks like oversight.
* update_indexes - in successful cases, used to determine which index types
* require new index entries for this tuple
Something like this?
regards, Sergei
On 6 Feb 2025, at 20:00, Sergei Kornilov <sk@zsrv.org> wrote:
I found that the parameter type was changed by commit 19d8e23 (Ignore BRIN indexes when checking for HOT updates), but the comment was not updated. Looks like oversight.
I agree with your analysis, unless objected to I'll apply the attached in
shortly to update the documentation.
--
Daniel Gustafsson
Attachments:
0001-Fix-parameter-description-in-comment.patchapplication/octet-stream; name=0001-Fix-parameter-description-in-comment.patch; x-unix-mode=0644Download
From 954ae3187beb17496be129895a402b137cc7c8f8 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Thu, 6 Feb 2025 22:32:55 +0100
Subject: [PATCH] Fix parameter description in comment
Commit 19d8e23 changed the datatype of the update_index parameter but
accidentally missed updating the documentation comment.
Reported-by: Sergei Kornilov <sk@zsrv.org>
Discussion: https://postgr.es/m/1097041738868417@zxq47dct3nfd6lzq.sas.yp-c.yandex.net
---
src/include/access/tableam.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 131c050c15f..a856543bfd8 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -1520,8 +1520,8 @@ table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid,
* Output parameters:
* tmfd - filled in failure cases (see below)
* lockmode - filled with lock mode acquired on tuple
- * update_indexes - in success cases this is set to true if new index entries
- * are required for this tuple
+ * update_indexes - in successful cases this indicates the index types
+ * which require new index entries for this tuple
*
* Normal, successful return value is TM_Ok, which means we did actually
* update it. Failure return codes are TM_SelfModified, TM_Updated, and
--
2.39.3 (Apple Git-146)
On Fri, Feb 7, 2025 at 3:08 AM Daniel Gustafsson <daniel@yesql.se> wrote:
On 6 Feb 2025, at 20:00, Sergei Kornilov <sk@zsrv.org> wrote:
I found that the parameter type was changed by commit 19d8e23 (Ignore BRIN indexes when checking for HOT updates), but the comment was not updated. Looks like oversight.
I agree with your analysis, unless objected to I'll apply the attached in
shortly to update the documentation.
+ * update_indexes - in successful cases this indicates the index types
+ * which require new index entries for this tupleAFAIK, summarising indexes may not necessarily always need a new
entry. So the following sentence looks more accurate
in successful cases, this indicates the types of indexes (summarising
vs non-summarising) which need an update based on this tuple. The text
in the parenthese would clarify "index types", but if it's too
verbose, it may be omitted.
--
Best Wishes,
Ashutosh Bapat