Re: Table AM Interface Enhancements
Pavel Borisov <pashkin.elfe@gmail.com>
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
Commits
-
Remove extra comment at TableAmRoutine.scan_analyze_next_block
- 70a845c04a47 17.0 landed
-
revert: Generalize relation analyze in table AM interface
- 6377e12a5a52 17.0 landed
-
Revert: Allow table AM to store complex data structures in rd_amcache
- 922c4c461d21 17.0 landed
-
Revert: Allow table AM tuple_insert() method to return the different slot
- 8dd0bb84da7d 17.0 landed
-
Revert: Allow locking updated tuples in tuple_update() and tuple_delete()
- 193e6d18e553 17.0 landed
-
Revert: Let table AM insertion methods control index insertion
- da841aa4dc27 17.0 landed
-
Revert: Custom reloptions for table AM
- bc1e2092ebb8 17.0 landed
-
Provide a way block-level table AMs could re-use acquire_sample_rows()
- dd1f6b0c172a 17.0 landed
-
Custom reloptions for table AM
- 9bd99f4c26fe 17.0 landed
- c95c25f9af4b 17.0 landed
-
Use streaming I/O in ANALYZE.
- 041b96802efa 17.0 cited
-
Revert "Custom reloptions for table AM"
- 867cc7b6ddb9 17.0 landed
-
Let table AM insertion methods control index insertion
- b1484a3f1910 17.0 landed
-
Generalize relation analyze in table AM interface
- 27bc1772fc81 17.0 landed
-
Improve error message for tts_(virtual|minimal)_is_current_xact_tuple
- 41d3780d3d29 17.0 landed
-
Add comments on some MinimalTupleSlots methods usage
- 10baee0c95d1 17.0 landed
-
Add TupleTableSlotOps.is_current_xact_tuple() method
- 0997e0af273d 17.0 landed
-
Allow table AM tuple_insert() method to return the different slot
- c35a3fb5e067 17.0 landed
-
Allow table AM to store complex data structures in rd_amcache
- 02eb07ea89d2 17.0 landed