Horton

Re: Table AM Interface Enhancements

Nikita Malakhov <hukutoc@gmail.com>

View thread View on postgr.es
From: Nikita Malakhov <hukutoc@gmail.com>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-11-29T14:27:20Z
Lists: pgsql-hackers 
Hi,

Pavel, as far as I understand Alexander's idea assertion and especially
ereport
here does not make any sense - this method is not considered to report
error, it
silently calls if there is underlying [free] function and simply falls
through otherwise,
also, take into account that it could be located in the uninterruptible
part of the code.

On the whole topic I have to

On Wed, Nov 29, 2023 at 4:56 PM Pavel Borisov <pashkin.elfe@gmail.com>
wrote:

> Hi, Alexander!
>
>> I'm planning to review some of the other patches from the current
>> patchset soon.
>>
>
> I've looked into the patch 0003.
> The patch looks in good shape and is uncontroversial to me. Making memory
> structures to be dynamically allocated is simple enough and it allows to
> store complex data like lists etc. I consider places like this that expect
> memory structures to be flat and allocated at once are because the was no
> need in more complex ones previously. If there is a need for them, I think
> they could be added without much doubt, provided the simplicity of the
> change.
>
> For the code:
> +static inline void
> +table_free_rd_amcache(Relation rel)
> +{
> + if (rel->rd_tableam && rel->rd_tableam->free_rd_amcache)
> + {
> + rel->rd_tableam->free_rd_amcache(rel);
> + }
> + else
> + {
> + if (rel->rd_amcache)
> + pfree(rel->rd_amcache);
> + rel->rd_amcache = NULL;
> + }
>
> here I suggest adding Assert(rel->rd_amcache == NULL) (or maybe better an
> error report) after calling free_rd_amcache to be sure the custom
> implementation has done what it should do.
>
> Also, I think some brief documentation about writing this custom method is
> quite relevant maybe based on already existing comments in the code.
>
> Kind regards,
> Pavel
>


-- 
Regards,
Nikita Malakhov
Postgres Professional
The Russian Postgres Company
https://postgrespro.ru/

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