Contributing 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 were discussed in detail, some ideas for the first patch were proposed, and it was told where and how these patches should be sent. Reference was also made to additional sources of information regarding the internal structure of the RDBMS.

Now we look at examples of real patches taken in PostgreSQL lately. Some of these patches were written directly by me, and I actively participated in the development of others as a reviewer. These are relatively small patches. At the time of this writing, I have been developing PostgreSQL for less than a year, and I have not been involved in developing a database before (exactly like developing in C 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, and not necessarily PostgreSQL. In order not to write longridov, the article is divided into parts.
')
Interested please follow under cat.

1. Delete duplicate code in ReorderBufferCleanupTXN ()

I like to go through the PostgreSQL code from time to time with different static analyzers , especially the Clang Static Analyzer. Often these analyzers swear at some dubious nonsense, but in the midst of this nonsense you can sometimes find really very suspicious pieces of code. 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 is rather strange to do the same in the if and else blocks. After a short discussion of this problem in the mailing list and just one patch rewrite, 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. Fix double variable initialization

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

 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 accepted instantly without any questions.

Discussion: 20160316112019.64057481@fujitsu
Commit: bd0ab28912d7502b237b8aeb95d052abe4ff6bc6

3. Correction of typos in the comments

In any large enough project there is a fair amount of typos. Finding them is easy by turning on spell checking in your IDE or text editor. I personally write code in Vim . To check the 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, exactly like all the other configuration files, is available here .

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

Discussion: (something can not 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) { 

It can be noted that two different procedures have a completely identical description, which is clearly some kind of a jamb.

Discussion: 20160310120506.5007ea28@fujitsu
Commit: 090b287fc59e7a44da8c3e0823eecdc8ea4522f2

5. Vorning when compiling on FreeBSD

Most PostgreSQL developers are running MacOS and Linux. Therefore, it is useful to try to build a project on an exotic type of Microsoft Windows :) or FreeBSD. Using this technique, for example, I managed to find out that PostgreSQL on FreeBSD is going with the following warning signals:

 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 was not too difficult to fix this problem, although it required you to tinker with Autotools , which, in my experience, is usually not a very pleasant experience.

Discussion: 20160310163632.53d8e2cc@fujitsu
Commit: 0e9b89986b7ced6daffdf14638a25a35c45423ff

To be continued...

As you see, in order to start contributing to PostgreSQL, you do not need a deep knowledge of relational database devices or ten years of C programming experience. By and large, anyone can become a contributor, in theory, even if they cannot program at all. In this part, the most trivial patches were considered. Next time we will look at patches more interestingly, solving the problem of lock contention, reducing the complexity of the algorithm from O (N) to O (1), implementing the bypass of binary trees, repairing resource leaks, and not only.

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

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