Re: Pluggable toaster - Mailing list pgsql-hackers

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@gmail.com> wrote:

On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <hukutoc@gmail.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/