Re: Pluggable toaster - Mailing list pgsql-hackers
| From | Nikita Malakhov |
|---|---|
| Subject | Re: Pluggable toaster |
| Date | |
| Msg-id | CAN-LCVP94miKg1j5GF+WVhyG4zDPF4ruG8ZnCR-4cBpG=rhbSQ@mail.gmail.com Whole thread Raw |
| In response to | Re: Pluggable toaster (Robert Haas <robertmhaas@gmail.com>) |
| Responses |
Re: Pluggable toaster
(Robert Haas <robertmhaas@gmail.com>)
|
| List | pgsql-hackers |
Hi,
Thank you very much for your review, I'd like to get it much earlier. I'm currently working on cleaning up code, and would try to comment as much as possible.
This patch set is really a large set of functionality, it was divided into 4 logically complete
parts, but anyway these parts contain a lot of changes themselves.
- Yes, you're right, new syntax is added. I'm also working on the documentation part for it;
- Patches actually consist of a lot of minor commits. As I see we have to squash them
to provide parts as 2-3 main commits without any unnecessary garbage;
- Is 'git apply' not a valid way to apply such patches?
We'll try to straighten these issues out asap.
Thank you!
On Mon, Apr 4, 2022 at 7:40 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 4, 2022 at 12:13 PM Nikita Malakhov <hukutoc@gmail.com> wrote:
> I'm really sorry. Messed up some code while merging rebased branches with previous (v1)
> patches issued in December and haven't noticed that seg fault because of corrupted code
> while running check-world.
> I've fixed messed code in Dummy Toaster package and checked twice - all's correct now,
> patches are applied correctly and tests are clean.
> Thank you for pointing out the error and for your patience!
Hi,
This patch set doesn't seem anywhere close to committable to me. For example:
- It apparently adds a new command called CREATE TOASTER but that
command doesn't seem to be documented anywhere.
- contrib/dummy_toaster is added in patch 1 with a no implementation
and code comments that say "Bloom index utilities" and then those
comments are fixed and an implementation is added in later patches.
- What is supposedly patch 1 is actually 32 patch files concatenated
together. It doesn't apply properly either with 'patch' or 'git am' so
I don't understand what we would even do with this.
- None of these patches have appropriate commit messages.
- Many of these patches add 'XXX' comments or errors or other
obviously unfinished bits of code. Some of this may be cleaned up by
later patches but it's hard to tell because right now one can't even
apply the patch set properly. Even if that were possible, it's the job
of the person submitting the patch to organize the patch into
independent, separately committable chunks that are self-contained and
have good comments and good commit messages for each one.
I don't think based on the status of this patch set that it's even
possible to provide useful feedback on the design at this point, never
mind getting anything committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
pgsql-hackers by date: