On 2024/03/07 16:59, Ronan Dunklau wrote:
Le mercredi 6 mars 2024, 20:28:44 CET Stephen Frost a écrit :
I agree that this would generally be a useful thing to have.

Thanks !


Does that seem correct ?

Definitely needs to have a 'REVOKE ALL ON FUNCTION' at the end of the
upgrade script, similar to what you'll find at the bottom of
pg_visibility--1.1.sql in the tree today, otherwise anyone could run it.

Beyond that, I'd suggest a function-level comment above the definition
of the function itself (which is where we tend to put those- not at the
point where we declare the function).

Thank you for the review. Here is an updated patch for both of those.

Here are my review comments:

The documentation for pg_freespace needs updating.


A regression test for pg_truncate_freespace_map() should be added.


+       /* Only some relkinds have a freespace map */
+       if (!RELKIND_HAS_TABLE_AM(rel->rd_rel->relkind))
+               ereport(ERROR,
+                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                errmsg("relation \"%s\" is of wrong relation 
kind",
+                                               RelationGetRelationName(rel)),
+                                
errdetail_relkind_not_supported(rel->rd_rel->relkind)));

An index can have an FSM, but this code doesn't account for that.


+               smgrtruncate(RelationGetSmgr(rel), &fork, 1, &block);

Shouldn't truncation be performed after WAL-logging due to the WAL rule?
I'm not sure if the current order might actually cause significant issues
in FSM truncation case, though.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


Reply via email to