Re: Allow table AMs to define their own reloptions

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Ajit Awekar <ajitpostgres(at)gmail(dot)com>, Aleksander Alekseev <aleksander(at)tigerdata(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Junwang Zhao <zhjwpku(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, Julien Tachoires <julien(at)tachoires(dot)me>
Subject: Re: Allow table AMs to define their own reloptions
Date: 2026-09-28 13:23:39
Message-ID: 04c18e23-32c0-4ce8-85f8-e47f1c027ed0@dunslane.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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(at)gmail(dot)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 Content-Type Size
v8-0001-Add-amoptions-callback-to-table-access-methods.patch text/x-patch 65.4 KB

In response to

Browse pgsql-hackers by date

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