Re: [DOCS] HOT - correct claim about indexes not referencing old line pointers

On Thu, Mar 14, 2024 at 9:07 PM James Coleman <jtc331(at)gmail(dot)com> wrote:
> Obviously I have reasons for the other changes I made: for example,
> "no longer visible" improves the correctness, since being an old
> version isn't sufficient. I removed the "In summary" sentence because
> it simply doesn't follow from the prose before it. That sentence
> simply restates information already appearing earlier in almost as
> simple a form, so it's redundant. But more importantly it's just not
> actually a summary of the text before it, so removing it improves the
> documentation.
>
> I can explain my reasoning further if desired, but I fear it would
> simply frustrate you further, so I'll stop here.
>
> If the goal here is the most minimal patch possible, then please
> commit what you proposed. I am interested in improving the document
> further, but I don't know how to do that easily if the requirement is
> effectively "must only change one specific detail at a time". So, that
> leaves me feeling a bit frustrated also.

I don't think the goal is to have the most minimal patch possible, but
at the same time, I'm responsible for what I commit. If I commit a
patch that changes something and somebody, especially another
committer, writes an email and says "hey, why the heck did you change
this, you dummy?" then I need to be able to answer that question, and
saying "uh, well, James Coleman had it in his patch and he didn't
explain why it was there and I didn't know either but I just kind of
committed it anyway" is not where I want to be. Almost without
exception, it's not the patch author who gets asked why they included
a certain thing in the patch; it's the committer who gets asked why it
got committed that way -- and at least as I see it, if there's not a
good answer to that question, it's the committer who gets judged, not
the patch author. In some cases, such questions and judgements arrive
without warning YEARS after the commit.

So, to protect myself against questions from other hackers that end,
at least implicitly, in "you dummy," I try to make sure there's an
adequate paper trail for everything I commit. Between the commit
message, the comments, and the email discussion, it needs to be
crystal clear why something was thought to be a good idea at the time.
Hopefully, it will be clear enough that I never even get a question,
because the reader will be able to understand ON THEIR OWN why
something was done and either go "oh, that make sense" or "well, I get
why they did that, but I disagree because $REASON" ... but if that
doesn't quite work out, then I hope that at the very least the paper
trail will be good enough that I can reconstruct the reasoning if
needed. For a recent example of a case where I clearly failed do a
good enough job to keep someone from asking a "you dummy" question,
see the http://postgr.es/m/6030bdec-0de1-4da2-b0b3-335007eae97f@iki.fi
and in particular the paragraph that starts with "However".

Therefore, if I realize when reading the patch that it contains
odd-looking changes which I can't relate to the stated goal, it's
getting bounced, pretty much 100% of the time. If it's a small patch
and I really care about it and it's a new submitter who doesn't know
any better, I might choose to remove those changes myself and commit
the rest; and if I like those changes for other reasons, I might
choose to commit them separately. In any other situation, I'm just
going to tell the submitter that they need to take out the unrelated
changes and move on to the next email thread.

In this particular situation, I see that this whole discussion is
slightly moot by virtue of 7a9328e8e40534fb4de41b4ac152e3c6989d84e7
(Jeff Davis, 2024-03-05) which removed the "In summary, heap-only
tuple updates can only be created if columns used by indexes are not
updated" sentence that you also removed. But there is an important
difference between that patch and yours, at least IMHO. You argue that
the sentence in question didn't flow from what precedes it, but I
don't agree with that as a rationale; I think that sentence flows
perfectly well from what precedes it and is a generally adequate
summary of the point of the section. That we disagree on the merits of
that sentence is fine; there's no hope that we're all going to agree
on everything. What makes me frustrated is that you didn't provide
that rationale, which greatly complicates the whole discussion,
because now I have to spend time getting you to either take it out or
provide your justification before we can even reach the point of
arguing about the substance.

At least in my judgment, the patch Jeff committed doesn't have that
problem, because the rationale that he gives in the commit message
(partly by reference to another commit) is that heap-only tuples now
can, in circumstances, be created even if columns used by indexes ARE
updated. Based on that justification, it seems really clear to me that
it was right to do SOMETHING to the sentence in question. Whether
removing the sentence was better than some other alternative is
debatable, and I might well have done something different, but between
reading the commit message and reading the diff, it's 100%
understandable to me what his reasoning was for every single byte that
is in that diff. So if I love what he committed, cool! And if I hate
what he committed, I'm well-placed to argue, because I know what I'm
arguing against.

Also, if I do feel like arguing, the fact that I know his reasoning
will also make it a heck of a lot easier to come up with a proposal
that meets my goals without undermining his. I can see, for example,
that I can't simply propose to put that sentence back the way that it
was; based on his reason for removing it, he obviously isn't going to
like that. But if, say, my goal were to have some kind of a summary
sentence there because I just thought that was really helpful, I could
write a patch to insert a new summary sentence there and then say
"hey, Jeff, you took this summary sentence out because it's not
accurate, but here's a new one which I think is accurate and I'd like
to add it back." Maybe he'd like that, and maybe he wouldn't, but it
would have a shot, because it would take his reasoning into account,
which I can do, because I know what it was. I want people who read the
patches that I commit to have the same opportunities.

--
Robert Haas
EDB: http://www.enterprisedb.com