MMAP Buffers

On 15.04.2011 13:32, Radosław Smogura wrote:

If I may, I want to share some concept to use mmap in PG. It's far, far
away from perfect, but it's keeps WAL before data. As well I crated
table, with index, inserted few values, and I done vacuum full on this
table. Db inits welcome from orginal sources.

Performance of read (if backend is loaded) is really good, query time
goes down from 450ms to about 410ms. Update may be slower - but work is
in progress (I will start with write, as I went to point when simple
updates may be performed). Even that I didn't covered all aspects off
updating, it's simple to do it, just to call PreopareBufferToUpdate
before modifing buffer, ofc some ideas of increasing this are still in
my head.

Any comments, suggestions welcome.

The patch is quite hard to read because of random whitespace changes and
other stylistic issues, but I have a couple of high-level questions on
the design:

* Does each process have its own mmappings, or are the mmapping done to
shared buffers?

* How do you handle locking? Do you still need to allocate a shared
buffer for each mmapped page?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 15 Apr 2011 14:33:37 +0300, Heikki Linnakangas wrote:

On 15.04.2011 13:32, Radosław Smogura wrote:

If I may, I want to share some concept to use mmap in PG. It's far,
far
away from perfect, but it's keeps WAL before data. As well I crated
table, with index, inserted few values, and I done vacuum full on
this
table. Db inits welcome from orginal sources.

Performance of read (if backend is loaded) is really good, query
time
goes down from 450ms to about 410ms. Update may be slower - but work
is
in progress (I will start with write, as I went to point when simple
updates may be performed). Even that I didn't covered all aspects
off
updating, it's simple to do it, just to call PreopareBufferToUpdate
before modifing buffer, ofc some ideas of increasing this are still
in
my head.

Any comments, suggestions welcome.

The patch is quite hard to read because of random whitespace changes
and other stylistic issues, but I have a couple of high-level
questions on the design:

Yes, but, hmm... in Netbeans I had really long gaps (probably 8 spaces,
from tabs), so deeper "ifs", comments at the and of variables, went of
out my screen. I really wanted to not format this, but sometimes I
needed.

* Does each process have its own mmappings, or are the mmapping done
to shared buffers?

Those are MAP_SHARED mappings, but each process has it's own pointer to
this.

* How do you handle locking?

I do not do locking... I do different thing (worst)... When buffer
should be updated, it gets shared buffer, content is copied (so
situation almost same like fread), and depending on situation content is
used directly or pages between mmaping and shared (updatable) regions
are swapped - it keeps tuple pointers, etc. I really would be happy if
such method (lock flushing to file) could exists.

Do you still need to allocate a shared buffer for each mmapped page?

Currently each mmaped page has additional shared buffer, but it's
almost ready to use independent pool of shared buffers. This will be
good, as mmaped buffers could cover whole system cache, keeping maybe
10%-20% of this size for write in SHMEM.

I think about MAP_PRIVATE, but those has some pluses and minuses, e.g.
MAP_SHARED may be, for less critical systems, simplier equipped with GUC
mmap_direct_write=true.

Regards,
Radek

On 04/15/2011 08:12 AM, Radosław Smogura wrote:

The patch is quite hard to read because of random whitespace changes
and other stylistic issues, but I have a couple of high-level
questions on the design:

Yes, but, hmm... in Netbeans I had really long gaps (probably 8
spaces, from tabs), so deeper "ifs", comments at the and of variables,
went of out my screen. I really wanted to not format this, but
sometimes I needed.

Netbeans is possibly not very well suited to working on postgres code.
AFAIK emacs and/or vi(m) are used by almost all the major developers.

cheers

andrew

Radoslaw,

10% improvement isn't very impressive from a switch to mmap. What workload did you test with? What I'd really like to see is testing with databases which are 50%, 90% and 200% the size of RAM ... that's where I'd expect the greatest gain from limiting copying.

Netbeans is possibly not very well suited to working on postgres code.
AFAIK emacs and/or vi(m) are used by almost all the major developers.

Guys, can we *please* focus on the patch for now, rather than the formatting, which is fixable with sed?
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
San Francisco

Joshua Berkus <josh@agliodbs.com> Friday 15 April 2011 18:55:04

Radoslaw,

10% improvement isn't very impressive from a switch to mmap. What workload
did you test with? What I'd really like to see is testing with databases
which are 50%, 90% and 200% the size of RAM ... that's where I'd expect
the greatest gain from limiting copying.

I think 10% is quite good, as my stand-alone test of mmap vs. read shown that
speed up of copying 100MB data to mem may be from ~20ms to ~100ms (depends on
destination address). Of course deeper, system test simulating real usage will
say more. In any case after good deals with writes, I will speed up reads. I
think to bypass smgr/md much more and to expose shared id's (1,2,3...) for
each file segment.

Going to topic...

In attachment I sent test-scripts which I used to fill data, nothing complex
(left from 2nd level caches).

Query I've used to measure was SELECT count(substr(content, 1, 1)) FROM
testcase1 WHERE multi_id > 50000;

Timings ware taken from psql.

I didn't made load (I have about 2GB of free sapce at /home, and 4GB RAM) and
stress (I'm not quite ready to try concurrent updates of same page - may fail,
notice is and place to fix is in code) tests yet.

Netbeans is possibly not very well suited to working on postgres code.
AFAIK emacs and/or vi(m) are used by almost all the major developers.

Guys, can we *please* focus on the patch for now, rather than the
formatting, which is fixable with sed?

Netbeans is quite good, of course it depends who likes what. Just try 7.0 RC
2.

Regards,
Radek

Joshua Berkus wrote:

Guys, can we *please* focus on the patch for now, rather than the formatting, which is fixable with sed?

Never, and that's not true. Heikki was being nice; I wouldn't have even
slogged through it long enough to ask the questions he did before
kicking it back as unusable. A badly formatted patch makes it
impossible to evaluate whether the changes from a submission are
reasonable or not without the reviewer fixing it first. And you can't
automate correcting it, it takes a lot of tedious manual work. Start
doing a patch review every CommitFest cycle and you very quickly realize
it's not an ignorable problem. And lack of discipline in minimizing
one's diff is always a sign of other code quality issues.

Potential contributors to PostgreSQL should know that a badly formatted
patch faces an automatic rejection, because no reviewer can work with it
easily. This fact is not a mystery; in fact it's documented at
http://wiki.postgresql.org/wiki/Submitting_a_Patch : "The easiest way
to get your patch rejected is to make lots of unrelated changes, like
reformatting lines, correcting comments you felt were poorly worded etc.
Each patch should have the minimum set of changes required to fulfil the
single stated objective." I think I'll go improve that text
next--something like "Ways to get your patch rejected" should be its own
section.

The problem here isn't whether someone used an IDE or not, it's that
this proves they didn't read their own patch before submitting it.
Reading one's own diff and reflecting on what you've changed is one of
the extremely underappreciated practices of good open-source software
development. Minimizing the size of that diff is perhaps the most
important thing someone can do in order to make their changes to a piece
of software better. Not saying something that leads in that direction
would be a disservice to the submitter.

P.S. You know what else I feel should earn an automatic rejection
without any reviewer even looking at the code? Submitting a patch that
claims to improve performance and not attaching the test case you used,
along with detailed notes about before/after tests on your own
hardware. A hand wave "it's faster" is never good enough, and it's
extremely wasteful of our limited reviewer resources to try and
duplicate what the submitter claimed. Going to add something about that
to the submission guidelines too.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

On Apr 16, 2011, at 1:48 AM, Greg Smith <greg@2ndquadrant.com> wrote:

P.S. You know what else I feel should earn an automatic rejection without any reviewer even looking at the code?

Greg is absolutely right. And to the two he listed, let me add another of my own gripes: failing to provide submission notes that explain how the patch works, and how it addresses the conceptually difficult issues raised previously. The OP says that this patch maintains the WAL-before-data rule without any explanation of how it accomplishes that seemingly quite amazing feat. I assume I'm going to have to read this patch at some point to refute this assertion, and I think that sucks. I am pretty nearly 100% confident that this approach is utterly doomed, and I don't want to spend a lot of time on it unless someone can provide me with a compelling explanation of why my confidence is misplaced. But spending a lot of time on it is exactly what I'm going to have to do, because reading a undocumented patch full of spurious garbage to refute a hand-wavy claim of correctness is time-consuming, and if I give up on it without reading it, someone will yell "unfair, unfair!"

None of this is to say that I don't appreciate Radoslaw's interest in contributing, because I very much do. But I also think it's important to realize that we have a finite number of reviewers and they have finite time. Trying to minimize the amount of time that it takes someone to review or commit your patch is a service to the whole community, and we should acknowledge that it has value and appreciate the people who consistently do it.

...Robert

Greg Smith <greg@2ndquadrant.com> writes:

Reading one's own diff and reflecting on what you've changed is one of
the extremely underappreciated practices of good open-source software
development. Minimizing the size of that diff is perhaps the most
important thing someone can do in order to make their changes to a piece
of software better. Not saying something that leads in that direction
would be a disservice to the submitter.

A couple further comments on that thought:

* Another argument for avoiding unnecessary changes is that the larger
your patch's change footprint, the more likely it is to create merge
conflicts for people working on other patches. Now, if they're
necessary changes, that's the price of parallelism in development.
But gratuitous whitespace changes add nothing and they do have costs.

* On the other side of the coin, I have seen many a patch that was
written to minimize the length of the diff to the detriment of
readability or maintainability of the resulting code, and that's *not*
a good tradeoff. Always do what makes the most sense from a long-run
perspective.

I keep wanting to do a talk arguing that everything you need to know
about good patch style can be derived from the mantra "Make the patch
look like the code had always been there". If the functionality had
been designed in on day one, where would it be placed and how would it
be coded? You might be able to make the patch diff shorter with some
shortcut or other, but that's not the way to do it.

regards, tom lane

Tom Lane wrote:

* On the other side of the coin, I have seen many a patch that was
written to minimize the length of the diff to the detriment of
readability or maintainability of the resulting code, and that's *not*
a good tradeoff.

Sure. that's possible. But based on the reviews I've done, I'd say that
the fact someone is even aware that minimizing their diff is something
important to consider automatically puts them far ahead of the average
new submitter. There are a high percentage of patches where the
submitter generates a diff and sents it without even looking at it.
That a person would look at their diff and go too far without trying to
make it small doesn't happen nearly as much.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

Robert Haas wrote:

The OP says that this patch maintains the WAL-before-data rule without any explanation of how it accomplishes that seemingly quite amazing feat. I assume I'm going to have to read this patch at some point to refute this assertion, and I think that sucks.

I don't think you have to read any patch that doesn't follow the
submission guidelines. The fact that you do is a great contribution to
the community. But if I were suggesting how your time would be best
spent improving PostgreSQL, "reviewing patches that don't meet coding
standards" would be at the bottom of the list. There's always something
better for the project you could be working on instead.

I just added
http://wiki.postgresql.org/wiki/Submitting_a_Patch#Reasons_your_patch_might_be_returned
, recycling some existing text, adding some new suggestions.

I hope I got the tone of that text right. The intention was to have a
polite but clear place to point submitters to when their suggestion
doesn't meet the normal standards here, such that they might even get
bounced before even entering normal CommitFest review. This MMAP patch
looks like it has all 5 of the problems mentioned on that now more
focused list.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

On Sat, Apr 16, 2011 at 7:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:

The OP says that this patch maintains the WAL-before-data rule without any explanation of how it accomplishes that seemingly quite amazing feat.  I assume I'm going to have to read this patch at some point to refute this assertion, and I think that sucks. I am pretty nearly 100% confident that this approach is utterly doomed, and I don't want to spend a lot of time on it unless someone can provide me with a compelling explanation of why my confidence is misplaced.

Fwiw he did explain how he did that. Or at least I think he did --
it's possible I read what I expected because what he came up with is
something I've recently been thinking about.

What he did, I gather, is treat the mmapped buffers as a read-only
copy of the data. To actually make any modifications he copies it into
shared buffers and treats them like normal. When the buffers get
flushed from memory they get written and then the pointers get
repointed back at the mmapped copy. Effectively this means the shared
buffers get extended to include all of the filesystem cache instead of
having to evict buffers from shared buffers just because you want to
read another one that's already in filesystem cache.

It doesn't save the copying between filesystem cache and shared
buffers for buffers that are actually being written to. But it does
save some amount of other copies on read-only traffic and it can even
save some i/o. It does require a function call before each buffer
modification where the pattern is currently <lock buffer>, <mutate
buffer>, <mark buffer dirty>. From what he describes he needs to add a
<prepare buffer for mutation> between the lock and mutate.

I think it's an interesting experiment and it's good to know how to
solve some of the subproblems. Notably, how do you extend files or
drop them atomically across processes? And how do you deal with
getting the mappings to be the same across all the processes or deal
with them being different? But I don't think it's a great long-term
direction. It just seems clunky to have to copy things from mmapped
buffers to local buffers and back. Perhaps the performance testing
will show that clunkiness is well worth it but we'll need to see that
for a wide variety of workloads to judge that.

--
greg

Greg Stark <gsstark@mit.edu> Saturday 16 April 2011 13:00:19

On Sat, Apr 16, 2011 at 7:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:

The OP says that this patch maintains the WAL-before-data rule without
any explanation of how it accomplishes that seemingly quite amazing
feat. I assume I'm going to have to read this patch at some point to
refute this assertion, and I think that sucks. I am pretty nearly 100%
confident that this approach is utterly doomed, and I don't want to
spend a lot of time on it unless someone can provide me with a
compelling explanation of why my confidence is misplaced.

Fwiw he did explain how he did that. Or at least I think he did --
it's possible I read what I expected because what he came up with is
something I've recently been thinking about.

What he did, I gather, is treat the mmapped buffers as a read-only
copy of the data. To actually make any modifications he copies it into
shared buffers and treats them like normal. When the buffers get
flushed from memory they get written and then the pointers get
repointed back at the mmapped copy. Effectively this means the shared
buffers get extended to include all of the filesystem cache instead of
having to evict buffers from shared buffers just because you want to
read another one that's already in filesystem cache.

It doesn't save the copying between filesystem cache and shared
buffers for buffers that are actually being written to. But it does
save some amount of other copies on read-only traffic and it can even
save some i/o. It does require a function call before each buffer
modification where the pattern is currently <lock buffer>, <mutate
buffer>, <mark buffer dirty>. From what he describes he needs to add a
<prepare buffer for mutation> between the lock and mutate.

I think it's an interesting experiment and it's good to know how to
solve some of the subproblems. Notably, how do you extend files or
drop them atomically across processes? And how do you deal with
getting the mappings to be the same across all the processes or deal
with them being different? But I don't think it's a great long-term
direction. It just seems clunky to have to copy things from mmapped
buffers to local buffers and back. Perhaps the performance testing
will show that clunkiness is well worth it but we'll need to see that
for a wide variety of workloads to judge that.

In short words, I swap, exchange (clash of terms) VM pages to prevent pointers
(only if needed). I tried to directly point to new memory area, but I saw that
some parts of code really depends on memory pointed by original pointers, e.g.
Vaccumm uses hint bits setted by previous scan (it depends on this if bit is
set or not! so for it it's not only hint). Just from this case I can't assume
there is no more such places, so VM pages swap does it for me.

Stand alone tests shows for me that this process (with copy from mmap) is
2x-3x time longer then previous. But until someone will not update whole
table, then benefit will be taken from pre-update scan, index scans, larger
availability of memory (you don't eat cache memory to keep copy of cache in
ShMem). Everything may be slower when database fits in ShMem, and similarly
(2nd level bufferes may increase performance slightly).

I reserve memory for whole segment even if file is smaller. Extending is by
wirte one byte at the end of block (here may come deal with Unfiorm Buffer
Caches, if I remember name well). For current processors, and current
implementation database size is limited to about 260TB (no dynamic segment
reservation is performed).

Truncation not implemented.

Each buffer descriptor has tagVersion to simple check if buffer tag has
changed. Descriptors (partially) are mirrored in local memory, and versions
are checked. Currently each re-read (is pointed to smgr/md), but introduce
shared segment id, and assuming each segment has constant maximum number of
blocks, will make it faster (this will be something like current buffer tag),
even version field will be unneeded.

I saw problems with vacuum, as it reopens relation and I got mappings of same
file twice (minor problem). Important will be about deletion, when pointers
must invalidated in "good way".

Regards,
Radek.

On Sat, Apr 16, 2011 at 8:48 AM, Greg Smith <greg@2ndquadrant.com> wrote:

Joshua Berkus wrote:

Guys, can we *please* focus on the patch for now, rather than the
formatting, which is fixable with sed?

Never, and that's not true.  Heikki was being nice; I wouldn't have even
slogged through it long enough to ask the questions he did before kicking it
back as unusable.  A badly formatted patch makes it impossible to evaluate
whether the changes from a submission are reasonable or not without the
reviewer fixing it first.  And you can't automate correcting it, it takes a
lot of tedious manual work.  Start doing a patch review every CommitFest
cycle and you very quickly realize it's not an ignorable problem.  And lack
of discipline in minimizing one's diff is always a sign of other code
quality issues.

Potential contributors to PostgreSQL should know that a badly formatted
patch faces an automatic rejection, because no reviewer can work with it
easily.  This fact is not a mystery; in fact it's documented at
http://wiki.postgresql.org/wiki/Submitting_a_Patch :  "The easiest way to
get your patch rejected is to make lots of unrelated changes, like
reformatting lines, correcting comments you felt were poorly worded etc.
Each patch should have the minimum set of changes required to fulfil the
single stated objective."  I think I'll go improve that text next--something
like "Ways to get your patch rejected" should be its own section.

The problem here isn't whether someone used an IDE or not, it's that this
proves they didn't read their own patch before submitting it.  Reading one's
own diff and reflecting on what you've changed is one of the extremely
underappreciated practices of good open-source software development.
 Minimizing the size of that diff is perhaps the most important thing
someone can do in order to make their changes to a piece of software better.
 Not saying something that leads in that direction would be a disservice to
the submitter.

P.S. You know what else I feel should earn an automatic rejection without
any reviewer even looking at the code?  Submitting a patch that claims to
improve performance and not attaching the test case you used, along with
detailed notes about before/after tests on your own hardware.  A hand wave
"it's faster" is never good enough, and it's extremely wasteful of our
limited reviewer resources to try and duplicate what the submitter claimed.
 Going to add something about that to the submission guidelines too.

Give the OP a break - he was not "re-styling", he was clearly trying to make
crappily indented Postgres code readable in his editor. Reading the patch
would not matter because the original code would still be crappily indented.

Yes, such patch is bad, but what should the proper response be in such
situation?

Hint: it can be both polite and short.

--
marko

Greg Stark <gsstark@mit.edu> writes:

What he did, I gather, is treat the mmapped buffers as a read-only
copy of the data. To actually make any modifications he copies it into
shared buffers and treats them like normal. When the buffers get
flushed from memory they get written and then the pointers get
repointed back at the mmapped copy.

That seems much too late --- won't other processes still be looking at
the stale mmap'ed version of the page until a write-out happens?

I'm pretty concerned about the memory efficiency of this too, since it
seems like it's making it *guaranteed*, not just somewhat probable,
that there are two copies in RAM of every database page that's been
modified since the last checkpoint (or so).

regards, tom lane

On Fri, 2011-04-15 at 12:32 +0200, Radosław Smogura wrote:

I didn't included this, as diff, because of ~150kb size (mainly
configure scripts, which are included in SVC). Due to this, You may
download it from
http://softperience.eu/downloads/pg_mmap_20110415.diff.bz2 (Legal:
Work
under PostgreSQL BSD Lincense). Patch is just GIT diff, later I will
try
to grab some git.

Btw., about 87% of this patch are diffs against configure and
pg_config.h.in, which are useless. If you strip those out, your patch
will be small enough to submit inline.

All,

Never, and that's not true. Heikki was being nice; I wouldn't have
even
slogged through it long enough to ask the questions he did before
kicking it back as unusable. A badly formatted patch makes it
impossible to evaluate whether the changes from a submission are
reasonable or not without the reviewer fixing it first.

Then you can say that politely and firmly with direct reference to the problem, rather than making the submitter feel bad.

"Thank you for taking on testing an idea we've talked about on this list for a long time and not had the energy to test. However, I'm having a hard time evaluating your patch for a few reasons ...(give reasons). Would it be possible for you to resolve these and resubmit so that I can give the patch a good evaluation?"

... and once *one* person on this list has made such a comment, there is no need for two other hackers to pile on the reformat-your-patch bandwagon.

Our project has an earned reputation for being rejection-happy curmudgeons. This is something I heard more than once at MySQLConf, including from one student who chose to work on Drizzle instead of PostgreSQL for that reason. I think that we could stand to go out of our way to be helpful to first-time submitters.

That doesn't mean that we have to accept patches mangled by using an IDE designed for Java, and which lack test cases. However, we can be nice about it.

--
Josh Berkus
Niceness Nazi

Radoslaw,

I think 10% is quite good, as my stand-alone test of mmap vs. read
shown that
speed up of copying 100MB data to mem may be from ~20ms to ~100ms
(depends on
destination address). Of course deeper, system test simulating real
usage will
say more. In any case after good deals with writes, I will speed up
reads. I
think to bypass smgr/md much more and to expose shared id's (1,2,3...)
for
each file segment.

Well, given the risks to durability and stability associated with using MMAP, I doubt anyone would even consider it for a 10% throughput improvement. However, I don't think the test you used demonstrates the best case for MMAP as a performance improvement.

In attachment I sent test-scripts which I used to fill data, nothing
complex
(left from 2nd level caches).

Query I've used to measure was SELECT count(substr(content, 1, 1))
FROM
testcase1 WHERE multi_id > 50000;

Timings ware taken from psql.

I didn't made load (I have about 2GB of free sapce at /home, and 4GB
RAM) and
stress (I'm not quite ready to try concurrent updates of same page -
may fail,
notice is and place to fix is in code) tests yet.

Yes, but this test case doesn't offer much advantage to MMAP. Where I expect it would shine would be cases where the database is almost as big as, or much bigger than RAM ... where the extra data copying by current code is both frequent and wastes buffer space we need to use. As well as concurrent reads from the same rows.

You can write a relatively simple custom script using pgBench to test this; you don't need a big complicated benchmark. Once we get over the patch cleanup issues, I might be able to help with this.

Netbeans is quite good, of course it depends who likes what. Just try
7.0 RC
2.

I don't know if you've followed the formatting discussion, but apparently there's an issue with Netbeans re-indenting lines you didn't even edit. It makes your patch hard to read or apply. I expect that Netbeans has some method to reconfigure indenting, etc.; do you think you could configure it to PostgresQL standards so that this doesn't get in the way of evaluation of your ideas?

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

Joshua Berkus wrote:

Then you can say that politely and firmly with direct reference to the problem, rather than making the submitter feel bad.

That's exactly what happened. And then you responded that it was
possible to use a patch without fixing the formatting first. That's not
true, and those of us who do patch review are tired of even trying.

Our project has an earned reputation for being rejection-happy curmudgeons. This is something I heard more than once at MySQLConf, including from one student who chose to work on Drizzle instead of PostgreSQL for that reason. I think that we could stand to go out of our way to be helpful to first-time submitters.

I'll trade you anecdotes by pointing out that I heard from half a dozen
business people that the heavy emphasis on quality control and standards
was the reason they were looking into leaving MySQL derived
distributions for PostgreSQL.

I've spent days of time working on documentation to help new submitters
get their patches improve to where they meet this community's
standards. This thread just inspired another round of that. What
doesn't help is ever telling someone they can ignore those and still do
something useful we're interested in.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

On Fri, Apr 15, 2011 at 3:12 PM, Radosław Smogura
<rsmogura@softperience.eu> wrote:

On Fri, 15 Apr 2011 14:33:37 +0300, Heikki Linnakangas wrote:

The patch is quite hard to read because of random whitespace changes
and other stylistic issues, but I have a couple of high-level
questions on the design:

Yes, but, hmm... in Netbeans I had really long gaps (probably 8 spaces, from
tabs), so deeper "ifs", comments at the and of variables, went of out my
screen. I really wanted to not format this, but sometimes I needed.

Seems no one else has mentioned it yet -- Postgres uses non-standard
tab-width of 4, instead of 8. If you see ugly code in Postgres, thats why...

--
marko