Re: Table AM Interface Enhancements - Mailing list pgsql-hackers

From Nikita Malakhov
Subject Re: Table AM Interface Enhancements
Date
Msg-id CAN-LCVOwWpTvSyeXDyMBCZdGZo-BE_M2HXwjE7kFLDY-UdZ4-Q@mail.gmail.com
Whole thread Raw
In response to Re: Table AM Interface Enhancements  (Pavel Borisov <pashkin.elfe@gmail.com>)
Responses Re: Table AM Interface Enhancements
List 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

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: trying again to get incremental backup
Next
From: Pavel Borisov
Date:
Subject: Re: Table AM Interface Enhancements