Re: Allow table AMs to define their own reloptions

From: Ajit Awekar <ajitpostgres(at)gmail(dot)com>
To: Aleksander Alekseev <aleksander(at)tigerdata(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Andrew Dunstan <andrew(at)dunslane(dot)net>, 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-08 10:03:15
Message-ID: CAER375N_RfkWev54wSj9wmX9dDjX5Ms7wEmpn3f8JJ9RA6QWug@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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
>
>
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrei Lepikhov 2026-09-08 10:07:48 Re: [PATCH] Planner support function for generate_subscripts()
Previous Message cca5507 2026-09-08 10:02:52 [PATCH] Report userid to PgBackendStatus for standalone backends