Re: Pluggable toaster

Hi,
Thanks for advices.
We have 4 branches, for each patch provided, you can check them out -
(come copy-paste from the very fist email, where the patches were proposed)
1) 1_toaster_interface
https://github.com/postgrespro/postgres/tree/toaster_interface
Introduces syntax for storage and formal toaster API. Adds column
atttoaster to pg_attribute, by design this column should not be equal to
invalid oid for any toastable datatype, ie it must have correct oid for
any type (not column) with non-plain storage. Since toaster may support
only particular datatype, core should check correctness of toaster set
by toaster validate method. New commands could be found in
src/test/regress/sql/toaster.sql. Also includes modification of pg_dump.

2) 2_toaster_default
https://github.com/postgrespro/postgres/tree/toaster_default
Built-in toaster implemented (with some refactoring) using toaster API
as generic (or default) toaster. dummy_toaster here is a minimal
workable example, it saves value directly in toast pointer and fails if
value is greater than 1kb.

3) 3_toaster_snapshot
https://github.com/postgrespro/postgres/tree/toaster_snapshot
The patch implements technology to distinguish row's versions in toasted
values to share common parts of toasted values between different
versions of rows

4) 4_bytea_appendable_toaster
https://github.com/postgrespro/postgres/tree/bytea_appendable_toaster
Contrib module implements toaster for non-compressed bytea columns,
which allows fast appending to existing bytea value. Appended tail
stored directly in toaster pointer, if there is enough space to do it.

Working on refactoring according to your recommendations.
Thank you!

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/