Re: Per-table storage parameters for TableAM/IndexAM extensions

Поиск
Список
Период
Сортировка
От Dilip Kumar
Тема Re: Per-table storage parameters for TableAM/IndexAM extensions
Дата
Msg-id CAFiTN-sXf+JQ4X8yOee2KLpkmE216Hjmvv1vY43Mac_UKRx0xA@mail.gmail.com
обсуждение исходный текст
Ответ на Per-table storage parameters for TableAM/IndexAM extensions  (Sadhuprasad Patro <b.sadhu@gmail.com>)
Список pgsql-hackers
On Wed, Dec 29, 2021 at 10:38 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:
Hi,

Currently all the storage options for a table are very much specific
to the heap but a different AM might need some user defined AM
specific parameters to help tune the AM. So here is a patch which
provides an AM level routine so that instead of getting parameters
validated using “heap_reloptions” it will call the registered AM
routine.

+1 for the idea.
 

e.g:
-- create a new access method and table using this access method
CREATE ACCESS METHOD myam TYPE TABLE HANDLER <new_tableam_handler>;

CREATE TABLE mytest (a int) USING myam ;

--a new parameter is to set storage parameter for only myam as below
ALTER TABLE mytest(squargle_size = '100MB');

This will work for CREATE TABLE as well I guess as normal relation storage parameter works now right?


The user-defined parameters will have meaning only for the "myam",
otherwise error will be thrown. Our relcache already allows the
AM-specific cache to be stored for each relation.

Open Question: When a user changes AM, then what should be the
behavior for not supported storage options? Should we drop the options
and go with only system storage options?
Or throw an error, in which case the user has to clean the added parameters.

IMHO, if the user is changing the access method for the table then it should be fine to throw an error if there are some parameters which are not supported by the new AM.  So that user can take a calculative call and first remove those storage options before changing the AM.

I have a few comments on the patch, mostly cosmetics.

1.
+ Assert(routine->taboptions != NULL);

Why AM is not allowed to register the NULL function, if NULL is registered that means the AM
does not support any of the storage parameters.
2.
@@ -1358,6 +1358,7 @@ untransformRelOptions(Datum options)
  return result;
 }
 
+
 /*
  * Extract and parse reloptions from a pg_class tuple.
  *

Unwanted hunk (added extra line)

3.
+ * Parse options for heaps, views and toast tables. This is
+ * implementation of relOptions for access method heapam.
  */

Better to say access method heap instead of heapam.
4.
+ * Parse options for tables.
+ *
+ * taboptions tables AM's option parser function
+ *      reloptions options as text[] datum
+ *      validate error flag

Function header comment formatting is not proper, it also has uneven spacing between words.
5.
-extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
+extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc,
+ reloptions_function taboptions)

Indentation is not proper, run pgindent on this.

5.
>Currently all the storage options for a table are very much specific to the heap but a different AM might need some user defined AM specific parameters to help tune the AM. So here is a patch which provides an AM level routine so that instead of getting >parameters validated using “heap_reloptions” it will call the registered AM routine.

Wrap these long commit message lines at 80 characters.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

В списке pgsql-hackers по дате отправления:

Предыдущее
От: SATYANARAYANA NARLAPURAM
Дата:
Сообщение: Re: Throttling WAL inserts when the standby falls behind more than the configured replica_lag_in_bytes
Следующее
От: Dilip Kumar
Дата:
Сообщение: Re: Throttling WAL inserts when the standby falls behind more than the configured replica_lag_in_bytes