- PVSM.RU - https://www.pvsm.ru -
Ранее в статье Становимся контрибьютером в PostgreSQL [1] был подробно рассмотрен процесс разработки PostgreSQL и используемые при этом инструменты, были предложены некоторые идеи для первого патча и рассказано, куда и как эти патчи нужно посылать. Также были приведены ссылки на дополнительные источники информации касательно внутреннего устройства РСУБД.
Теперь же мы рассмотрим примеры реальных патчей, принятых в PostgreSQL за последнее время. Какие-то из этих патчей были написаны непосредственно мной, при разработке других я активно участвовал в качестве ревьювера. Это сравнительно небольшие патчи. На момент написания этих строк я занимаюсь разработкой PostgreSQL менее года, и ранее разработкой СУБД я не занимался (ровно как и разработкой на языке C за деньги). Поэтому есть основания полагать, что данные патчи будут интересны новичкам, желающим начать участвовать в разработке открытых проектов, притом не обязательно именно PostgreSQL. Чтобы не писать лонгридов, статья разбита на части.
Заинтересовавшихся прошу проследовать под кат.
Мне нравится время от времени проходиться по коду PostgreSQL разными статическими анализаторами [2], особенно 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 [3]
Коммит: b6182081be4a795d70b966be2542ad32d1ffbc20 [4]
Честно говоря, я уже не помню, была ли эта проблема найдена глазами, или же статическим анализатором. В нескольких местах был обнаружен код в стиле:
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 [5]
Коммит: bd0ab28912d7502b237b8aeb95d052abe4ff6bc6 [6]
В любом достаточно крупном проекте присутствует изрядное количество опечаток. Найти их очень просто, включив проверку орфографии в вашей IDE или текстовом редакторе. Я лично пишу код в Vim [7]. Для проверки орфографии в ~/.vimrc у меня есть строчки:
command! SpellOn :set spell spelllang=en_us,ru_ru
command! SpellOff :set nospell
Если кому-то интересно, то полная версия моего ~/.vimrc, ровно как и всех остальных конфигурационных файлов, доступны здесь [8].
Нередко опечатки появляются по той причине, что перед принятием патчей коммиттеры немного, совсем чуть-чуть, переписывают их. В результате получается совершенно новый код, который никто до этого не вычитывал. Можно каждую неделю слать несколько патчей, просто внимательно вычитывая новые коммиты и находя в них опечатки!
Обсуждение: (что-то не удается найти)
Коммит: 2d8a1e22b109680204cb015a30e5a733a233ed64 [9]
Помимо опечаток в комментариях встречаются и другие виды ошибок. Например, в результате коммита b6fb6471 [10] был добавлен такой кусок кода:
/*-----------
* 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 [11]
Коммит: 090b287fc59e7a44da8c3e0823eecdc8ea4522f2 [12]
Большинство разработчиков 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 [13], что, по моему опыту, обычно не очень приятное занятие.
Обсуждение: 20160310163632.53d8e2cc@fujitsu [14]
Коммит: 0e9b89986b7ced6daffdf14638a25a35c45423ff [15]
Как видите, чтобы начать контрибьютить в PostgreSQL, не требуется глубокого знания устройства реляционных баз данных или десяти лет опыта программирования на языке C. По большому счету, стать контрибьютором может любой человек, в теории — даже не умеющий программировать вообще. В этой части были рассмотрены, пожалуй, самые тривиальные патчи. В следующий раз мы рассмотрим патчи поинтереснее, решающие проблему lock contention, уменьшающие сложность алгоритма с O(N) до O(1), реализующие обход бинарных деревьев, чинящие утечки ресурсов, и не только.
Как всегда, я буду рад любым вашим комментариям и вопросам!
Автор: Postgres Professional
Источник [16]
Сайт-источник PVSM.RU: https://www.pvsm.ru
Путь до страницы источника: https://www.pvsm.ru/razrabotka/185206
Ссылки в тексте:
[1] Становимся контрибьютером в PostgreSQL: https://habrahabr.ru/company/postgrespro/blog/308442/
[2] разными статическими анализаторами: http://eax.me/c-static-analysis/
[3] 20160404190345.54d84ee8@fujitsu: https://www.postgresql.org/message-id/flat/20160404190345.54d84ee8@fujitsu#20160404190345.54d84ee8@fujitsu
[4] b6182081be4a795d70b966be2542ad32d1ffbc20: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b6182081be4a795d70b966be2542ad32d1ffbc20;hp=c7f68bea22bf680a4eab4b8b1592b3c90bc634ac
[5] 20160316112019.64057481@fujitsu: https://www.postgresql.org/message-id/flat/20160316112019.64057481%40fujitsu#20160316112019.64057481@fujitsu
[6] bd0ab28912d7502b237b8aeb95d052abe4ff6bc6: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=bd0ab28912d7502b237b8aeb95d052abe4ff6bc6;hp=c27033ff7c17b5100d02c454a0eebb95ec7b91cc
[7] пишу код в Vim: http://eax.me/vim-commands/
[8] доступны здесь: https://github.com/afiskon/freebsd-on-desktop-v2/tree/master/home/eax
[9] 2d8a1e22b109680204cb015a30e5a733a233ed64: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=2d8a1e22b109680204cb015a30e5a733a233ed64;hp=aa698d753566f68bdd54881d30b1a515b0327b0e
[10] коммита b6fb6471: https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=b6fb6471f6afaf649e52f38269fd8c5c60647669
[11] 20160310120506.5007ea28@fujitsu: https://www.postgresql.org/message-id/flat/20160310120506.5007ea28%40fujitsu#20160310120506.5007ea28@fujitsu
[12] 090b287fc59e7a44da8c3e0823eecdc8ea4522f2: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=090b287fc59e7a44da8c3e0823eecdc8ea4522f2;hp=cc402116ca156babcd3ef941317f462a96277e3a
[13] повозиться с Autotools: http://eax.me/autotools/
[14] 20160310163632.53d8e2cc@fujitsu: https://www.postgresql.org/message-id/flat/20160310163632.53d8e2cc%40fujitsu#20160310163632.53d8e2cc@fujitsu
[15] 0e9b89986b7ced6daffdf14638a25a35c45423ff: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=0e9b89986b7ced6daffdf14638a25a35c45423ff;hp=101fd9349eddb7e9ed84a239145d5230a9bc7336
[16] Источник: https://habrahabr.ru/post/309488/?utm_source=habrahabr&utm_medium=rss&utm_campaign=best
Нажмите здесь для печати.