Thread: You could be a PostgreSQL Patch Reviewer!

PostgreSQL community members:

Do you love the PostgreSQL project?  Do you want to contribute to it?
Do you want to help produce the next version of PostgreSQL? (9.2)

Well, you *can*.  You can be a patch reviewer -- one of the single most
valuable things you can contribute to the project.

You do not have to be a C coder to be a patch reviewer.  Pretty much all
you need to know is:
- how to checkout PostgreSQL from Git
- how to build PostgreSQL from source
- how to apply a patch

If you know those three things, you can help with patch review.  Of
course, if you do know C, you can be even more help ... and learn the
PostgreSQL source in the process.

We especially need folks who are able to build PostgreSQL on Windows, as
we have several Windows-specific patches and no reviewers for them.

The First CommitFest for version 9.2 is underway *right now* and we need
your help. Help PostgreSQL ... review a patch!

http://wiki.postgresql.org/wiki/Reviewing_a_Patch

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Jun 15, 2011 at 3:49 PM, Josh Berkus <josh@agliodbs.com> wrote:
> PostgreSQL community members:
>
> Do you love the PostgreSQL project?  Do you want to contribute to it?
> Do you want to help produce the next version of PostgreSQL? (9.2)
>
> Well, you *can*.  You can be a patch reviewer -- one of the single most
> valuable things you can contribute to the project.
>
> You do not have to be a C coder to be a patch reviewer.  Pretty much all
> you need to know is:
> - how to checkout PostgreSQL from Git
> - how to build PostgreSQL from source
> - how to apply a patch
>
> If you know those three things, you can help with patch review.  Of
> course, if you do know C, you can be even more help ... and learn the
> PostgreSQL source in the process.
>
> We especially need folks who are able to build PostgreSQL on Windows, as
> we have several Windows-specific patches and no reviewers for them.
>
> The First CommitFest for version 9.2 is underway *right now* and we need
> your help. Help PostgreSQL ... review a patch!
>
> http://wiki.postgresql.org/wiki/Reviewing_a_Patch

This sounds suspiciously like you are trying to get me to join the army...

merlin

On 16/06/11 04:49, Josh Berkus wrote:

> We especially need folks who are able to build PostgreSQL on Windows, as
> we have several Windows-specific patches and no reviewers for them.

Urrrrrrrggh. Does that mean I have to volunteer myself? ;-)

I kind of regret learning to develop on Windows, because I find it a
horrid environment*, but so does everybody else, most of whom have been
clever enough not to learn it. They can say "I don't know Windows" and
they're safe.

Any chance of flagging patches on the commitfest when they're platform
specific? I'm hurting for time but will check out Windows-specific stuff
if it's an area the project particularly needs help with.

* Except for Visual C++'s debugger, which is a godsend, and so good it's
almost worth learning to code on Windows to use. I will never use gdb
for c++ again given the choice. gdb is a C debugger with bizarre
delusions that it's also a C++ debugger. Most of my C++ code is
cross-platform primarily so I can code on Linux, interactively debug on
Windows, and use Linux for valgrind memory debugging.

--
Craig Ringer

Craig Ringer <craig@postnewspapers.com.au> writes:
> Any chance of flagging patches on the commitfest when they're platform
> specific? I'm hurting for time but will check out Windows-specific stuff
> if it's an area the project particularly needs help with.

Well, a quick look through
https://commitfest.postgresql.org/action/commitfest_view/inprogress
suggests that these could use attention:

Latch implementation: wake on postmaster death, reduce archiver wakeups
    Has a Windows-specific implementation, which needs testing

POSIX shared memory
    Does this work on Windows?

Allow multiple Postgres clusters running on the same machine to distinguish themselves in the event log
    Windows-only feature, I assume

Add Support for building with Visual Studio 2010
    Needs testing not only with VS2010, but older versions

libpq SSL with non-blocking sockets (WIP)
    Might need testing on Windows, not sure


            regards, tom lane

Did anyone ever fix the annoying thing where uuid_generate_v4()
doesn't work on Windows 64bit?

On Wed, Jun 15, 2011 at 9:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Craig Ringer <craig@postnewspapers.com.au> writes:
>> Any chance of flagging patches on the commitfest when they're platform
>> specific? I'm hurting for time but will check out Windows-specific stuff
>> if it's an area the project particularly needs help with.
>
> Well, a quick look through
> https://commitfest.postgresql.org/action/commitfest_view/inprogress
> suggests that these could use attention:
>
> Latch implementation: wake on postmaster death, reduce archiver wakeups
>        Has a Windows-specific implementation, which needs testing
>
> POSIX shared memory
>        Does this work on Windows?
>
> Allow multiple Postgres clusters running on the same machine to distinguish themselves in the event log
>        Windows-only feature, I assume
>
> Add Support for building with Visual Studio 2010
>        Needs testing not only with VS2010, but older versions
>
> libpq SSL with non-blocking sockets (WIP)
>        Might need testing on Windows, not sure
>
>
>                        regards, tom lane
>
> --
> Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-general
>

Mike Christensen <mike@kitchenpc.com> writes:
> Did anyone ever fix the annoying thing where uuid_generate_v4()
> doesn't work on Windows 64bit?

AFAIR that was an issue with the uuid-ossp library, so it would be a
matter for that upstream group to deal with.

            regards, tom lane

Wouldn't it be faster/better/easier if Postgres just had its own built
in UUID generator?  Last I tested generating a bunch of UUIDs, it was
quite slow (well compared to MS SQL anyway)..

On Wed, Jun 15, 2011 at 10:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mike Christensen <mike@kitchenpc.com> writes:
>> Did anyone ever fix the annoying thing where uuid_generate_v4()
>> doesn't work on Windows 64bit?
>
> AFAIR that was an issue with the uuid-ossp library, so it would be a
> matter for that upstream group to deal with.
>
>                        regards, tom lane
>

Mike Christensen <mike@kitchenpc.com> writes:
> Wouldn't it be faster/better/easier if Postgres just had its own built
> in UUID generator?

Why would it be?  If you think you can easily improve on uuid-ossp,
you should go help them.

            regards, tom lane

On Wed, Jun 15, 2011 at 10:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Mike Christensen <mike@kitchenpc.com> writes:
>> Wouldn't it be faster/better/easier if Postgres just had its own built
>> in UUID generator?
>
> Why would it be?  If you think you can easily improve on uuid-ossp,
> you should go help them.

I have no knowledge on the subject one way or the other, that's why
I'm asking..  If the answer is "no, there would be no benefit" then
that's the answer..  However, it seems like generating UUIDs in Win64
has been broken, perhaps, forever which is why I question if there's a
better library to use on the Windows platform (or having a single UUID
generator built in for all platforms)..

I'd love to help out with Postgres (and several other open source
projects), perhaps some day though I'm more of a web/.NET guy..

Mike

On 06/15/2011 04:49 PM, Josh Berkus wrote:
> You do not have to be a C coder to be a patch reviewer.  Pretty much all
> you need to know is:
> - how to checkout PostgreSQL from Git
> - how to build PostgreSQL from source
> - how to apply a patch
>

And you don't even really need to know these things well.  If you're on
a UNIX system that has git installed, here's a complete example of how
to build a custom PostgreSQL that includes a patch for review:

cd
git clone git://github.com/gregs1104/peg.git
export PATH="$HOME/peg:$PATH"
mkdir pgwork
peg init test
cd pgwork/src/test
patch -p1 < ~/mytest.patch
. peg build
psql

Just substitute the name of the patch you're using in the "patch" step
here, and if it applies correctly (one of the first things patch review
intends to test for) you're done.  This does a local installation of
PostgreSQL into a tree under your home directory, with reasonable
defaults for all of the parts related to downloading the source code and
compiling it.  See the documentation for the peg utility at
https://github.com/gregs1104/peg for more examples

--
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books

Josh Berkus schrieb:

> You do not have to be a C coder to be a patch reviewer.  Pretty much all
> you need to know is:
> - how to checkout PostgreSQL from Git
> - how to build PostgreSQL from source
> - how to apply a patch
>
> If you know those three things, you can help with patch review.  Of
> course, if you do know C, you can be even more help ... and learn the
> PostgreSQL source in the process.

I can build PostgreSQL on different FreeBSD-Versions (7, 8.1, 8.2).

Is this in any kind related to the PG-Buildfarm? It sounds like this
could be automated very easily.

> We especially need folks who are able to build PostgreSQL on Windows, as
> we have several Windows-specific patches and no reviewers for them.

If there is an rough descriptions, this should be also possible.

Greetings from Germany,
Torsten
--
http://www.dddbl.de - ein Datenbank-Layer, der die Arbeit mit 8
verschiedenen Datenbanksystemen abstrahiert,
Queries von Applikationen trennt und automatisch die Query-Ergebnisse
auswerten kann.

Craig,

> Well, a quick look through
> https://commitfest.postgresql.org/action/commitfest_view/inprogress
> suggests that these could use attention:

Let me know which one of these you want to take first.  I'll put your name by it.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
San Francisco

> I have no knowledge on the subject one way or the other, that's why
> I'm asking.. If the answer is "no, there would be no benefit" then
> that's the answer.. However, it seems like generating UUIDs in Win64
> has been broken, perhaps, forever which is why I question if there's a
> better library to use on the Windows platform (or having a single UUID
> generator built in for all platforms)..

We don't know of a better UUID generator, and barring evidence to the contrary, it seems much more efficient to
contributeto the uuid-ossp project than to invent our own.  As far as I can tell, the UUID extension is designed to
supportconfigurable UUID libraries, so if you could find one which worked well for Windows, it would probably be fairly
easyto extend UUID to support it. 

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
San Francisco

In response to Joshua Berkus <josh@agliodbs.com>:

>
> > I have no knowledge on the subject one way or the other, that's why
> > I'm asking.. If the answer is "no, there would be no benefit" then
> > that's the answer.. However, it seems like generating UUIDs in Win64
> > has been broken, perhaps, forever which is why I question if there's a
> > better library to use on the Windows platform (or having a single UUID
> > generator built in for all platforms)..
>
> We don't know of a better UUID generator, and barring evidence to the contrary, it seems much more efficient to
contributeto the uuid-ossp project than to invent our own.  As far as I can tell, the UUID extension is designed to
supportconfigurable UUID libraries, so if you could find one which worked well for Windows, it would probably be fairly
easyto extend UUID to support it. 

The Windows system API has functions built into the OS for generating
UUIDs, for example:
http://msdn.microsoft.com/en-us/library/aa379322

I have almost no experience using this, but it might be worth investigating
if PostgreSQL's code is abstract enough to use different implementations
and the ossp library is not reliable on Windows.

--
Bill Moran
http://www.potentialtech.com
http://people.collaborativefusion.com/~wmoran/

Radoslaw,

> https://commitfest.postgresql.org/action/patch_view?id=565
> https://commitfest.postgresql.org/action/patch_view?id=579

Actually, I assigned you to 580 instead of 579, because supposedly Noah
is reviewing 579.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Craig Ringer <craig@postnewspapers.com.au> Thursday 16 of June 2011 06:14:13
> On 16/06/11 04:49, Josh Berkus wrote:
> > We especially need folks who are able to build PostgreSQL on Windows, as
> > we have several Windows-specific patches and no reviewers for them.
>
> Urrrrrrrggh. Does that mean I have to volunteer myself? ;-)
>
> I kind of regret learning to develop on Windows, because I find it a
> horrid environment*, but so does everybody else, most of whom have been
> clever enough not to learn it. They can say "I don't know Windows" and
> they're safe.
>
> Any chance of flagging patches on the commitfest when they're platform
> specific? I'm hurting for time but will check out Windows-specific stuff
> if it's an area the project particularly needs help with.
>
> * Except for Visual C++'s debugger, which is a godsend, and so good it's
> almost worth learning to code on Windows to use. I will never use gdb
> for c++ again given the choice. gdb is a C debugger with bizarre
> delusions that it's also a C++ debugger. Most of my C++ code is
> cross-platform primarily so I can code on Linux, interactively debug on
> Windows, and use Linux for valgrind memory debugging.
>
> --
> Craig Ringer

I may try to review this patches about XML & XPATH (as I know(n) something
about XML):
https://commitfest.postgresql.org/action/patch_view?id=565
https://commitfest.postgresql.org/action/patch_view?id=579

Regards,
Radek