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

From Andrew Dunstan
Subject Re: Allow table AMs to define their own reloptions
Date
Msg-id 04c18e23-32c0-4ce8-85f8-e47f1c027ed0@dunslane.net
Whole thread
In response to Allow table AMs to define their own reloptions  (Julien Tachoires <julien@tachoires.me>)
List pgsql-hackers


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
Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: Stabilize and shorten test_checksums/013_rewind test
Next
From: Nitin Motiani
Date:
Subject: [PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check