Horton

Re: Table AM Interface Enhancements

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Andres Freund <andres@anarazel.de>, Pavel Borisov <pashkin.elfe@gmail.com>, Japin Li <japinli@hotmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Michael Paquier <michael@paquier.xyz>, Heikki Linnakangas <hlinnaka@iki.fi>, Thomas Munro <thomas.munro@gmail.com>, Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: 2024-04-11T20:26:52Z
Lists: pgsql-hackers 
On Thu, Apr 11, 2024 at 1:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> I understand that I'm the bad guy of this release, not sure if my
> opinion counts.
>
> But what is going on here?  I hope this work is targeting pg18.
> Otherwise, do I get this right that this post feature-freeze works on
> designing a new API?  Yes, 27bc1772fc masked the problem.  But it was
> committed on Mar 30.  So that couldn't justify why the proper API
> wasn't designed in time.  Are we judging different commits with the
> same criteria?

I mean, Andres already said that the cleanup was needed possibly in
17, and possibly in 18.

As far as fairness is concerned, you'll get no argument from me if you
say the streaming read stuff was all committed far later than it
should have been. I said that in the very first email I wrote on the
"post-feature freeze cleanup" thread. But if you're going to argue
that there's no opportunity for anyone to adjust patches that were
sideswiped by the reverts of your patches, and that if any such
adjustments seem advisable we should just revert the sideswiped
patches entirely, I don't agree with that, and I don't see why anyone
would agree with that. I think it's fine to have the discussion, and
if the result of that discussion is that somebody says "hey, we want
to do X in 17 for reason Y," then we can discuss that proposal on its
merits, taking into account the answers to questions like "why wasn't
this done before the freeze?" and "is that adjustment more or less
risky than just reverting?" and "how about we just leave it alone for
now and deal with it next release?".

> IMHO, 041b96802e should be just reverted.

IMHO, it's too early to decide that, because we don't know what change
concretely is going to be proposed, and there has been no discussion
of why that change, whatever it is, belongs in this release or next
release.

I understand that you're probably not feeling great about being asked
to revert a bunch of stuff here, and I do think it is a fair point to
make that we need to be even-handed and not overreact. Just because
you had some patches that had some problems doesn't mean that
everything that got touched by the reverts can or should be whacked
around a whole bunch more post-freeze, especially since that stuff was
*also* committed very late, in haste, way closer to feature freeze
than it should have been. At the same time, it's also important to
keep in mind that our goal here is not to punish people for being bad,
or to reward them for being good, or really to make any moral
judgements at all, but to produce a quality release. I'm sure that,
where possible, you'd prefer to fix bugs in a patch you committed
rather than revert the whole thing as soon as anyone finds any
problem. I would also prefer that, both for your patches, and for
mine. And everyone else deserves that same consideration.

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

Commits

  1. Remove extra comment at TableAmRoutine.scan_analyze_next_block

  2. revert: Generalize relation analyze in table AM interface

  3. Revert: Allow table AM to store complex data structures in rd_amcache

  4. Revert: Allow table AM tuple_insert() method to return the different slot

  5. Revert: Allow locking updated tuples in tuple_update() and tuple_delete()

  6. Revert: Let table AM insertion methods control index insertion

  7. Revert: Custom reloptions for table AM

  8. Provide a way block-level table AMs could re-use acquire_sample_rows()

  9. Custom reloptions for table AM

  10. Use streaming I/O in ANALYZE.

  11. Revert "Custom reloptions for table AM"

  12. Let table AM insertion methods control index insertion

  13. Generalize relation analyze in table AM interface

  14. Improve error message for tts_(virtual|minimal)_is_current_xact_tuple

  15. Add comments on some MinimalTupleSlots methods usage

  16. Add TupleTableSlotOps.is_current_xact_tuple() method

  17. Allow table AM tuple_insert() method to return the different slot

  18. Allow table AM to store complex data structures in rd_amcache