Re: Table AM Interface Enhancements

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: Re: Table AM Interface Enhancements
Date: 2024-04-10 13:19:15
Message-ID: CA+TgmobZUnJQaaGkuoeo22Sydf9=mX864W11yZKd6sv-53-aEQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 10, 2024 at 8:20 AM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> I agree with the facts. But I have a different interpretation on
> this. The patch was committed as 11470f544e on March 23, 2023, then
> reverted on April 3. I've proposed the revised version, but Andres
> complained that this is the new API design days before FF.

Well, his first complaint that your committed patch was full of bugs:

https://www.postgresql.org/message-id/20230323003003.plgaxjqahjgkuxrk%40awork3.anarazel.de

When you commit a patch and another committer writes a post-commit
review saying that your patch has so many serious problems that he
gave up on reviewing before enumerating all of them, that's a really
bad sign. That should be a strong signal to you to step back and take
a close look at whether you really understand the area of the code
that you're touching well enough to be doing whatever it is that
you're doing. If I got a review like that, I would have reverted the
patch instantly, given up for the release cycle, possibly given up on
the patch permanently, and most definitely not tried again to commit
unless I was absolutely certain that I'd learned a lot in the meantime
*and* had the agreement of the committer who wrote that review (or
maybe some other committer who was acknowledged as an expert in that
area of the code).

What you did instead is try to do a bunch of post-commit fixup in a
desperate rush right before feature freeze, to which Andres
understandably objected. But that was your second mistake, not your
first one.

> Then the
> patch with this design was published in the thread for the year with
> periodical rebases. So, I think I expressed my intention with that
> design before 2023 FF, nobody prevented me from expressing objections
> or other feedback during the year. Then I realized that 2024 FF is
> approaching and decided to give this another try for pg18.

This doesn't seem to match the facts as I understand them. It appears
to me that there was no activity on the thread from April until
November. The message in November was not written by you. Your first
post to the thread after April of 2023 was on March 19, 2024. Five
days later you said you wanted to commit. That doesn't look to me like
you worked diligently on the patch set throughout the year and other
people had reasonable notice that you planned to get the work done
this cycle. It looks like you ignored the patch for 11 months and then
committed it without any real further feedback from anyone. True,
Pavel did post and say that he thought the patches were in good shape.
But you could hardly take that as evidence that Andres was now content
that the problems he'd raised earlier had been fixed, because (a)
Pavel had also been involved beforehand and had not raised the
concerns that Andres later raised and (b) Pavel wrote nothing in his
email specifically about why he thought your changes or his had
resolved those concerns. I certainly agree that Andres doesn't always
give as much review feedback as I'd like to have from him in, and it's
also true that he doesn't always give that feedback as quickly as I'd
like to have it ... but you know what?

It's not Andres's job to make sure my patches are not broken. It's my
job. That applies to the patches I write, and the patches written by
other people that I commit. If I commit something and it turns out
that it is broken, that's my bad. If I commit something and it turns
out that it does not have consensus, that is also my bad. It is not
the fault of the other people for not helping me get my patches to a
state where they are up to project standard. It is my fault, and my
fault alone, for committing something that was not ready. Now that
does not mean that it isn't frustrating when I can't get the help I
need. It is extremely frustrating. But the solution is not to commit
anyway and then blame the other people for not providing feedback.

I mean, committing without explicit agreement from someone else is OK
if you're pretty sure that you've got everything sorted out correctly.
But I don't think that the paper trail here supports the narrative
that you worked on this diligently throughout the year and had every
reason to believe it would be acceptable to the community. If I'd
looked at this thread, I would have concluded that you'd abandoned the
project. I would have expected that, when you picked it up again,
there would be a series of emails over a period of time carefully
working through the various issues that had been raised, inviting
specific commentary on specific discussion points, and generally
refining the work, and then maybe a suggestion of a commit at the end.
I would not have expected an email or two basically saying "well,
seems like it's all fixed now," followed by a commit.

> Do you propose a ban from March 1 to the end of any year? I think the
> first doesn't make sense, because it leaves only 2 months a year for
> the work. That would create a potential rush during these 2 month and
> could serve exactly opposite to the intention. So, I guess this means
> a ban from March 1 to the FF of any year. The situation now is quite
> unpleasant for me. So I'm far from repeating this next year.
> However, if there should be a formal ban, it should be specified.
> Does it relate to the patches I've pushed, all patches in this thread,
> all similar patches, all table AM patches, or other API patches?

I meant from March 1 to feature freeze, but maybe I should have
proposed that you shouldn't ever commit these patches. The more I look
at this, the less happy I am with how you did it.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tender Wang 2024-04-10 13:32:32 Re: Revise some error messages in split partition code
Previous Message Jacob Champion 2024-04-10 13:10:56 Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?