Request a Demo
Home   >   mailing lists

Re: Allow table AMs to define their own reloptions - Mailing list pgsql-hackers

From Ajit Awekar
Subject Re: Allow table AMs to define their own reloptions
Date
Msg-id CAER375NyucpO_f735Q69PaHuJvNZiqrsJJQxrNy30Tmw-vE6Nw@mail.gmail.com
Whole thread
In response to Re: Allow table AMs to define their own reloptions  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: Allow table AMs to define their own reloptions
List pgsql-hackers
Tree view
I tested v8 and found below two issues.

1. SET before RESET.  Final state (heap, fillfactor=50) is valid. but resulting in an error ERROR:  unrecognized parameter "option_int". However Reset before set works as expected.

postgres=# CREATE EXTENSION dummy_table_am;
CREATE EXTENSION
postgres=# CREATE TABLE x (a int);
CREATE TABLE
postgres=# ALTER TABLE x SET ACCESS METHOD dummy_table_am, SET (option_int = 25);
ALTER TABLE
postgres=# SELECT reloptions FROM pg_class WHERE oid = 'x'::regclass;
   reloptions    
-----------------
 {option_int=25}
(1 row)

postgres=# ALTER TABLE x SET ACCESS METHOD heap, SET (fillfactor = 50), RESET (option_int);
ERROR:  unrecognized parameter "option_int"
postgres=# SELECT (SELECT amname FROM pg_am WHERE oid = relam) AS amname, reloptions FROM pg_class WHERE oid = 'x'::regclass;
     amname     |   reloptions    
----------------+-----------------
 dummy_table_am | {option_int=25}
(1 row)

postgres=# ALTER TABLE x SET ACCESS METHOD heap, RESET (option_int), SET (fillfactor = 50);
ALTER TABLE
postgres=# SELECT (SELECT amname FROM pg_am WHERE oid = relam) AS amname, reloptions FROM pg_class WHERE oid = 'x'::regclass;
 amname |   reloptions    
--------+-----------------
 heap   | {fillfactor=50}
(1 row)

2.The current dummy_table_am always embeds StdRdOptions, so it cannot
  catch this. For this issue  make dummy_table_am's struct start with
  {int32 vl_len_; int pad1; int pad2; int option_int;}, keep only the
  option_int registration, dt_relopt_tab[1], and set
  has_std_options_prefix = false.

postgres=# CREATE EXTENSION dummy_table_am;
CREATE EXTENSION
postgres=# CREATE TABLE t0 (a int, b text) USING dummy_table_am WITH (option_int = 0);
ERROR:  unexpected toast_value_type value 0
postgres=# CREATE TABLE t2 (a int, b text) USING dummy_table_am WITH (option_int = 2);
CREATE TABLE
postgres=# SELECT atttypid::regtype AS chunk_id_type FROM pg_attribute WHERE attrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 't2'::regclass) AND attname = 'chunk_id';                                                  
 chunk_id_type
---------------
 oid8
(1 row)
postgres=# CREATE TABLE tn (a int, b text) USING dummy_table_am;
CREATE TABLE
postgres=# SELECT atttypid::regtype AS chunk_id_type FROM pg_attribute WHERE attrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'tn'::regclass) AND attname = 'chunk_id';
 chunk_id_type
---------------
 oid
(1 row)

I think RelationGetToastValueType() is not guarded by RelationHasStdRdOptions() causing this issue.

Possible fix for second issue:
#define RelationGetToastValueType(relation, defaulttarg) \        (RelationHasStdRdOptions(relation) ? \         ((StdRdOptions *) (relation)->rd_options)->toast_value_type : (defaulttarg))

Thanks & Best Regards,
Ajit

On Mon, 28 Sept 2026 at 18:53, Andrew Dunstan <andrew@dunslane.net> wrote:


On 2026-09-08 Tu 8:34 AM, Ajit Awekar wrote:
Hi all,

I forgot to attach the TAM code used for repro. please find it attached

Thanks & Best Regards,
Ajit

On Tue, 8 Sept 2026 at 15:33, Ajit Awekar <ajitpostgres@gmail.com> wrote:
Hi hackers,

I managed to get a crash with the patch. Below are the details


Repro:

Any table AM that sets amoptions but leaves has_std_options_prefix false
and returns a bytea smaller than sizeof(StdRdOptions) will
crash on VACUUM of a table that has a toastable column.

postgres=# CREATE EXTENSION tiny_table_am;
CREATE EXTENSION
postgres=# CREATE TABLE t_tiny (a int, b text) USING tiny_table_am WITH (option_int = 7);
CREATE TABLE
postgres=# INSERT INTO t_tiny VALUES (1, repeat('x', 10000));
INSERT 0 1
postgres=# VACUUM t_tiny;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.


Root cause:
vacuum_rel() in  has two places that read
rel->rd_options as a StdRdOptions to hand storage parameters down to the
relation's TOAST table. Only one of them was updated to use the new
RelationHasStdRdOptions() guard:

    ~line 2214 (correctly guarded):
      relopts = merge_toast_reloptions(RelationHasStdRdOptions(rel) ?
                                       (StdRdOptions *) rel->rd_options : NULL,
                                       params.main_relopts);

    ~line 2310-2312 (still just checks != NULL):
      if (OidIsValid(toast_relid) && rel->rd_options)
      {
          memcpy(&relopts_copy, rel->rd_options, sizeof(StdRdOptions));
          toast_vacuum_params.main_relopts = &relopts_copy;
      }




Suggested fix
-------------
Same guard as the nearby, already-fixed call:

--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2307,9 +2307,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
     * Hand our storage parameters down for the TOAST table to inherit.  Take
     * a copy while we still have the relation open; the relcache entry can go
     * away once we close it.
     */
-   if (OidIsValid(toast_relid) && rel->rd_options)
+   if (OidIsValid(toast_relid) && RelationHasStdRdOptions(rel))
    {
        memcpy(&relopts_copy, rel->rd_options, sizeof(StdRdOptions));
        toast_vacuum_params.main_relopts = &relopts_copy;
    }



Thanks, this should be fixed in v8 attached.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Vlad Lesin
Date:
Subject: Re: ReplicationSlotRelease() clobbers another backend's statusFlags entry
Next
From: Färber, Franz-Josef (StMUK)
Date:
Subject: WG: AW: Extract text from XML, pay attention to XML Entities