MIME-Version: 1.0
References: <CAF4Au4zsdZ3t=f+YAaq12-SzqfCxfOTc1NanjBcUsw+5L-0iiA@mail.gmail.com>
 <224711f9-83b7-a307-b17f-4457ab73aa0a@sigaev.ru> <CA+TgmoYOZ0Ofoo1DJ+iRa6Mg8MACu0eb8tr-uggfNMZmD6bpkA@mail.gmail.com>
 <e2659551-b473-761e-0fe5-6c34f0480882@sigaev.ru>
In-Reply-To: <e2659551-b473-761e-0fe5-6c34f0480882@sigaev.ru>
From: Nikita Malakhov <hukutoc@gmail.com>
Date: Thu, 10 Mar 2022 11:47:32 +0300
Message-ID: <CAN-LCVORMn85jAL4GCWpQf+H+eZ3vFoO2r+tOEn7k47uw2A9gg@mail.gmail.com>
Subject: Re: Pluggable toaster
To: Teodor Sigaev <teodor@sigaev.ru>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000006c1a0405d9d94468"
Archived-At: <https://www.postgresql.org/message-id/CAN-LCVORMn85jAL4GCWpQf%2BH%2BeZ3vFoO2r%2BtOEn7k47uw2A9gg%40mail.gmail.com>
Precedence: bulk

--0000000000006c1a0405d9d94468
Content-Type: multipart/alternative; boundary="0000000000006c1a0205d9d94466"

--0000000000006c1a0205d9d94466
Content-Type: text/plain; charset="UTF-8"

Hi Hackers,

In addition to original patch set for Pluggable Toaster, we have two more
patches
(actually, small, but important fixes), authored by Nikita Glukhov:

1) 0001-Fix-toast_tuple_externalize.patch
This patch fixes freeing memory in case of new toasted value is the same as
old one,
this seems incorrect, and in this case the function just returns instead of
freeing old value;

2) 0002-Fix-alignment-of-custom-TOAST-pointers.patch
This patch adds data alignment for new varatt_custom data structure in
building tuples,
since varatt_custom must be aligned for custom toasters (in particular,
this fix is very
important to JSONb Toaster).

These patches must be applied on top of the latter
4_bytea_appendable_toaster_v1.patch.
Please consider them in reviewing Pluggable Toaster.

Regards.


On Wed, Feb 2, 2022 at 10:35 AM Teodor Sigaev <teodor@sigaev.ru> wrote:

> > I agree ... but I'm also worried about what happens when we have
> > multiple table AMs. One can imagine a new table AM that is
> > specifically optimized for TOAST which can be used with an existing
> > heap table. One can imagine a new table AM for the main table that
> > wants to use something different for TOAST. So, I don't think it's
> > right to imagine that the choice of TOASTer depends solely on the
> > column data type. I'm not really sure how this should work exactly ...
> > but it needs careful thought.
>
> Right. that's why we propose a validate method  (may be, it's a wrong
> name, but I don't known better one) which accepts several arguments, one
> of which is table AM oid. If that method returns false then toaster
> isn't useful with current TAM, storage or/and compression kinds, etc.
>
> --
> Teodor Sigaev                      E-mail: teodor@sigaev.ru
>                                        WWW: http://www.sigaev.ru/
>
>
>
--
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/

--0000000000006c1a0205d9d94466
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Hackers,</div><div dir=3D"ltr"><br><di=
v>In addition to original patch set for Pluggable Toaster, we have two more=
 patches=C2=A0</div><div>(actually, small, but important fixes), authored b=
y Nikita Glukhov:</div><div><br></div><div>1) 0001-Fix-toast_tuple_external=
ize.patch</div><div>This patch fixes freeing memory in case of new toasted =
value is the same as old one,=C2=A0</div><div>this seems incorrect, and in =
this case the function just returns instead of freeing old value;</div><div=
><br></div><div>2) 0002-Fix-alignment-of-custom-TOAST-pointers.patch</div><=
div>This patch adds data alignment for new varatt_custom data structure in =
building tuples,=C2=A0</div><div>since varatt_custom must be aligned for cu=
stom toasters (in particular, this fix is very=C2=A0</div><div>important to=
 JSONb Toaster).</div><div><br></div><div>These patches must be applied on =
top of the latter 4_bytea_appendable_toaster_v1.patch.</div><div>Please con=
sider them in reviewing Pluggable Toaster.</div><div><br></div><div>Regards=
.</div><div><br></div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr"=
 class=3D"gmail_attr">On Wed, Feb 2, 2022 at 10:35 AM Teodor Sigaev &lt;<a =
href=3D"mailto:teodor@sigaev.ru">teodor@sigaev.ru</a>&gt; wrote:<br></div><=
blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l=
eft:1px solid rgb(204,204,204);padding-left:1ex">&gt; I agree ... but I&#39=
;m also worried about what happens when we have<br>
&gt; multiple table AMs. One can imagine a new table AM that is<br>
&gt; specifically optimized for TOAST which can be used with an existing<br=
>
&gt; heap table. One can imagine a new table AM for the main table that<br>
&gt; wants to use something different for TOAST. So, I don&#39;t think it&#=
39;s<br>
&gt; right to imagine that the choice of TOASTer depends solely on the<br>
&gt; column data type. I&#39;m not really sure how this should work exactly=
 ...<br>
&gt; but it needs careful thought.<br>
<br>
Right. that&#39;s why we propose a validate method=C2=A0 (may be, it&#39;s =
a wrong <br>
name, but I don&#39;t known better one) which accepts several arguments, on=
e <br>
of which is table AM oid. If that method returns false then toaster <br>
isn&#39;t useful with current TAM, storage or/and compression kinds, etc.<b=
r>
<br>
-- <br>
Teodor Sigaev=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 E-mail: <a href=3D"mailto:teodor@sigaev.ru" target=3D"_blank=
">teodor@sigaev.ru</a><br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0WWW: <a h=
ref=3D"http://www.sigaev.ru/" rel=3D"noreferrer" target=3D"_blank">http://w=
ww.sigaev.ru/</a><br>
<br>
<br>
</blockquote></div><br clear=3D"all"><div>--<br></div><div dir=3D"ltr" clas=
s=3D"gmail_signature"><div dir=3D"ltr"><div style=3D"color:rgb(34,34,34)">N=
ikita Malakhov<br></div><div style=3D"color:rgb(34,34,34)">Postgres Profess=
ional=C2=A0</div><div style=3D"color:rgb(34,34,34)"><a href=3D"https://post=
grespro.ru/" target=3D"_blank">https://postgrespro.ru/</a></div></div></div=
></div>

--0000000000006c1a0205d9d94466--
--0000000000006c1a0405d9d94468
Content-Type: application/octet-stream; 
	name="0001-Fix-toast_tuple_externalize.patch"
Content-Disposition: attachment; 
	filename="0001-Fix-toast_tuple_externalize.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_l0kqwntt0>
X-Attachment-Id: f_l0kqwntt0

RnJvbSBjMTYxMTRiZjE3Y2VlNWI1NjQ5ZDNkZmE0NjJjNGE4YjU5NGZiMmYwIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBOaWtpdGEgR2x1a2hvdiA8bi5nbHVob3ZAcG9zdGdyZXNwcm8u
cnU+CkRhdGU6IFR1ZSwgMjIgRmViIDIwMjIgMjA6NTI6NDUgKzAzMDAKU3ViamVjdDogW1BBVENI
IDEvMl0gRml4IHRvYXN0X3R1cGxlX2V4dGVybmFsaXplKCkKCi0tLQogc3JjL2JhY2tlbmQvYWNj
ZXNzL3RhYmxlL3RvYXN0X2hlbHBlci5jIHwgMyArKysKIDEgZmlsZSBjaGFuZ2VkLCAzIGluc2Vy
dGlvbnMoKykKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9hY2Nlc3MvdGFibGUvdG9hc3RfaGVs
cGVyLmMgYi9zcmMvYmFja2VuZC9hY2Nlc3MvdGFibGUvdG9hc3RfaGVscGVyLmMKaW5kZXggYmZi
ZTk0MWQ2YWMuLjExZDg3MDUzNWUxIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9hY2Nlc3MvdGFi
bGUvdG9hc3RfaGVscGVyLmMKKysrIGIvc3JjL2JhY2tlbmQvYWNjZXNzL3RhYmxlL3RvYXN0X2hl
bHBlci5jCkBAIC0zNDUsNiArMzQ1LDkgQEAgdG9hc3RfdHVwbGVfZXh0ZXJuYWxpemUoVG9hc3RU
dXBsZUNvbnRleHQgKnR0YywgaW50IGF0dHJpYnV0ZSwgaW50IG1heERhdGFMZW4sCiAJCQkJCQkJ
CQkJIG1heERhdGFMZW4sIG9wdGlvbnMpCiAJCQkpOwogCisJaWYgKCp2YWx1ZSA9PSBvbGRfdmFs
dWUpCisJCXJldHVybjsKKwogCWlmICgoYXR0ci0+dGFpX2NvbGZsYWdzICYgVE9BU1RDT0xfTkVF
RFNfRlJFRSkgIT0gMCkKIAkJcGZyZWUoRGF0dW1HZXRQb2ludGVyKG9sZF92YWx1ZSkpOwogCWF0
dHItPnRhaV9jb2xmbGFncyB8PSBUT0FTVENPTF9ORUVEU19GUkVFOwotLSAKMi4yNS4xCgo=
--0000000000006c1a0405d9d94468
Content-Type: application/octet-stream; 
	name="0002-Fix-alignment-of-custom-TOAST-pointers.patch"
Content-Disposition: attachment; 
	filename="0002-Fix-alignment-of-custom-TOAST-pointers.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_l0kqwnu11>
X-Attachment-Id: f_l0kqwnu11

RnJvbSA2ZGI2NjMwOTQzYmUzOWVlZTMxY2IwNDExZTJhODQ5ZWM5N2JjNjZjIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBOaWtpdGEgR2x1a2hvdiA8bi5nbHVob3ZAcG9zdGdyZXNwcm8u
cnU+CkRhdGU6IFdlZCwgMTIgSmFuIDIwMjIgMDE6MjM6MTggKzAzMDAKU3ViamVjdDogW1BBVENI
IDIvMl0gRml4IGFsaWdubWVudCBvZiBjdXN0b20gVE9BU1QgcG9pbnRlcnMKCi0tLQogc3JjL2Jh
Y2tlbmQvYWNjZXNzL2NvbW1vbi9oZWFwdHVwbGUuYyB8IDUgKysrKy0KIHNyYy9pbmNsdWRlL2Fj
Y2Vzcy90dXBtYWNzLmggICAgICAgICAgfCAyICstCiAyIGZpbGVzIGNoYW5nZWQsIDUgaW5zZXJ0
aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9hY2Nlc3Mv
Y29tbW9uL2hlYXB0dXBsZS5jIGIvc3JjL2JhY2tlbmQvYWNjZXNzL2NvbW1vbi9oZWFwdHVwbGUu
YwppbmRleCAwYjU2YjBmYTVhOS4uNDdjODA4ZDQ2MmYgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5k
L2FjY2Vzcy9jb21tb24vaGVhcHR1cGxlLmMKKysrIGIvc3JjL2JhY2tlbmQvYWNjZXNzL2NvbW1v
bi9oZWFwdHVwbGUuYwpAQCAtMjQyLDcgKzI0MiwxMCBAQCBmaWxsX3ZhbChGb3JtX3BnX2F0dHJp
YnV0ZSBhdHQsCiAJCQllbHNlCiAJCQl7CiAJCQkJKmluZm9tYXNrIHw9IEhFQVBfSEFTRVhURVJO
QUw7Ci0JCQkJLyogbm8gYWxpZ25tZW50LCBzaW5jZSBpdCdzIHNob3J0IGJ5IGRlZmluaXRpb24g
Ki8KKwkJCQlpZiAoVkFSQVRUX0lTX0NVU1RPTSh2YWwpKQorCQkJCQlkYXRhID0gKGNoYXIgKikg
YXR0X2FsaWduX25vbWluYWwoZGF0YSwKKwkJCQkJCQkJCQkJCQkgIGF0dC0+YXR0YWxpZ24pOwor
CQkJCS8qIGVsc2Ugbm8gYWxpZ25tZW50LCBzaW5jZSBpdCdzIHNob3J0IGJ5IGRlZmluaXRpb24g
Ki8KIAkJCQlkYXRhX2xlbmd0aCA9IFZBUlNJWkVfRVhURVJOQUwodmFsKTsKIAkJCQltZW1jcHko
ZGF0YSwgdmFsLCBkYXRhX2xlbmd0aCk7CiAJCQl9CmRpZmYgLS1naXQgYS9zcmMvaW5jbHVkZS9h
Y2Nlc3MvdHVwbWFjcy5oIGIvc3JjL2luY2x1ZGUvYWNjZXNzL3R1cG1hY3MuaAppbmRleCA2NWFj
MWVmM2ZjOC4uZGRiMTY0ZWYyYmEgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL2FjY2Vzcy90dXBt
YWNzLmgKKysrIGIvc3JjL2luY2x1ZGUvYWNjZXNzL3R1cG1hY3MuaApAQCAtMTA0LDcgKzEwNCw3
IEBACiAgKi8KICNkZWZpbmUgYXR0X2FsaWduX2RhdHVtKGN1cl9vZmZzZXQsIGF0dGFsaWduLCBh
dHRsZW4sIGF0dGRhdHVtKSBcCiAoIFwKLQkoKGF0dGxlbikgPT0gLTEgJiYgVkFSQVRUX0lTX1NI
T1JUKERhdHVtR2V0UG9pbnRlcihhdHRkYXR1bSkpKSA/IFwKKwkoKGF0dGxlbikgPT0gLTEgJiYg
KCFWQVJBVFRfSVNfQ1VTVE9NKERhdHVtR2V0UG9pbnRlcihhdHRkYXR1bSkpICYmIFZBUkFUVF9J
U19TSE9SVChEYXR1bUdldFBvaW50ZXIoYXR0ZGF0dW0pKSkpID8gXAogCSh1aW50cHRyX3QpIChj
dXJfb2Zmc2V0KSA6IFwKIAlhdHRfYWxpZ25fbm9taW5hbChjdXJfb2Zmc2V0LCBhdHRhbGlnbikg
XAogKQotLSAKMi4yNS4xCgo=
--0000000000006c1a0405d9d94468--