Thread: Commitfest patches mostly assigned ... status
Hackers, At this point, almost all patches have been assigned to reviewers. If you submitted a patch and don't get feedback by Saturday, take a look at who's in the "reviewers" column and send them a query. Since I have no way to track when patches are assigned to reviewers, I have no idea if some of them are sitting on their hands. Some patches have not been assigned to reviewers for various reasons. The following weren't assigned because they are complex and really need a high-end hacker or committer to take them on: libpq events rmgr hooks and contrib/rmgr_hook CLUSTER using sort instead of index scan The following were not assigned because there has already been discussion on this list debating them being a good idea. These need consensus on this list before they get assigned to a reviewer: remove --inputdir and --outputdir from pg_regress GUC: Case-insensitive units Allow has_table_privilege(...,'usage') on sequences I've assigned some reviewers to "WIP" patches with instructions to report back on their general experience with building, functionality and spec. Note that patches need not have only one reviewer! If you have time, please take on testing some of the more complex patches, especially: Column-level Permissions Common Table Expressions SE-PostgreSQL patches Also, note that the following patches need performance testing on a variety of platforms. Everyone should help with this. GSoC Improved Hash Indexing posix fadvises operator restrictivity function for text search CLUSTER using sort instead of index scan Thanks for you input, and let's close this commitfest in a week! --CommitFest "Mom"
Josh Berkus <josh@agliodbs.com> writes:
> Also, note that the following patches need performance testing on a
> variety of platforms. Everyone should help with this.
> GSoC Improved Hash Indexing
> posix fadvises
> operator restrictivity function for text search
> CLUSTER using sort instead of index scan
The last two of those are not code-complete so I'm not sure that
performance testing is all that meaningful for them. But by all
means, test the first two ...
regards, tom lane
On Thu, Sep 11, 2008 at 1:53 AM, Josh Berkus <josh@agliodbs.com> wrote: > Some patches have not been assigned to reviewers for various reasons. The > following weren't assigned because they are complex and really need a > high-end hacker or committer to take them on: > > libpq events Alvaro actually performed a review on libpq events. We are waiting on his feedback on our changes (based on his review) and the newly submitted documentation. I'll update the wiki accordingly. I wasn't sure if Alvaro was claiming reviewer status or not. It may be ready to go in as-is unless we draw any new objections. Anybody who wants to look should ping Alvaro and/or Tom. merlin
Merlin Moncure escribió: > On Thu, Sep 11, 2008 at 1:53 AM, Josh Berkus <josh@agliodbs.com> wrote: > > Some patches have not been assigned to reviewers for various reasons. The > > following weren't assigned because they are complex and really need a > > high-end hacker or committer to take them on: > > > > libpq events > > Alvaro actually performed a review on libpq events. We are waiting on > his feedback on our changes (based on his review) and the newly > submitted documentation. I'll update the wiki accordingly. I wasn't > sure if Alvaro was claiming reviewer status or not. It may be ready > to go in as-is unless we draw any new objections. The patch looks OK to me as it was the last time I looked at it; maybe Tom has more comments, so I decided against just committing it. I admit I haven't checked the docs. Actually a minor gripe ... should PQsetvalue be PQsetValue? :-) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > > Actually a minor gripe ... should PQsetvalue be PQsetValue? :-) > We were mimicing PQgetvalue, which uses a lowercase 'v'. Is there a preference, none on this end. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Merlin Moncure escribi�:
>> Alvaro actually performed a review on libpq events. We are waiting on
>> his feedback on our changes (based on his review) and the newly
>> submitted documentation. I'll update the wiki accordingly. I wasn't
>> sure if Alvaro was claiming reviewer status or not. It may be ready
>> to go in as-is unless we draw any new objections.
> The patch looks OK to me as it was the last time I looked at it; maybe
> Tom has more comments, so I decided against just committing it.
I haven't got round to looking at it in this fest. If anyone else wants
to look it over, feel free. I think Josh is overrating the patch's
review difficulty --- anyone who uses libpq a lot could review it,
IMHO. You certainly wouldn't need any backend-internals knowledge.
regards, tom lane
On Thu, Sep 11, 2008 at 5:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The patch looks OK to me as it was the last time I looked at it; maybe >> Tom has more comments, so I decided against just committing it. > > I haven't got round to looking at it in this fest. If anyone else wants > to look it over, feel free. I think Josh is overrating the patch's > review difficulty --- anyone who uses libpq a lot could review it, > IMHO. You certainly wouldn't need any backend-internals knowledge. > Josh is probably basing that on the long history of discussion/revision cycles. There is very little change from the design that was hammered out except for the late breaking tweak of how the passthru pointer works. I think the subtext here is that we are waiting on you to sign off on the patch (or not). merlin
> Josh is probably basing that on the long history of > discussion/revision cycles. Yep, and that *I* don't understand what the patch does, so I'm not going to turn a newbie reviewer loose on it. -- --Josh Josh Berkus PostgreSQL San Francisco
Josh Berkus wrote: >> Josh is probably basing that on the long history of >> discussion/revision cycles. > > Yep, and that *I* don't understand what the patch does, so I'm not going to > turn a newbie reviewer loose on it. > Here is a quick overview, there are two parts to the patch: 1. event system This allows one to register "PQregisterEventProc" a per-conn callback function with libpq that gets called when particular events occur. Currently, the events tell you when a conn or result is created, reset, destroyed and copied. It is generic enough to add more events in the future. By receiving events about libpq objects, you can properly allocate and free userland memory (PQfinish, PQexec, PQclear, etc... trigger events) and associate it with the libpq object: thus PQsetInstanceData(conn...) or PQsetResultInstanceData(res....). This is basically adding members to the opaque PGconn or PGresult during runtime. This "instance data" can be retreived via PQinstanceData and PQresultInstanceData. To shine a different light on it, apps normally wrap a PGconn or PGresult within their own structures, but now you can wrap the app structures inside a PGconn or PGresult. This may help, its the libpqtypes PGEventProc implementation. http://libpqtypes.esilo.com/browse_source.html?file=events.c Also check out the patches sgml docs if you get a chance. 2. result management There are also four functions that provide more control over PGresult. If you need to create a result from scratch, expands on the PQmakeEmptyPGResult idea. PQcopyResult - copy a given source result, flags argument determines what portions of the result are copied. PQsetResultAttrs - sets the attributes of the reuslt (its columns). PQsetvalue - sets a tuple value in a result PQresultAlloc - thin wrapper around the internal pqResultAlloc. Uses the result's block allocater, which allows PQclear to properly free all memory assocaited with a PGresult. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/
Josh Berkus wrote: > Hackers, > > At this point, almost all patches have been assigned to reviewers. If > you submitted a patch and don't get feedback by Saturday, take a look at > who's in the "reviewers" column and send them a query. Since I have no > way to track when patches are assigned to reviewers, I have no idea if > some of them are sitting on their hands. : > Note that patches need not have only one reviewer! If you have time, > please take on testing some of the more complex patches, especially: > Column-level Permissions > Common Table Expressions > SE-PostgreSQL patches I updated my patches: http://archives.postgresql.org/pgsql-hackers/2008-09/msg00859.php It contains rebasing to the latest CVS HEAD, a new hook to avoid bypass access controls, and a small fix. In addition, I tried to attach several short explanations about key points to understand the big patch. I wrote the documentation of SE-PostgreSQL, but it is not a description for *the patch*. If reviewers have any unclear things, please feel free to ask me. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>