API курильщика или «Хорошо, что я не психопат»

СУБД Pangolin, в которой я теперь работаю, основана на open-source'ном код PostgreSQL. А значит, наша кодовая база является бесконечным источником говнокода поучительных примеров 😁

Я вчера два часа жизни потратил на отлавливание проезда по памяти. Написал код, который создаёт новый кортеж для последующей вставки его в таблицу. И там segfault 💣 А Postgres — это всякие межпроцессные взаимодействия, синхронизация через общую память и прочие места, где легко накосячить. Ну вот я копал в эту сторону, обложившись GDB, breakpoint'ами и отладочным выводом в логи.

Какого же было моё возмущение, когда я нашёл причину 😬🤬

В Postgres есть довольно простая функция создания нового кортежа — heap_form_tuple:

HeapTuple heap_form_tuple(
    TupleDesc tupleDescriptor,
    const Datum *values,
    const bool *isnull);

Она принимает некое описание кортежа и два массива:
- значения его колонок
- булевский массив, который задаёт, какие колонки имеют значение NULL
Размер этих массивов записан в tupleDescriptor.

А ещё для удобства есть константы, которые задают номера колонок системных таблиц. То есть массив values можно заполнять вот так:

Datum *values = ...;
bool *isnulls = ...;

values[Anum_pg_authid_oid] = ObjectIdGetDatum(roleid);
values[Anum_pg_authid_rolname] = CStringGetDatum("user");
...
isnulls[Anum_pg_authid_rolvaliduntil] = true; // Говорим, что эта колонка имеет значение NULL

HeapTuple tuple = heap_form_tuple(desc, values, isnulls);

И какого же было моё изумление, что нумерация констант Anum_pg_authid_oid, Anum_pg_authid_rolname и т.п. начинается с единицы! С единицы, Карл! В языке, где вся индексация везде осуществляется с нуля! 🤯

И проезд по памяти у меня происходил в строке isnulls[Anum_pg_authid_rolvaliduntil] = true, потому что Anum_pg_authid_rolvaliduntil имеет значение 12, а в массиве isnulls как раз было выделено 12 элементов 🤦‍♂️

А значит, чтобы корректно использовать удобные константы, нужно каждый раз вычитать из них 1 при заполнении кортежа (код ниже взят отсюда):

new_record[Anum_pg_authid_rolname - 1] =
   DirectFunctionCall1(namein, CStringGetDatum(stmt->role));
new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(issuper);
new_record[Anum_pg_authid_rolinherit - 1] = BoolGetDatum(inherit);
new_record[Anum_pg_authid_rolcreaterole - 1] = BoolGetDatum(createrole);
new_record[Anum_pg_authid_rolcreatedb - 1] = BoolGetDatum(createdb);
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin);
new_record[Anum_pg_authid_rolreplication - 1] = BoolGetDatum(isreplication);
new_record[Anum_pg_authid_rolconnlimit - 1] = Int32GetDatum(connlimit);

Хороший API легко использовать правильно и тяжело — неправильно. API выше — плохой. Гораздо лучше было бы пронумеровать константы с нуля, а внутри функций, которым нужна нумерация с единицы, делать +1. А так получилось, что деталь реализации пролезла в интерфейс, и отобрала у меня 2 часа на дебаг 😔

Как говорится, пишите код так, как будто сопровождать его будет склонный к насилию психопат, который знает, где вы живете. Хорошо, что я не психопат и, разобравшись с этим, продолжил заниматься рабочими задачами. А то ж ведь мог и в git blame полезть 😆