Оглавление

• I Трудности селектописательства

• II Доступные средства

• III Защита от CODE SMELL

• IV Испытания

• P.S.

.\TeamTools.Linter.CommandLine.exe --dir "c:\source\my_project" --evaluate

I Трудности селектописательства

Практика открыть SSMS, подключиться к проду, кликнуть по названию хранимки и выбрать пункт меню «Modify», после манипуляций нажать F5, всё не уйдёт в небытие. Можно согласиться, что с опытом нарабатывается чутьё на потенциальные проблемы, но, что гораздо важнее, и знание возможных мероприятий, практик, приёмов, позволяющих снизить вероятность ошибки, упростить обнаружение проблемы, проникшей в код, ограничить негативные последствия.

Подобные практики хорошо описаны в материалах по DevOps-идеологии и в SWEBoK. К тематике статьи ближе всего эти:

• организация исходного кода;

• автоматизированное тестирование;

• ревью изменений;

• непрерывная интеграция;

• метрики качества кода.

Экосистема SQL-разработки

В экосистемах других направлений программирования давно стали нормой, частью элементарной гигиены проектов инструменты наподобие eslint, flake8, StyleCop, каждый с более чем сотней встроенных правил. Статьи с описанием базовой настройки репозитория, номинального пайплайна CI/CD для этих направлений в обязательном порядке содержат пункты с подключением статического анализатора кода, с запуском юнит-тестов и расчётом покрытия кода. Экосистема SQL-разработки выглядит на этом фоне скромно: вот sqlproj, который выдаст dacpac для деплоя, а в остальном полагайтесь на себя.

Фреймворк Microsoft SSDT предлагает структурированное хранение исходников базы данных для SQL Server, а вместе с этим инструмент для автоматизации развёртывания. Но вначале исходники должны храниться в VCS, а не на инстансе СУБД. Как же этого добиться, если по сей день доводится слышать от участника разработки проекта БД с активным программированием развесистых хранимых процедур, что система контроля версий бессмысленна, поскольку предыдущую версию кода возможно поднять из бэкапа БД... Нет исходников — невозможно проводить код-ревью в общем понимании этого процесса, отсутствует прозрачная история изменений, невозможно построение цепочки CI/CD. Без конвейера непрерывной интеграции некуда встроить статический анализ кода и юнит-тесты так, чтобы они гарантированно проверяли конкретное состояние исходников. Как следствие, зрелость процессов разработки безальтернативно остаётся на невысоком уровне.

Сложно согласиться с тем, что это данность и норма — узнавать об абсолютно явных ошибках через сломанный деплой, нескончаемый поток исключений в рантайме. Или что бурные дискуссии на ревью о размере отступов и регистре инструкций есть занятие, достойное траты времени и нервов специалистов.

В сложно выкроенных сценариях допустить едва заметную ошибку вероятно и при хорошем покрытии тестами, с код-ревью и прочими полезными ритуалами, но так ли много нужно для ошибки при программировании наугад. В T-SQL за примерами неочевидных ошибок далеко ходить не придётся.

Коварный NULL

Обработка значений NULL является стандартной рутиной при работе с данными, написании SQL-скриптов, однако и здесь не обходится без спотыкания о подводные камни. Пожалуй, самая известная функция для обработки нуллов — ISNULL. Первый параметр — значение, которое может оказаться нуллом, второй параметр — что подставить вместо нулла в этой ситуации. Распространённый сценарий для применения этой функции — обработка входных параметров (пусть будет простой DECLARE вместо развесистой шапки создания хранимки с параметром):

DECLARE @arg CHAR(2)
SELECT ISNULL(@arg, '<NULL>');

Казалось бы, всё элементарно и просто, однако выведено будет не '<NULL>', а '<N'. Особенностью функции ISNULL является то, что тип результата будет абсолютно такой же, какой был у первого параметра, вплоть до точности числа или длины строки. Таким образом, поскольку переменная @arg имеет тип CHAR(2), то и результат будет именно этого типа, то есть значение из второго параметра в примере обрежется до двух символов.

Чуть более яркий пример:

DECLARE @arg TINYINT
SELECT ISNULL(@arg, -1);

пользователь мог выбрать значение от 0 до 255, но ничего не выбрал, поэтому обозначим отсутствие выбора числом вне этого диапазона. Вот только TINYINT совершенно неприспособлен хранить в себе отрицательные значения, и в данном примере вовсе произойдёт ошибка в рантайме вида arithmetic overflow. При этом что парсинг средствами DB Engine, что компиляция SSDT никакой проблемы не видят, поскольку синтаксис валиден.

COALESCE, IFNULL, IIF организованы иначе и с типами данных работают аналогично конструкции CASE, синтаксическим сахаром вокруг которого они и являются. Для CASE сиквел вначале просчитывает результирующий тип с учётом приоритета типов и общего намерения поменьше потерять в преобразованиях, затем уже проводит вычисления. В такой реализации не будет ни усечения, ни out of range ошибки из примеров выше:

DECLARE @arg1 TINYINT
PRINT IIF(@arg1 IS NULL, -1, @arg1);

DECLARE @arg2 CHAR(2)
PRINT COALESCE(@arg2, '<NULL>');

Арифметические действия и сравнение с нуллом могут вернуть лишь NULL/UNKNOWN и код вида @a <> NULL не является ANSI-совместимым, тем не менее снова компилятор SSDT не видит проблемы, хотя она очевидна. В итоге на проде может оказаться код, изначально неспособный выполнять задуманное. Подобные фрагменты, к слову, способны возникать и у опытных специалистов после нескольких раундов рефакторинга, переосмысления ранее вполне валидного кода. Ещё проще упустить момент обработки переменной, в которую фактически так ничего и не положили:

DECLARE @value INT

-- always false condition
IF 1 = 0
    SET @value = 1
    
-- lots of code

IF @value > 0
    SELECT 'yes'

В развесистой хранимке несложно упустить один из вариантов ветвлений кода, который приведёт к подобной ситуации, а вот проявление порождённой проблемы обнаружить будет непросто. И снова ни SSDT, ни компилятор SQL SERVER ничего не подскажут.

Опять же, распространённый ход, но тоже code smell:

SELECT
    id,
    title,
    NULL as start_time
FROM foo
WHERE client_id = @client_id

столбец start_time по замыслу автора никогда не может быть заполнен и потому там стоит просто NULL. Выглядит здорово, но какой тип у этого NULL? Сам нулл типа не имеет, как и значения — он UNKNOWN, но вот резалтсет без вариантов состоит из строк и столбцов, а у столбца конкретный тип быть обязан. И этот тип несложно выяснить:

EXEC sp_describe_first_result_set
    @tsql = N'SELECT NULL as start_time',
    @params = NULL,
    @browse_information_mode = 1;

он будет в такой ситуации всегда INT. Однако имя столбца start_time подозрительно не похоже на типичный INT. Кажется, это должно быть или время, или дата со временем. В общем случае подобный код к ошибке не приведёт, разве что на принимающей стороне окажется нечто со строгой типизацией. Зато может попортить нервы и отнять уйму сил на выяснение, что же там подразумевалось. Явная или неявная проблема может возникнуть в ETL, в отображении пользователю, в аналитике при склейке данных. А достаточно было чётко обозначить намерения и явно присвоить столбцу задуманный тип данных.

Беспринципные строки

Со строками в T-SQL работать вполне удобно, хоть в чём-то, может, и не хватает моментов, которые предоставляют многие ООП-языки. Так, строковые столбцы, а вместе с ними и скалярные переменные, обязательно имеют ограниченную длину (кроме варианта длины MAX, который появился в SQL SERVER не сразу). То есть, если в такую переменную или такой столбец попытаться сохранить значение длиннее, оно обрежется:

DECLARE @str VARCHAR(5) = '1234567890'
SELECT @str

в этом случае переменная сохранит только первые 5 символов, и ни для компилятора, ни для рантайма это проблемой не является, хотя ситуация похожа на абсурд. Часть данных молча потеряется. Для столбцов, к слову, сиквел будет чуть более бдителен и при вставке выдаст string or binary data would be truncated, но скаляры ему глубоко безразличны. Как и юникод:

DECLARE @str VARCHAR(5) = '😛⭐❤🚀💼'
SELECT @str

В примере плохо всё, но сиквел и SSDT это устраивает. Если исправить тип данных переменной на NVARCHAR, то лучше не станет, поскольку справа остаётся однобайтовая строка вне зависимости от фактического содержимого. Только если ещё и справа добавить N перед открывающей кавычкой, наконец, случится задуманное. И литералы — это самый простой кейс, в котором всё же возможно дров наломать. Молчаливое согласие DB Engine и SSDT вполне способны породить сложнопоправимый беспорядок в перекладывании данных из одних колонок в другие, при передаче параметров между хранимыми процедурами и функциями, в обработке результата — сквозной контроль совместимости и равнозначности типов в T-SQL отсутствует.

Отдельный сюрприз для любителей что-то закопипастить из браузера, затем вставить в код — невидимые символы (вот их целая коллекция):

DECLARE @str NVARCHAR(5) = N'‎‌'
SELECT '"' + @str + '"', LEN(@str), DATALENGTH(@str)

Notepad++ при включённом просмотре всех символов отобразит литерал как LRM ZWNJ. Визуальная длина 0, фактическая — 2 символа. Если закрадётся где-то в литерале и попадёт на прод, то лишний пассажир обнаружится нескоро. И ведь на ревью такие символы никто глазами не увидит. Железяке эти байты великолепно видны, но железо молчит.

Парсер не компайлер

Считать железяка тоже вроде бы способна, но ей нужно объяснить, что, собственно, подсчитывать. Например, DB Engine подсчитать количество столбцов, перечисленных в INSERT вполне способен, а SSDT — нет:

DECLARE @foo TABLE (a INT, b INT, c INT, d INT, e INT)

INSERT INTO @foo(a, b, c, d, e)
VALUES (1, 2)

DECLARE cr CURSOR FAST_FORWARD FOR
SELECT a, b
FROM @foo

OPEN cr

FETCH NEXT FROM cr
INTO @a, @b, @c, @d, @e

CLOSE cr
DEALLOCATE cr

Очевидно, что и FETCH здесь проблемный, но если не заметить это глазами, то ошибка проявится только при раскатке на целевой инстанс Sql Server.

Бывает и так, что усталость даёт о себе знать, и распространённые в иных сценариях паттерны стреляют там, где сам от себя не ожидаешь:

CREATE TRIGGER tr ON dbo.foo
AFTER INSERT, UPDATE, DELETE
AS
BEGIN
    RETURN 0;
END

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

CREATE FUNCTION my_fn(@inc INT)
RETURNS INT
AS
BEGIN
    SET NOCOUNT ON;
    
    RETURN @inc + 1;
END

ведь функции не допускают выставления опций в своём теле. Безусловно, если писать не наугад, а проверять своё творчество в dev-контуре, то подобные промашки проявят себя мгновенно. Впрочем, нередко ошибочные строки возникают в последний момент от внезапного приступа наведения финальной красоты перед коммитом, а то и вовсе не понятно как: то ли CTRL+S забыл нажать в IDE, то ли правильный файл лёг невесть куда.

Загадочные обстоятельства

В приведённом ниже примере код выглядит в целом довольно стройным: перечислены поля для вставки, расставлены алиасы, применен SET-based, а не построчный подход, относительно современный OUTPUT-clause.

INSERT INTO dbo.clients(client_name)
    OUTPUT
        INSERTED.client_id,
        INSERTED.current_status,
        DELETED.current_status
    INTO @changes(client_id, current_status_new, current_status_old)
SELECT jrq.full_name
FROM dbo.join_requests jrq
WHERE jrq.confirmed = 1

Вот только не очень понятно, откуда же возьмутся DELETED-значения, если это вставка. Кто знает, какой путь проделал автор, чтобы оказаться в этой точке. Может, сначала был MERGE, потом UPDATE и INSERT, в конце остался только INSERT. А может это не до конца адаптированная паста откуда-то. Но итоговый факт простой: никаких current_status_old здесь получить невозможно.
В похожей ситуации может оказаться разработчик триггера, который сначала был AFTER INSERT, UPDATE, DELETE, а после нескольких итераций остался только на INSERT, при этом расчёт на заполненность системной таблицы DELETED остался.

Копипаста и, как недавно выяснили коллеги, кодогенерирующий ИИ вполне способны порождать такие фрагменты:

UPDATE c SET
    blocked = 1
FROM dbo.clients c
WHERE client_id = client_id

где с точки зрения синтаксиса T-SQL и рантайма всё идеально, никаких вопросов нет: хочет разработчик сравнивать одно и то же — кто я такой, чтобы спорить. Стоит отметить, нагенеренные примеры с always true предикатом WHERE встретились (пока) только в тестах, которые мы гоняем на пустых клонах, поэтому смертельного ничего не произошло. Но если в пайплайне CI/CD нет элемента, способного предотвратить доведение проблемного кода до продакшена, то и в бизнес-логике почему бы не стрельнуть подобной апдейт-бомбе.

Паста рукотворная — стабильный поставщик загадочных обстоятельств в реальном коде. Вот что здесь имелось ввиду:

SELECT
    CASE
        WHEN tariff_code = 'ECONOM' THEN 0.5
        WHEN tariff_code = 'PRO' AND balance > 0 THEN 0.7
        WHEN tariff_code = 'PRO' AND balance > 0 THEN 1.9
        WHEN tariff_code = 'LUX' THEN 2.0
    END AS price_magnifier

так 0.7 или 1.9? Очевидно, с момента появления такого кода при соблюдении условий всегда отрабатывала первая версия; вопрос, насколько это было корректно. Но что-то же имел в виду автор последующей строки. В последний раз, когда такое у себя нарыли, долго думали, общались с аналитиком, но тот тоже не хотел пробуждать древнее зло.

Древнее ли, новодельное, зло вполне может отказаться быть пробуждённым, если его обложить вот такими условиями:

IF @clocks > 100
BEGIN
    IF @clocks < 0
    BEGIN
        PRINT 'I am Muzzy. I''m hungry.'
    END
END

мало ли: было 200, стало -7. Всякое бывает. Пример хоть и утрирован до комедийности, однако в километровых дремучих хранимках несколько раз описанный паттерн фактически находили. Кто знает, то ли с расстановкой BEGIN-END, ELSE-ов автор промахнулся, то ли раньше что-то меняло значение переменной перед вторым условием. В любом случае, код очевидно некорректен, и снова это способен заметить либо сверхзоркий глаз, либо доскональное тестирование. Статический анализ мог бы заметить противоречие ещё до коммита, но Microsoft не доукомплектовывает SSDT ничем подобным.

Идеально, но нет

В другой своей статье приводил пример последствий отсутствия обработки ошибок, здесь приведу только фрагмент псевдокода:

BEGIN TRAN
    INSERT...
    UPDATE...
    UPDATE...
COMMIT TRAN

на первый взгляд выглядит великолепно: обозначен блок атомарных изменений, есть явное управление транзакцией. Но часть ошибок, в том числе контроля целостности данных, by design не приводит к прерыванию батча и откату транзакции. Таким образом, фактически здесь заложена возможность фрагментарного применения изменений. Нужна не менее явная обработка ошибок с помощью блока TRY-CATCH (постоянная проверка @@ERROR в старых версиях SQL SERVER) и/или включение XACT_ABORT. Без этого код неполноценен.

Ошибки, к слову, могут не только не приводить к прерыванию транзакции, но и вовсе не генерировать исключений:

USE msdb;
GO

-- 1. Создание джоба
DECLARE @JobID UNIQUEIDENTIFIER;

EXEC sp_add_job
    @job_name = N'Мой тестовый джоб',
    @enabled = 1,
    @description = N'Тестовый джоб с одним шагом',
    @category_name = N'[Uncategorized (Local)]',
    @owner_login_name = N'sa',
    @USE msdb;
GO

-- 1. Создание джоба
DECLARE @JobID UNIQUEIDENTIFIER;

EXEC sp_add_job
    @job_name = N'Мой тестовый джоб',
    @enabled = 1,
    @description = N'Тестовый джоб с одним шагом',
    @category_name = N'[Uncategorized (Local)]',
    @owner_login_name = N'sa',
    @job_id = @JobID OUTPUT;

-- 2. Добавление шага к джобу
EXEC sp_add_jobstep
    @job_id = @JobID,
    @step_name = N'Шаг 1 - Выполнение запроса',
    @subsystem = N'TSQL',
    @command = N'SELECT GETDATE() AS CurrentTime;
                WAITFOR DELAY ''00:00:10'';
                PRINT ''Шаг выполнен успешно!''',
    @on_success_action = 1,  -- Перейти к следующему шагу (если есть)
    @on_fail_action = 2,    -- Завершить выполнение с ошибкой
    @database_name = N'master';

-- 3. Назначение джоба целевому серверу
EXEC sp_add_jobserver
    @job_id = @JobID,
    @server_name = N'(LOCAL)';  -- Локальный сервер

-- 4. Создание расписания для джоба
EXEC sp_add_schedule
    @schedule_name = N'Ежедневно в 14:00',
    @freq_type = 4,           -- Ежедневно
    @freq_interval = 1,      -- Каждый день
    @active_start_time = 140000,  -- Время начала: 14:00:00
    @schedule_id = @ScheduleID OUTPUT;

-- 5. Привязка расписания к джобу
EXEC sp_attach_schedule
    @job_id = @JobID,
    @schedule_id = @ScheduleID;
GO
job_id = @JobID OUTPUT;

-- 2. Добавление шага к джобу
EXEC sp_add_jobstep
    @job_id = @JobID,
    @step_name = N'Шаг 1 - Выполнение запроса',
    @subsystem = N'TSQL',
    @command = N'SELECT GETDATE() AS CurrentTime;
                WAITFOR DELAY ''00:00:10'';
                PRINT ''Шаг выполнен успешно!''',
    @on_success_action = 1,  -- Перейти к следующему шагу (если есть)
    @on_fail_action = 2,    -- Завершить выполнение с ошибкой
    @database_name = N'master';

-- 3. Назначение джоба целевому серверу
EXEC sp_add_jobserver
    @job_id = @JobID,
    @server_name = N'(LOCAL)';  -- Локальный сервер

-- 4. Создание расписания для джоба
EXEC sp_add_schedule
    @schedule_name = N'Ежедневно в 14:00',
    @freq_type = 4,           -- Ежедневно
    @freq_interval = 1,      -- Каждый день
    @active_start_time = 140000,  -- Время начала: 14:00:00
    @schedule_id = @ScheduleID OUTPUT;

-- 5. Привязка расписания к джобу
EXEC sp_attach_schedule
    @job_id = @JobID,
    @schedule_id = @ScheduleID;
GO

Дело в том, что все эти хранимки никогда не генерируют исключение. Хоть два раза их TRY-CATCH'ем обложи, а всё равно, если что-то пошло не так даже на первой команде, то скрипт отработает до конца, в итоге гора родит мышь: Волобуев, вот ваш ноль созданных объектов.

EXEC @res = sp_add_job
    ...
IF @res <> 0
    THROW ...

Эти и некоторые другие системные процедуры о неполадках сообщают только через код возврата, и корректный скрипт должен был бы каждый раз абсолютно олдскульно, «бай хэндз» собирать этот самый код возврата и убеждаться, что он равен нулю, возвращаемому в случае успеха.

В приведённом примере создания и наполнения джоба почти каждая хранимка возвращает некий новый ID через OUTPUT-параметр. И сиквелу снова без разницы, указали вы OUTPUT атрибут в сигнатуре вызова или нет. Но без этого атрибута код мгновенно теряет смысл. Так, кроме sp_addjob вся соль в OUTPUT-параметре также у хранимок sp_xml_preparedocument, sp_OACreate и некоторых других. Очевидным образом программисты иногда забывают, что хранимки их собственноручной разработки, разбросанные по нескольким БД, тоже задуманы для возврата OUTPUT-параметров. Но бывает, что весь код про параметры сложен гораздо плотнее:

DECLARE @i INT

EXEC sp_executesql
    N'SET @modified_parameter = 1',
    N'@modified_parameter INT OUTPUT',
    @modified_parameter = @i

SELECT @i

тем не менее и в этих «трёх соснах» несложно заблудиться. Заморочились с корректным описанием динамики, но всего лишь потеряли признак OUTPUT в последнем параметре и вся суета потеряла смысл.

Подобные «приколы» обнаруживать в продакшен-коде отнюдь не весело, и очень бы хотелось, чтобы инструментарий разработки или хоть какой-нибудь элемент в CI/CD-пайплайне подобные code smell автоматически находил и подсвечивал.

II Доступные средства

Выявления проблем в коде T-SQL можно отнести к нескольким этапам:

1. Парсинг синтаксиса библиотекой ScriptDom.

2. Валидация структуры и ссылок фреймворком SSDT (DacFx).

3. Деплой скрипта и валидация средствами DB Engine.

4. Рантайм с конкретными значениями.

5. Корректность итоговых данных.

Первые два этапа отсекают лишь самые грубые ошибки: битые ссылки, совсем кривой синтаксис — но не гарантируют даже успешное преодоление стадии деплоя. При деплое валидация синтаксиса и взаимосвязей выполняются средствами непосредственно DB Engine, а не вспомогательных внешних механизмов, поэтому вполне могут возникнуть ошибки даже в синтаксисе, не выявленные на более ранних этапах.

Обидно бывает обнаружить неспособность кода работать уже в рантайме. Однако это не так страшно, как молчаливая некорректная работа. Если выполнение кода не приводит к исключению с точки зрения DB Engine и логики разработанных алгоритмов, реализованных программистом ограничений, но при этом результат не соответствует задуманному, то такую ошибку найти сложнее всего. Знакомые с концепцией SDLC понимают, что чем на более позднем этапе реализации программного продукта выявляется несоответствие, баг, тем дороже он обходится и в смысле буквальных финансовых потерь, но также и с точки зрения затрачиваемых ресурсов на выявление и устранение проблемы.

От полноценного набора инструментов всё же ожидают, что очевидные ошибки, как и сомнительные трюки, будут выявлены на ранних этапах цикла разработки. Для этого в дополнение к средствам компиляции и юнит-тестам применяют статические анализаторы. Приличный линтер не только помогает находить явные баги, но также подсвечивает сомнительные, плохо пахнущие (CODE SMELL) фрагменты, а заодно позволяет контролировать соблюдение соглашений по оформлению кода, именованию объектов и переменных.

Линтеры для SQL

$ — только платный вариант.
RIP — официально закрыт или в репозитории много лет нет активности.
SSDT — запуск через подключение к SQLPROJ-проекту в качестве анализатора, построенного на DacFx.
Src — проект выложен в Open Source.
Число правил — округлённое количество правил, которое поставляется с продуктом, и в скобках указано, сколько правил из общего числа посвящено именно T-SQL, а не ANSI SQL или другому диалекту.

Хотел добавить столбец «Habr» со ссылками на тематические статьи — думал найти приличные обзоры, но их не оказалось.

Если оставить активные проекты, у которых ≥ 50 правил, то остаётся 8 вариантов, из них 4 платные, 2 построены на движке SSDT и только у трёх открыты исходники. Выбор небогат, что-либо из представленного сложно назвать признаваемым сообществом T-SQL разработчиков стандартом. Ниже приведу некоторые особенности части названных инструментов.

ApexSql может работать не по исходникам, а по метаданным, системным таблицам на стороне инстанса SQL Server. Поставляется со встроенным редактором кастомных правил (я по-быстрому не разобрался, как пользоваться). Есть форматтер.

SonarSource прикручивается как плагин к SonarQube. На тормозящий экстеншен к VS жалобы сохраняются.

SqlFluff ориентирован скорее на форматирование и контроль его соблюдения. Зато поддерживает почти 30 диалектов SQL.

SqlServer.Rules пишет @ErikEJ, которого часто можно заметить в issues проекта DacFx.

tsqllint построен на ScriptDom, кроссплатформенный. Допускает подключение кастомных плагинов, построенных на той же концепции что и он сам.

TeamTools.Linter.TSQL — моё поделие, поэтому поместил в хвост таблицы, иначе получилось бы нескромно. Построен на ScriptDom, консольник поставляется с двумя плагинами: T-SQL и SSDT. В последнем реализованы несколько правил про конфигурирование SQLPROJ-проектов.

Утилиты от RedGate хоть официально и завершили свой жизненный путь, бинарники всё же возможно найти и пользоваться. Страницы с описанием этих инструментов пока доступны.

Выбор подхода

Более системным вариантом на первый взгляд кажется интеграция с SSDT. В нашем случае первые же эксперименты показали, что свои правила делать не особенно удобно, а подключать их к сборке нужно через простановку галочек в IDE. Распространять внутреннее решение или публичный анализатор пришлось бы вручную, непонятным образом раскладывая DLL в дебри Program Files, что дополнительно усложняется урезанными правами на машинах компании. То есть не ясно, в чём итоговая ценность подстраивания под особенности SSDT. Кроме прочего, хорошо бы иметь возможность пролинтить один конкретный SQL-файл или директорию с кучей таких файлов, не собирая их в SQLPROJ и не запуская билд проекта.

Стоит отметить, что пытающийся прийти на замену SSDT фреймворк в SDK-стиле не стоит на месте и некоторые проблемы хотя бы отчасти решены. Неожиданно для меня самого в SDK довольно оперативно затащили мой же фича-реквест про подтягивание анализаторов как нугет-пакетов (по аналогии с проектами .NET), чтобы забыть про чудовищный костыль с подкладыванием в Program Files, но решение не взлетело. Я к этому моменту мониторить состояние этого SDK уже почти прекратил, но подмечал, что ErikEJ один за одним открывает баг-репорты про то, что эта схема не работает. Обидно. Не я один надеялся однажды перевести все проекты команды на этот SDK. Тем не менее сохраняю надежду, что придёт время и SDK-style проекты полноценно заработают. Вариант работы не пофайлово, а с полной моделью проекта позволил бы добавить массу интересных правил в линтер. Видел статью коллеги с обзором разработки таких правил, для себя это направление развития окончательно не закрываю.

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

III Защита от CODE SMELL

На сегодня в линтере реализованы правила 17 категорий, среди которых:

• FA — как раз коллекция того, что абсолютно точно приведёт к деплой-тайм или рантайм ошибке, но гарантированно будет пропущено SSDT; там не так мало, полюбопытствуйте.

• Категория CD относится к проблематике регулярного развёртывания БД утилитой sqlpackage. Например, некоторые конструкции приводят к тому, что sqlpackage постоянно видит различие, хотя объект не меняется (вечный дифф), и пересоздаёт его. Для таблиц это может означать перезаливку всех данных через на лету созданную временную таблицу.

• В категориях DM и HD, в частности, собраны правила, обращающие внимание на то, что эксплуатация БД, её обслуживание и детальная настройка — не вопрос исходников. Это работа ДБА, администраторов инфраструктуры, и в исходниках проекта подобной информации делать нечего.

Для ин-мемори и нативно-компилируемых модулей действует немало ограничений, часть синтаксиса и функций для такого кода недоступны — есть правила и про это. Большинство примеров из начала статьи линтер успешно отлавливает. Правила под некоторые непокрытые сценарии запланированы на последующие обновления, остальное пока остаётся в заметках на полях.

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

CREATE PROCEDURE dbo.foo
    @i TINYINT
AS
BEGIN
    DECLARE @n INT = 100
    
    IF @i < 10
    BEGIN
        SET @i += @n
        
        IF @i > 200
            PRINT '> 200'

        SET @i = @i * 10
    END
    
    SELECT DATEFROMPARTS(2026, @i, 1)
END

В этом утрированном примере после первого IF линтер будет понимать, что @i находится в диапазоне 0-9 , после прибавления @n в диапазоне 100-109. Таким образом сравнение в следующем IF с 200 не имеет смысла, а присвоение с умножением на 10 приведёт к выходу за диапазон значений типа TINYINT. На вызове DATEFROMPARTS линтер отметит, что второй параметр, который отвечает за месяц, может принимать значения от 1 до 12, а переменная @i такого ограничения не имеет. Эффект от работы эвалюатора хорошо заметен при анализе сложных хранимок с запутанным ветвлением, где шанс упустить взаимосвязь операций из разных фрагментов кода весьма велик. Кроме целых чисел он умеет, например, проследить значение строки через несколько склеек из просчитываемых фрагментов или хотя бы оценить итоговую длину такой строки. Так можно обнаружить потенциальные неявные усечения, непреднамеренный переход к однобайтовой работе со строкой, которая могла содержать юникод.

Если объединить работоспособные примеры из статьи в один файл, то линтер покажет такие замечания за вычетом несущественных:

.\all_examples.sql(2,20): Warning CS0946 : Literal size is too big: 2 < 7
.\all_examples.sql(6,21): Error FA0947 : Literal is out of range: -1 is out of range for TINYINT
.\all_examples.sql(25,6): Warning AM0109 : Table reference not schema qualified: foo
.\all_examples.sql(30,27): Warning CS0946 : Literal size is too big: 5 < 10
.\all_examples.sql(35,27): Warning CS0792 : Non-unicode literal contains Unicode symbol
.\all_examples.sql(35,27): Warning CS0946 : Literal size is too big: 5 < 8
.\all_examples.sql(40,30): Error FA0113 : Target column count does not match source
.\all_examples.sql(43,12): Warning CS0195 : Cursor isn't declared as LOCAL: cr
.\all_examples.sql(49,1): Warning RD0983 : Variable is fetched from cursor but never used: @a
.\all_examples.sql(49,1): Warning RD0983 : Variable is fetched from cursor but never used: @b
.\all_examples.sql(49,1): Warning RD0983 : Variable is fetched from cursor but never used: @c
.\all_examples.sql(49,1): Warning RD0983 : Variable is fetched from cursor but never used: @d
.\all_examples.sql(49,1): Warning RD0983 : Variable is fetched from cursor but never used: @e
.\all_examples.sql(49,17): Error FA0124 : Fetched values count does not match cursor columns count
.\all_examples.sql(56,16): Warning AM0108 : Object creation with no schema defined
.\all_examples.sql(56,16): Warning NM0962 : Trigger name violates naming pattern: missing table name prefix
.\all_examples.sql(60,5): Error FA0119 : Illegal RETURN <value>
.\all_examples.sql(60,5): Warning PF0145 : NOCOUNT option not set in trigger
.\all_examples.sql(64,17): Warning AM0108 : Object creation with no schema defined
.\all_examples.sql(68,5): Error FA0131 : SET <option> is illegal in current context
.\all_examples.sql(78,9): Warning CS0843 : Output table is not affected by this statement: DELETED
.\all_examples.sql(88,19): Info CS0853 : Left part of comparison is similar to the right part
.\all_examples.sql(95,14): Warning CS0959 : Duplicate expression in conditional flow has no effect: see another occurance at line 94
.\all_examples.sql(110,1): Warning CS0521 : System procedure return value isn't checked: sp_add_job
.\all_examples.sql(119,1): Warning CS0521 : System procedure return value isn't checked: sp_add_jobstep
.\all_examples.sql(131,1): Warning CS0521 : System procedure return value isn't checked: sp_add_jobserver
.\all_examples.sql(136,1): Warning CS0521 : System procedure return value isn't checked: sp_add_schedule
.\all_examples.sql(144,1): Warning CS0521 : System procedure return value isn't checked: sp_attach_schedule
.\all_examples.sql(153,5): Error FA0178 : Parameters definition in sp_executesql does not match passed params
.\all_examples.sql(162,5): Warning PF0144 : NOCOUNT option not set in stored procedure
.\all_examples.sql(174,12): Warning MA0167 : Computed column missing alias

Поставка

Линтер распространяется в виде CLI-утилиты. Её можно дёргать локально, добавить в качестве одного из шагов в пайплайн сборки; есть флаг --diff, чтобы при анализе пулл-реквестов проверять только изменённые файлы. И есть экстеншен для Visual Studio с подсветкой замечаний и подсказками; в публичный store пока не выкладывал.

В readme приведены примеры подключения линтера к SourceTree, SSMS в качестве External tool и болванка pre-commit хука, чтобы узнавать об упущенных замечаниях, не дожидаясь сломанного билда.

IV Испытания

С абстрактными примерами всё понятно, но что этот линтер сможет найти в осмысленном и многократно проверенном коде. Энтерпрайз-решения, понятно, никто не выложит, тем не менее в открытых исходниках кое-что любопытное подобрать возможно.

FirstResponderKit

[src] [report]

.\Install-Azure.sql(22616,40): Error FA0947 : Literal is out of range: -1 is out of range for bit

Про invalid/false комментарий понятен, вот только переменная @OutputDatabaseCheck объявлена с типом BIT, который отрицательные значения хранить не умеет. В данном случае линтер слегка сгущает краски: ошибки в рантайме не будет и переменная сохранит значение 1 — но зачем же тогда попытка работать с отрицательным значением? -1 в переменной ни при каких обстоятельствах не окажется.

.\Install-All-Scripts.sql(6363,36): Warning RD0925 : Redundant LIKE without wildcards

Вокруг написаны корректные NOT LIKE с %, но вот этот wildcard не содержит и фактически здесь происходит сравнение на равенство. Предположу, что LIKE был оставлен для единообразия этого блока проверок. А может, и потерялся % с какого-то края литерала.

.\Install-All-Scripts.sql(6840,87): Warning AM0903 : Multiple output to the same variable

В одну и ту же переменную одним вызовом кладём и значение через OUTPUT-параметр, и код возврата, который придёт через RETURN. Сиквел-то разберётся, что из этого в итоге окажется в переменной, но программист на какой из вариантов рассчитывал? Одновременно два значения переменная не сохранит. Который вариант устраивает программиста, так и нужно бы переписать: собираем либо OUTPUT, либо RETURN.

.\Install-All-Scripts.sql(36550,5): Warning CS0960 : Unreliable type check: ISDATE

Предположу, что причина использования ISDATE в необходимости поддерживать совместимость этих скриптов с разными версиями СУБД: TRY_CONVERT в старых версиях сиквела отсутствует. Кроме того, это ведь служебный скрипт, а не бизнес-логика, поэтому вариант можно считать приемлемым. Но в общем случае функции ISDATE, ISNUMBER применять не стоит: из-за своих особенностей они могут вернуть true даже для значений, которые потом конвертнуть, соответственно, в дату или число не получится.

DarlingData

[src] [report]

.\Clear Token Perm\ClearTokenPerm.sql(126,1): Warning CS0732 : The GO command is missing at the end of the script
.\Clear Token Perm\ClearTokenPerm.sql(129,1): Warning CS0161 : Unreachable code

Тело хранимки вроде как завершено, поставлен последний END, но за ним не идёт GO, а сразу следующее выражение. Для сиквела это означает, что эти последующие выражения тоже являются частью хранимки (EXEC, SELECT, TRUNCATE и так до конца скрипта или ближайшего GO). Судьба ретурна непонятна, возможно он был задуман как последняя команда в процедуре, но сейчас он просто превращает код, следующий за ним, в unreachable code. Возможно, этот RETURN-шлагбаум как раз и прячет проблему лишних выражений в хп. Но ведь и скрипт в таком случае не выполняет поставленных задач. В кодовой базе реальных систем тоже попадаются хранимки без GO в конце, но там чаще всего гранты к телу хп приклеиваются.

.\sp_PressureDetector\sp_PressureDetector.sql(299,23): Warning FA0911 : Format argument count does not match wildcard count

Тоже распространённая промашка: число символов подстановки не совпадает с числом параметров для форматирования. RAISERROR отчасти работает как FORMATMESSAGE и необязательные параметры (которые после трёх обязательных) нужны как раз для построения строки. Но в самой строке ни одного символа подстановки нет, и что имелось в виду неясно. То ли %s, %d потерялся, то ли @what_to_check просто не нужен в этом вызове.

.\sp_QuickieStore\sp_QuickieStore.sql(6201,9): Warning CS0197 : The order of cursor handling commands is incorrect: Cursor '@filter_cursor' was not deallocated

Открытие курсора и FETCH есть, а вот CLOSE и DEALLOCATE отсутствуют, причём не только в этом файле. Небрежная работа с курсором приводит к тому, что выделенные под него ресурсы не освобождаются дольше, чем нужно. После выхода из хранимки локальный курсор неявно будет уничтожен, но здесь, к примеру, обратите внимание на номер строки — и до конца хп ещё 9 тысяч строк. Бросать подвешенные в воздухе курсоры не стоит. Кроме того, в коде этого репозитория ни один курсор не объявлен с атрибутом LOCAL. Таким образом, если в БД выставлен дефолтный уровень видимости GLOBAL, то курсоры будут утекать при каждом вызове. Также в объявлениях курсоров здесь не указано, что они FORWARD_ONLY, READ_ONLY, хотя де-факто это так — лучше указать, чтобы сиквел корректнее и с некоторыми оптимизациями организовал с этими курсорами работу. А приличный набор команд жизненного цикла курсора всегда полный: DECLARE-OPEN-FETCH-CLOSE-DEALLOCATE.

.\sp_QuickieCache\sp_QuickieCache.sql(713,20): Warning AM0114 : Ambiguous output of unordered TOP

Здесь без фрагмента кода, поскольку срабатываний было немало: зачем-то по всему коду разбросаны SELECT TOP 1 без сортировки. Не понятно, для чего нужны топы без конкретного и гарантированного порядка.

DBA MultiTool

[src] [report]

.\sp_help_revlogin.sql(99,50): Warning CS0111 : Length or precision and scale should be defined for data type: VARCHAR

К слову о том, что строки в SQL не то же, что в условном C#. После конвертации VARCHAR будет иметь вполне конкретную длину. Осталось выяснить, эта ли длина была нужна автору кода. Да, в представленном фрагменте устроит что попало, поскольку это техническая строка с неким штампом времени, но в общем случае для контролируемой конвертации DATETIME в строку нужно было и нужную длину указать, а вдобавок к этому и стиль конвертации. Когда нужна только дата, то конвертировать надо одним образом, а если вплоть до миллисекунд, то другим. Подобная небрежность допустима лишь в одноразовых утилитарных скриптах.

.\sp_helpme.sql(493,23): Warning CS0177 : Invalid error state value: -1

В рантайме ошибки не будет, однако ни severity, ни state отрицательные значения принимать не умеют. Фактически RAISERROR отработает с нулями для этих параметров. Так зачем здесь мистические -1? Возможно это рубрика «не знаю, всегда так делал».

.\sp_sizeoptimiser.sql(192,55): Warning RD0944 : [NOT] EXISTS is preferred over COUNT predicate: use NOT EXISTS

В названии процедуры упоминается некая оптимизация, однако внутри неё самой неоптимальных моментов хватает. Здесь проверка наличия хотя бы одной строки в таблице выполнена через подсчёт полного числа строк ней. Зачем, если достаточно найти хотя бы одну? EXISTS так и работает. Да, в приведённом фрагменте табличные переменные-коротыши, но, во-первых, для общего случая замечание существенное, во-вторых, переписать без изъяна ничего не стоит. И синтаксис [NOT] EXISTS доступен со времён царя гороха.

.\install_dba-multitool.sql(3216,24): Warning CS0196 : Cursor isn't declared explicitly as READONLY or FOR UPDATE: login_curs
.\install_dba-multitool.sql(3216,24): Warning CS0982 : Cursor should be defined as FORWARD_ONLY or FAST_FORWARD: login_curs

Общее место для рассмотренных репозиториев: курсор не помечен как LOCAL, FAST_FORWARD, хотя задуман именно таким. Про LOCAL выше привёл рассуждения, про значимость FAST_FORWARD нашлась хабрастатья. Опять же, все три репозитория предлагаю служебные скрипты и хранимки, которые в режиме хайлоада никогда выполняться не будут, тем не менее соблюсти базовые правила гигиены при оформлении хранимок, курсоров и прочего несложно.

В приведённых примерах репозиториев грубых ошибок не нашлось, но подозрительных, неоптимальных фрагментов и оплошностей обнаружилось немало. Даже с учётом особой направленности проанализированного кода, где многие практики из энтерпрайз-разработки неприменимы, а показатели качества неактуальны, всё же есть на что обратить внимание. При этом обвинять авторов в некомпетентности (Брента Озара, ага) было бы странно: объём кода в каждом из представленных случаев большой, одномоментно осознать его целиком и во всех подробностях невозможно. Потому и проскакивают разные мелочи. А пока код не настоялся, обкатался на проде — и вовсе не мелочи.

Как в поговорке про сапожника без сапог немало программистов, которые занимаются автоматизацией, чтобы ускорить бизнес-процессы и снизить вероятность ошибки из-за человеческого фактора, продолжают сами полагаться на чутьё и надеяться на верность руки.

Благодарности

Огромное спасибо @Galliot, @Snugly, @KazyuraVasiliy за код-ревью, подсказки и работу над документацией линтера. В частности, Snugly сделал болванку подсветки замечаний в VS, без него бы я не разобрался. Также благодарен руководителю за то, что помог преодолеть длительное согласование публикации в open source.

Авторы tsqllint очень помогли мне освоиться в ScriptDom живыми сценариями полноценного применения библиотеки, как и сам продукт этих ребят некоторое время помогал нашей команде начинать устанавливать контроль над разнобоем в кодировании. Open source рулит.

P.S.

Переизобретать велосипед дело увлекательное, так появился и наш статический анализатор, и SqlCover. Де-факто единственный фреймворк юнит-тестирования tSQLt — построен энтузиастами. SDK-style фреймворк для T-SQL некоторыми ходами чрезвычайно напоминает MSBuild.Sdk.SqlProj, который тоже родился внутри коллектива, не связанного с командой разработки SSDT. Но было бы вовсе здорово, если бы точечные проявления индивидуальных инициатив смогли когда-то сформировать разумную экосистему, которую можно было бы брать из коробки и пользоваться.

Напишите в комментарии, каким инструментом статического анализа T-SQL кода пользуетесь, чем он плох или хорош, чем форматируете. Интересно узнать, в какие этапы SDLC и как именно вы интегрировали анализ кода, метрики качества кода. Кто решил эту задачку не для сиквела, а для PostgreSql, всё равно напишите.

Скиньте примеры, что любопытного удалось обнаружить, если решились прогнать линтер на своих проектах.