public inbox for [email protected]  
help / color / mirror / Atom feed
From: Jeff Davis <[email protected]>
To: Sadhuprasad Patro <[email protected]>
To: Jelte Fennema <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Per-table storage parameters for TableAM/IndexAM extensions
Date: Thu, 10 Feb 2022 10:37:54 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFF0-CFSy0wJCTfjJ6QLsHZGfFLAVKiHzgdTJyNnzqJfWpwh7w@mail.gmail.com>
References: <CAFF0-CG4KZHdtYHMsonWiXNzj16gWZpduXAn8yF7pDDub+GQMg@mail.gmail.com>
	<CAGECzQRDj2UrOvg-rJfWdTAPjh0WEjt7xBWV61bGTOH7jONcPw@mail.gmail.com>
	<CAFF0-CFSy0wJCTfjJ6QLsHZGfFLAVKiHzgdTJyNnzqJfWpwh7w@mail.gmail.com>

On Tue, 2022-01-18 at 22:44 +0530, Sadhuprasad Patro wrote:
> As of now, I have fixed the comments from Dilip & Rushabh and have
> done some more changes after internal testing and review. Please find
> the latest patch attached.

Hi,

Thank you for working on this! Some questions/comments:

At a high level, it seems there are some options that are common to all
tables, regardless of the AM. For instance, the vacuum/autovacuum
options. (Even if the AM doesn't require vacuum, then it needs to at
least be able to communicate that somehow.) I think parallel_workers
and user_catalog_table also fit into this category. That means we need
all of StdRdOptions to be the same, with the possible exception of
toast_tuple_target and/or fillfactor.

The current patch just leaves it up to the AM to return a bytea that
can be cast to StdRdOptions, which seems like a fragile API.

That makes me think that what we really want is to have *extra* options
for a table AM, not an entirely custom set. Do you agree?

If so, I suggest you refactor so that if validation doesn't recognize a
parameter, it calls a table AM method to validate it, and lets it in if
validation succeeds. That way all the stuff around StdRdOptions is
unchanged. When the table AM needs the parameter value, it can parse
pg_class.reloptions for itself and save it in rd_amcache.

Regards,
	Jeff Davis








view thread (2+ messages)

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Per-table storage parameters for TableAM/IndexAM extensions
  In-Reply-To: <[email protected]>

* 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