Контрибьютим в PostgreSQL: примеры реальных патчей, часть 1 из N09.09.2016 11:34

c0a2b3b47dbe4bce8a31b7aa10980f6e.png

Ранее в статье Становимся контрибьютером в PostgreSQL был подробно рассмотрен процесс разработки PostgreSQL и используемые при этом инструменты, были предложены некоторые идеи для первого патча и рассказано, куда и как эти патчи нужно посылать. Также были приведены ссылки на дополнительные источники информации касательно внутреннего устройства РСУБД.

Теперь же мы рассмотрим примеры реальных патчей, принятых в PostgreSQL за последнее время. Какие-то из этих патчей были написаны непосредственно мной, при разработке других я активно участвовал в качестве ревьювера. Это сравнительно небольшие патчи. На момент написания этих строк я занимаюсь разработкой PostgreSQL менее года, и ранее разработкой СУБД я не занимался (ровно как и разработкой на языке C за деньги). Поэтому есть основания полагать, что данные патчи будут интересны новичкам, желающим начать участвовать в разработке открытых проектов, притом не обязательно именно PostgreSQL. Чтобы не писать лонгридов, статья разбита на части.

Заинтересовавшихся прошу проследовать под кат.

1. Удаление дублированного кода в ReorderBufferCleanupTXN ()

Мне нравится время от времени проходиться по коду PostgreSQL разными статическими анализаторами, особенно Clang Static Analyzer. Часто эти анализаторы ругаются на какую-то сомнительную ерунду, но среди этой ерунды иногда можно найти действительно очень подозрительные куски кода. Один из таких кусков выглядел следующим образом: 

/* 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);
}

Согласитесь, довольно странно делать в блоках if и else одно и то же. После короткого обсуждения этой проблемы в рассылке и всего лишь одного переписывания патча код превратился в: 

/*
 * 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);

Обсуждение: 20160404190345.54d84ee8@fujitsu
Коммит: b6182081be4a795d70b966be2542ad32d1ffbc20

2. Исправление двойной инициализации переменных

Честно говоря, я уже не помню, была ли эта проблема найдена глазами, или же статическим анализатором. В нескольких местах был обнаружен код в стиле: 

char	   *qual_value;
ParseState *qual_pstate = make_parsestate(NULL);

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

Как видите, переменная инициализируется дважды, только напрасно грея процессор. Патч был принят моментально без особых вопросов.

Обсуждание: 20160316112019.64057481@fujitsu
Коммит: bd0ab28912d7502b237b8aeb95d052abe4ff6bc6

3. Исправление опечаток в комментариях

В любом достаточно крупном проекте присутствует изрядное количество опечаток. Найти их очень просто, включив проверку орфографии в вашей IDE или текстовом редакторе. Я лично пишу код в Vim. Для проверки орфографии в ~/.vimrc у меня есть строчки: 

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

Если кому-то интересно, то полная версия моего ~/.vimrc, ровно как и всех остальных конфигурационных файлов, доступны здесь.

Нередко опечатки появляются по той причине, что перед принятием патчей коммиттеры немного, совсем чуть-чуть, переписывают их. В результате получается совершенно новый код, который никто до этого не вычитывал. Можно каждую неделю слать несколько патчей, просто внимательно вычитывая новые коммиты и находя в них опечатки!

Обсуждение: (что-то не удается найти)
Коммит: 2d8a1e22b109680204cb015a30e5a733a233ed64

4. Исправление двух идентичных комментариев

Помимо опечаток в комментариях встречаются и другие виды ошибок. Например, в результате коммита b6fb6471 был добавлен такой кусок кода: 

/*-----------
 * 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)
{

Можно заметить, что две разные процедуры имеют совершенно одинаковое описание, что явно какой-то косяк.

Обсуждение: 20160310120506.5007ea28@fujitsu
Коммит: 090b287fc59e7a44da8c3e0823eecdc8ea4522f2

5. Ворнинги при компиляции на FreeBSD

Большинство разработчиков PostgreSQL сидят под MacOS и Linux. Поэтому бывает полезно попытаться собрать проект на экзотике типа Microsoft Windows:) или FreeBSD. Используя этот прием, мне, например, удалось обнаружить, что PostgreSQL на FreeBSD собирается со следующими 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.

Исправить эту проблему оказалось не слишком сложно, хотя и потребовало повозиться с Autotools, что, по моему опыту, обычно не очень приятное занятие.

Обсуждение: 20160310163632.53d8e2cc@fujitsu
Коммит: 0e9b89986b7ced6daffdf14638a25a35c45423ff

Продолжение следует…

Как видите, чтобы начать контрибьютить в PostgreSQL, не требуется глубокого знания устройства реляционных баз данных или десяти лет опыта программирования на языке C. По большому счету, стать контрибьютором может любой человек, в теории — даже не умеющий программировать вообще. В этой части были рассмотрены, пожалуй, самые тривиальные патчи. В следующий раз мы рассмотрим патчи поинтереснее, решающие проблему lock contention, уменьшающие сложность алгоритма с O (N) до O (1), реализующие обход бинарных деревьев, чинящие утечки ресурсов, и не только.

Как всегда, я буду рад любым вашим комментариям и вопросам!

Комментарии (0)

© Habrahabr.ru