The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Here’s review of Background Sessions v2 patch.
Patch provides API for controlling other backend. Also, patch contains Python 
API support for calling C API.

===Overall status===
Patch applies to current HEAD clearly.
Contains new test and passes existing tests.
Contains sufficient documentation.
Contains 2 TODO items. Not sure it's OK to it leave so.
Another patch from this commitfest (pg_background) is based on this patch.

I haven’t found a way to safely acquire status of session (without possibility 
of ereport(ERROR)).
I do not see how to pass massive data, except by one long char* SQL. All the 
values have to be formatted as text.
BackgroundSessionStart() result do not contain PID. This functionality is 
expected by pg_background (though, can be added separately by pg_background). I 
suppose, this is done to prevent API users from accessing internals of 
BackgroundSession structure. But some information have to be public, anyway.
bgsession.c code contains very little comments.
I do not think that switch inside a switch (see bgsession_worker_main()) is 
easy to follow.

There’s always something to improve, but I think that this patch is ready for 

PS. I’ve read the db_link patches, but this review does not apply to them. I 
suppose db_link refactoring would be useful and functionality is added, so I 
think these patches deserve separate commitfest entry.

Best regards, Andrey Borodin.

The new status of this patch is: Ready for Committer

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to