agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Álvaro Herrera <alvherre@kurilemu.de>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Ajit Awekar <ajitpostgres@gmail.com>
Cc: Aleksander Alekseev <aleksander@tigerdata.com>
Cc: pgsql-hackers@lists.postgresql.org, Junwang Zhao <zhjwpku@gmail.com>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: Julien Tachoires <julien@tachoires.me>
Subject: Re: Allow table AMs to define their own reloptions
Date: Thu, 1 Oct 2026 16:57:42 +0200
Message-ID: <ar5l4TRlCebwEp5s@alvherre.pgsql> (raw)
In-Reply-To: <a15db9d5-3ebb-4e6a-80e9-c900aad638a0@dunslane.net>
On 2026-Sep-30, Andrew Dunstan wrote:
> diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
> index ea3f2a6be99..6dfc6e8026c 100644
> --- a/src/include/access/tableam.h
> +++ b/src/include/access/tableam.h
> @@ -17,6 +17,7 @@
> #ifndef TABLEAM_H
> #define TABLEAM_H
>
> +#include "access/amapi.h"
> #include "access/relscan.h"
> #include "access/sdir.h"
> #include "access/xact.h"
I don't love this. I have a bunch of patches queued to remove includes
from other includes to reduce cross-header contamination. This kind of
change makes it impossible to remove the cross inclusion here and is
more or less a step backwards. (It's not _too_ bad because tableam.h is
not as widely used
Is there a better way to have a function definition that can be used in
both amapi.h and tableam.h without this cross-header inclusion?
Other comments on verbiage, just passing by:
I'm not much in love with the LLM-written comments TBH -- I don't think
phrases like "owns the option set entirely" are valuable, for example.
Also, the comment just above the ATValidateAccessMethodOptions() call in
ATController is redundant: it would be enough to say "validate options
as needed", and then have the comment atop ATValidateAccessMethodOptions()
itself carry the explanation of what we do and why.
In tableam.sgml, I'm not sure it makes much sense to state "The callback
has the same signature as the corresponding index AM callback". Why not
just say what the signature is without directing the user to read a
reference page that's not relevant to the topic of table AMs? I think
the first mention of reloptions in that page should be <firstterm>.
It also talks about validating and throwing ereport(ERROR) but it
doesn't say in so many words what must happen or not happen on each
possible value of 'validate'. It could be clearer.
There's also "raises an error rather than silently dropping the value".
I mean, why not say "rather than launching an ICBM"? Why not just
"raises an error, period"?
If I were the user of such an AM, I would not be sure how to interpret
the phrase "validate that the option read with SELECT reloptions FROM
pg_class are the ones that will be used". What does that mean exactly?
Should it say "examine" rather than "validate"?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"No me acuerdo, pero no es cierto. No es cierto, y si fuera cierto,
no me acuerdo." (Augusto Pinochet a una corte de justicia)
view thread (30+ messages)
Message-ID: <ar5l4TRlCebwEp5s@alvherre.pgsql>
Permalink: ../ar5l4TRlCebwEp5s@alvherre.pgsql/
Also on: postgresql.org/message-id/ar5l4TRlCebwEp5s@alvherre.pgsql
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de, andrew@dunslane.net, ajitpostgres@gmail.com, aleksander@tigerdata.com, zhjwpku@gmail.com, zsolt.parragi@percona.com, rafia.pghackers@gmail.com, julien@tachoires.me
Subject: Re: Allow table AMs to define their own reloptions
In-Reply-To: <ar5l4TRlCebwEp5s@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox