Contribute to PostgreSQL: Examples of Real Patches, Part 1 of N

Earlier in the article Becoming a Contributor in PostgreSQL, the PostgreSQL development process and the tools used in this article were discussed in detail, some ideas for the first patch were proposed, and it was told where and how these patches should be sent. Links were also provided to additional sources of information regarding the internal structure of the RDBMS.

Now we will look at examples of real patches adopted by PostgreSQL in recent times. Some of these patches were written directly by me, while developing others, I actively participated as a reviewer. These are relatively small patches. At the time of this writing, I’ve been developing PostgreSQL for less than a year, and I haven’t done DBMS development before (just like C language development for money). Therefore, there is reason to believe that these patches will be of interest to beginners who want to start participating in the development of open source projects, moreover, it is not necessarily PostgreSQL. In order not to write longreads, the article is divided into parts.

Interested persons please proceed to cat.

1. Removing duplicate code in ReorderBufferCleanupTXN ()

I like to go through the PostgreSQL code from time to time with various static analyzers , especially the Clang Static Analyzer. Often these analyzers swear at some dubious nonsense, but sometimes you can find really very suspicious pieces of code among this nonsense. One of these pieces looked like this:

/* delete from list of known subxacts */
if (txn->is_known_as_subxact)
{
    /* NB: nsubxacts count of parent will be too high now */
    dlist_delete(&txn->node);
}
/* delete from LSN ordered list of toplevel TXNs */
else
{
    dlist_delete(&txn->node);
}

Agree, it’s rather strange to do the same thing in if and else blocks. After a short discussion of this problem in the mailing list and just one rewrite of the patch, the code turned into:

/*
 * Remove TXN from its containing list.
 *
 * Note: if txn->is_known_as_subxact, we are deleting the TXN from its
 * parent's list of known subxacts; this leaves the parent's nsubxacts
 * count too high, but we don't care.  Otherwise, we are deleting the TXN
 * from the LSN-ordered list of toplevel TXNs.
 */
dlist_delete(&txn->node);

Discussion: 20160404190345.54d84ee8@fujitsu
Commit: b6182081be4a795d70b966be2542ad32d1ffbc20

2. Correction of double initialization of variables

Honestly, I don’t remember whether this problem was found by the eyes, or by a static analyzer. In several places, code was found in the style:

char	   *qual_value;
ParseState *qual_pstate = make_parsestate(NULL);
/* parsestate is built just to build the range table */
qual_pstate = make_parsestate(NULL);

As you can see, the variable is initialized twice, only in vain warming the processor. The patch was instantly accepted without any special questions.

Discussion: 20160316112019.64057481@fujitsu
Commit: bd0ab28912d7502b237b8aeb95d052abe4ff6bc6

3. Correction of typos in the comments

In any fairly large project there is a fair amount of typos. Finding them is very simple by enabling spell checking in your IDE or text editor. I personally write code in Vim . To check spelling in ~ / .vimrc, I have the lines:

command! SpellOn :set spell spelllang=en_us,ru_ru
command! SpellOff :set nospell

If anyone is interested, then the full version of my ~ / .vimrc, as well as all other configuration files, are available here .

Often typos appear for the reason that before accepting the patches, the committers rewrite them a little bit, just a little bit. The result is a completely new code that no one has ever read before. You can send a few patches every week just by carefully reading new commits and finding typos in them!

Discussion: (something cannot be found)
Commit: 2d8a1e22b109680204cb015a30e5a733a233ed64

4. Correction of two identical comments

In addition to typos in the comments, there are other types of errors. For example, as a result of the b6fb6471 commit , this piece of code was added:

/*-----------
 * pgstat_progress_update_param() -
 *
 * Update index'th member in st_progress_param[] of own backend entry.
 *-----------
 */
void
pgstat_progress_update_param(int index, int64 val)
{
   volatile PgBackendStatus *beentry = MyBEEntry;
   Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
   if (!beentry || !pgstat_track_activities)
       return;
   pgstat_increment_changecount_before(beentry);
   beentry->st_progress_param[index] = val;
   pgstat_increment_changecount_after(beentry);
}
/*-----------
 * pgstat_progress_end_command() -
 *
 * Update index'th member in st_progress_param[] of own backend entry.
 *-----------
 */
void
pgstat_progress_end_command(void)
{

You may notice that two different procedures have exactly the same description, which is clearly some kind of jamb.

Discussion: 20160310120506.5007ea28@fujitsu
Commit: 090b287fc59e7a44da8c3e0823eecdc8ea4522f2

5. Worings when compiling on FreeBSD

Most PostgreSQL developers sit under MacOS and Linux. Therefore, it can be useful to try to build a project on exotic such as Microsoft Windows :) or FreeBSD. Using this trick, for example, I was able to find that PostgreSQL on FreeBSD is going with the following warning:

pg_locale.c:1290:12: warning: implicit declaration of function
'wcstombs_l' is invalid in C99 [-Wimplicit-function-declaration]
result = wcstombs_l(to, from, tolen, locale);
pg_locale.c:1365:13: warning: implicit declaration of function
'mbstowcs_l' is invalid in C99 [-Wimplicit-function-declaration]
result = mbstowcs_l(to, str, tolen, locale);
2 warnings generated.

It turned out to be not too difficult to fix this problem, although it required to tinker with Autotools , which, in my experience, is usually not very pleasant.

Discussion: 20160310163632.53d8e2cc@fujitsu
Commit: 0e9b89986b7ced6daffdf14638a25a35c45423ff

To be continued...

As you can see, in order to start contributing to PostgreSQL, you don’t need to have a deep knowledge of relational database devices or ten years of programming experience in C. By and large, anyone can become a contributor, in theory - not even able to program at all. In this part, perhaps the most trivial patches were considered. Next time we will look at more interesting patches that solve the lock contention problem, reduce the complexity of the algorithm from O (N) to O (1), implement binary tree traversal, repair resource leaks, and more.

As always, I will be glad to any of your comments and questions!

Continued: Examples of real patches in PostgreSQL: part 2 of N