Re: Table AM Interface Enhancements

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Japin Li <japinli@hotmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-04-08T09:34:09Z
Lists: pgsql-hackers

Attachments

On Mon, Apr 8, 2024 at 10:18 AM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> On Mon, 8 Apr 2024 at 03:25, Alexander Korotkov <aekorotkov@gmail.com> wrote:
>>
>> Hi,
>>
>> On Mon, Apr 8, 2024 at 12:40 AM Andres Freund <andres@anarazel.de> wrote:
>> > On 2024-03-30 23:33:04 +0200, Alexander Korotkov wrote:
>> > > I've pushed 0001, 0002 and 0006.
>> >
>> > I briefly looked at 27bc1772fc81 and I don't think the state post this commit
>> > makes sense. Before this commit another block based AM could implement analyze
>> > without much code duplication. Now a large portion of analyze.c has to be
>> > copied, because they can't stop acquire_sample_rows() from calling
>> > heapam_scan_analyze_next_block().
>> >
>> > I'm quite certain this will break a few out-of-core AMs in a way that can't
>> > easily be fixed.
>>
>> I was under the impression there are not so many out-of-core table
>> AMs, which have non-dummy analysis implementations.  And even if there
>> are some, duplicating acquire_sample_rows() isn't a big deal.
>>
>> But given your feedback, I'd like to propose to keep both options
>> open.  Turn back the block-level API for analyze, but let table-AM
>> implement its own analyze function.  Then existing out-of-core AMs
>> wouldn't need to do anything (or probably just set the new API method
>> to NULL).
>
> I think that providing both new and old interface functions for block-based and non-block based custom am is an excellent compromise.
>
> The patch v1-0001-Turn-back.. is mainly an undo of part of the 27bc1772fc81 that had turned off _analyze_next_tuple..analyze_next_block for external callers. If some extensions are already adapted to the old interface functions, they are free to still use it.

Please, check this.  Instead of keeping two APIs, it generalizes
acquire_sample_rows().  The downside is change of
AcquireSampleRowsFunc signature, which would need some changes in FDWs
too.

------
Regards,
Alexander Korotkov

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