Re: Pluggable toaster

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(at)gmail(dot)com> wrote:

> On Mon, Apr 4, 2022 at 12:13 PM Nikita Malakhov <hukutoc(at)gmail(dot)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
>

--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/