From: | Nikita Malakhov <hukutoc(at)gmail(dot)com> |
---|---|
To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Greg Stark <stark(at)mit(dot)edu>, Teodor Sigaev <teodor(at)sigaev(dot)ru> |
Subject: | Re: Pluggable toaster |
Date: | 2022-08-02 06:15:12 |
Message-ID: | CAN-LCVNLfu-1n94CnZTeD7stToJ=xXGJhrENHjqONZ3n79RRqQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi hackers!
Sorry for the delay.
I've made changes according to Aleksander comments:
>This will produce a patchset with a consistent naming like:
>...
>Also cfbot [1] will know in which order to apply them.
Done.
>Unfortunately the three patches in question from this branch don't
>pass `make check`. Please update
>src/test/regress/expected/publication.out and make sure the patchset
>passes the rest of the tests at least on one platform before
>submitting.
Done, there was absent column in Publication tests.
>Finally, please update the commit messages. Each commit message should
>include a brief description (one line) , a detailed description (the
>body), and also the list of the authors, the reviewers and a link to
>the discussion. Please use [3] as a template.
Done.
Link to the rebased branch:
https://github.com/postgrespro/postgres/tree/toasterapi_clean
Please check.
Attach includes:
v11-0002-toaster-interface.patch - contains TOAST API with default Toaster
left as-is (reference implementation) and Dummy toaster as an example
(will be removed later as a part of refactoring?).
v11-0003-toaster-default.patch - implements reference TOAST as Default
Toaster
via TOAST API, so Heap AM calls Toast only via API, and does not have direct
calls to Toast functionality.
v11-0004-toaster-snapshot.patch - supports row versioning for TOASTed values
and some refactoring.
Aleksander, thank you and Matthias for the very helpful feedback!
On Fri, Jul 29, 2022 at 9:16 AM Nikita Malakhov <hukutoc(at)gmail(dot)com> wrote:
> Hi hackers!
>
> Aleksander, thanks for the remark, seems that we've missed recent change -
> the pubication
> test does not have the new column 'Toaster'. Will send a corrected patch
> tomorrow. Also, thanks
> for the patch name note, I've changed it as you suggested.
> I'm on vacation, so I read emails not very often and answers take some
> time, sorry.
>
>
> On Tue, Jul 26, 2022 at 11:23 AM Aleksander Alekseev <
> aleksander(at)timescale(dot)com> wrote:
>
>> Hi Nikita,
>>
>> Thanks for an update!
>>
>> > 0002_toaster_interface_v10 contains TOAST API with Dummy toaster as an
>> example (but I would,
>> > as recommended, remove Dummy toaster and provide it as an extension),
>> and default Toaster
>> > was left as-is (reference implementation).
>> >
>> > 0003_toaster_default_v9 implements reference TOAST as Default Toaster
>> via TOAST API,
>> > so Heap AM calls Toast only via API, and does not have direct calls to
>> Toast functionality.
>> >
>> > 0004_toaster_snapshot_v8 continues refactoring and has some important
>> changes (added
>> > into README.toastapi new part related TOAST API extensibility - the
>> virtual functions table).
>>
>> This numbering is confusing. Please use a command like:
>>
>> ```
>> git format-patch origin/master -v 42
>> ```
>>
>> This will produce a patchset with a consistent naming like:
>>
>> ```
>> v42-0001-foo-bar.patch
>> v42-0002-baz-qux.patch
>> ... etc ...
>> ```
>>
>> Also cfbot [1] will know in which order to apply them.
>>
>> > GIT branch with this patch resides here:
>> > https://github.com/postgrespro/postgres/tree/toasterapi_clean
>>
>> Unfortunately the three patches in question from this branch don't
>> pass `make check`. Please update
>> src/test/regress/expected/publication.out and make sure the patchset
>> passes the rest of the tests at least on one platform before
>> submitting.
>>
>> Personally I have a little set of scripts for this [2]. The following
>> commands should pass:
>>
>> ```
>> # quick check
>> ./quick-build.sh && ./single-install.sh && make installcheck
>>
>> # full check
>> ./full-build.sh && ./single-install.sh && make installcheck-world
>> ```
>>
>> Finally, please update the commit messages. Each commit message should
>> include a brief description (one line) , a detailed description (the
>> body), and also the list of the authors, the reviewers and a link to
>> the discussion. Please use [3] as a template.
>>
>> [1]: http://cfbot.cputube.org/
>> [2]: https://github.com/afiskon/pgscripts/
>> [3]:
>> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=784cedda0604ee4ac731fd0b00cd8b27e78c02d3
>>
>> --
>> Best regards,
>> Aleksander Alekseev
>>
>
>
> --
> Regards,
> Nikita Malakhov
> Postgres Professional
> https://postgrespro.ru/
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachment | Content-Type | Size |
---|---|---|
v11-0006-pluggable-toast-docs.patch.gz | application/x-gzip | 4.0 KB |
v11-0004-toaster-snapshot.patch.gz | application/x-gzip | 9.6 KB |
v11-0003-toaster-default.patch.gz | application/x-gzip | 34.3 KB |
v11-0002-toaster-interface.patch.gz | application/x-gzip | 44.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Julien Rouhaud | 2022-08-02 06:17:46 | Re: COPY TO (FREEZE)? |
Previous Message | Japin Li | 2022-08-02 05:59:39 | Re: Question about user/database-level parameters |