Re: Pluggable toaster

Hi hackers!

I've reworked the patch set according to recommendations of Aleksander
Alekseev, Robert Haas
and Matthias van de Meent, and decided, as it was recommended earlier,
include only the most
important part in the first set. Also, I've added a large README on
Pluggable TOAST to sources,
I'll be grateful for feedback on this README file. Also, some minor fixes
were included in patches.

Now patch set consists of 3 incremental patches:
0001_create_table_storage_v6.patch - This patch adds important part of SQL
syntax fix for
CREATE TABLE clause, which is mandatory for all Pluggable TOAST
functionality - processing of
SET STORAGE option correctly.
This patch is presented by Teodor Sigaev and is already discussed and
marked as ready for commit
in a separate thread;

0002_toaster_interface_v9.patch - The patch introduces TOAST API interface
and SQL syntax
allowing creation of custom Toaster (CREATE TOASTER ...) and assigning
Toaster to a table column
CREATE TABLE t (data bytea STORAGE EXTERNAL TOASTER bytea_toaster);
Default TOAST functionality is left intact, for the sake of not-so-big
one-time changes, and nothing
changes from user's perspective, but here user already can develop and plug
in custom Toasters;

0003_toaster_default_v8.patch - Introducing Default TOAST implemented via
TOAST API, and
a large README on Pluggable TOAST functionality for developers, put into
/src/backend/access/toast/README.toastapi

With a respect to Aleksander Alekseev
>>There is no need for contrib/dummy_toaster
>>similarly as there is no contrib/ for a dummy TableAM. The provided
>>example doesn't do much anyway since all the heavy lifting should be
>>done in the callbacks implementation.
we decided to leave the dummy Toaster as it is, because it's purpose is not
to show heavy lifting
done by internal Toaster implementation, but to demonstrate how Toaster
extension is defined
and plugged in.

Thank you for your attention and feedbacks!

On Fri, Jul 1, 2022 at 11:10 AM Aleksander Alekseev <
aleksander(at)timescale(dot)com> wrote:

> Hi Nikita,
>
> > Here is the patch set rebased onto current master (15 rel beta 2 with
> commit from 29.06).
>
> Thanks for the rebased patchset.
>
> This is a huge effort though. I suggest splitting it into several CF
> entries as we previously did with other patches (64-bit XIDs to name
> one, which BTW is arguably much simpler, but still we had to split
> it). This will simplify the review, limit the scope of the discussion
> and simplify passing the CI. Cfbot is already not happy with the
> patchset.
>
> 0001 - is already in a separate thread [1], that's good. I suggest
> marking it in the patch description for clarity.
> 0002, 0003 - I suggest focusing on these two in this thread and keep
> the rest of the changes for later discussion. Please submit 0004,
> 0005... next time, when we finish with 0001-0003.
>
> The order of proposed changes IMO is wrong.
>
> 0002 should refactor the default TOASTer in a manner similar to a
> pluggable one. Nothing should change from the user's perspective. If
> you do benchmarks, I suggest not to reference the previous talks. I
> familiarized myself with all the related talks linked before (took me
> some time...) and found them useless for the discussion since they
> don't provide exact steps to reproduce. Please provide exact scripts
> and benchmarks results for 0002 in this thread.
>
> 0003 should add an interface that allows replacing the default TOASTer
> with an alternative one. There is no need for contrib/dummy_toaster
> similarly as there is no contrib/ for a dummy TableAM. The provided
> example doesn't do much anyway since all the heavy lifting should be
> done in the callbacks implementation. For test purposes please use
> src/test/regress/.
>
> [1]:
> https://www.postgresql.org/message-id/de83407a-ae3d-a8e1-a788-920eb334f25b%40sigaev.ru
>
> --
> Best regards,
> Aleksander Alekseev
>

--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/