Re: Allow table AMs to define their own reloptions

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 & Best Regards,
> Ajit
>
> On Mon, 7 Sept 2026 at 18:11, Aleksander Alekseev <
> aleksander(at)tigerdata(dot)com> wrote:
>
>> Hi Andrew,
>>
>> > Attached is v7. Changes since v6:
>> >
>> > [...]
>>
>> That's an important feature, thanks for driving it. We wanted
>> something like this in the recent commit 0e944fe3 but ended up using a
>> GUC.
>>
>> There is one important design difference however. Your patch allows
>> given TAMs to have their own reloptions. In 0e944fe3 the task was
>> different. An extension that doesn't implement a TableAM should be
>> able to extend `CREATE TABLE foo WITH (...)` with its own options
>> independent of the underlying TAM. The way these options are
>> interpreted depends on the extension, not the underlying TAM.
>>
>> If we could separate reloptions extension from TAM implementation that
>> would be much more flexible. Do you think it's possible?
>>
>> --
>> Best regards,
>> Aleksander Alekseev
>>
>>
>>