Re: Pluggable toaster

Hi,
I reworked previous patch set according to recommendations. Patches
are generated by format-patch and applied by git am. Patches are based on
master from 03.11. Also, now we've got clean branch with incremental
commits
which could be easily rebased onto a fresh master.

Currently, there are 8 patches:

1) 0001_create_table_storage_v3.patch - SET STORAGE option for CREATE
TABLE command by Teodor Sigaev which is required by all the following
functionality;

2) 0002_toaster_interface_v6.patch - Toaster API (SQL syntax for toasters +
API)
with Dummy toaster as an example of how this API should be used, but with
default
toaster left 'as-is';

3) 0003_toaster_default_v5.patch - default (regular) toaster is implemented
via new API;

4) 0004_toaster_snapshot_v5.patch - refactoring of default toaster and
support
of versioned toasted rows;

5) 0005_bytea_appendable_toaster_v5.patch - bytea toaster by Nikita Glukhov
Custom toaster for bytea data with support of appending (instead of
rewriting)
stored data;

6) 0006_toasterapi_docs_v1.patch - brief documentation on Toaster API in Pg
docs;

7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom toast
pointer's
alignment required by bytea toaster by Nikita Glukhov;

8) 0008_fix_toast_tuple_externalize.patch - fixes toast_tuple_externalize
function
not to call toast if old data is the same as new one.

I would be grateful for feedback on the reworked patch set.

On Mon, Apr 4, 2022 at 11:18 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <hukutoc(at)gmail(dot)com> wrote:
> > - Is 'git apply' not a valid way to apply such patches?
>
> I have found that it never works. This case is no exception:
>
> [rhaas pgsql]$ git apply ~/Downloads/1_toaster_interface_v4.patch
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:253: trailing
> whitespace.
> toasterapi.o
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1276: trailing
> whitespace.
> {
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1294: trailing
> whitespace.
> * CREATE TOASTER name HANDLER handler_name
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:2261: trailing
> whitespace.
> * va_toasterdata could contain varatt_external structure for old Toast
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:3047: trailing
> whitespace.
> SELECT attnum, attname, atttypid, attstorage, tsrname
> error: patch failed: src/backend/commands/tablecmds.c:42
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:943
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:973
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:44
> error: src/backend/commands/tablecmds.c: patch does not apply
>
> I would really encourage you to use 'git format-patch' to generate a
> stack of patches. But there is no point in reposting 30+ patches that
> haven't been properly refactored into separate chunks. You need to
> maintain a branch, periodically rebased over master, with some
> probably-small number of patches on it, each of which is a logically
> independent patch with its own commit message, its own clear purpose,
> etc. And then generate patches to post from there using 'git
> format-patch'. Look into using 'git rebase -i --autosquash' and 'git
> commit --fixup' to maintain the branch, if you're not already familiar
> with those things.
>
> Also, it is a really good idea when you post the patch set to include
> in the email a clear description of the overall purpose of the patch
> set and what each patch does toward that goal. e.g. "The overall goal
> of this patch set is to support faster-than-light travel. Currently,
> PostgreSQL does not know anything about the speed of light, so 0001
> adds some code for speed-of-light detection. Building on this, 0002
> adds general support for disabling physical laws of the universe.
> Then, 0003 makes use of this support to disable specifically the speed
> of light." Perhaps you want a little more text than that for each
> patch, depending on the situation, but this gives you the idea, I
> hope.
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>

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