fix(sql): renumber engineering_networks 85→87 + dedup classid + add SRID 4326 #118

Merged

lekss361 merged 2 commits from fix/sql-engineering-networks-87-renumber into main

2026-05-14 19:51:12 +00:00

lekss361 commented

2026-05-14 19:30:12 +00:00

(Migrated from github.com)

Summary

Three critical pre-deploy fixes on untracked data/sql/85_engineering_networks_191fz.sql (per code review audit May 14) — file is renumbered to 87_ and corrected before its first prod application.

NN collision — 85_pzz_zones_ekb.sql уже занимает 85_ prefix. Alphabetic deploy order применил бы 85_engineering_* ДО 85_pzz_*. Renumber → 87_ (no dep на 85_pzz, applies after 86_v_bucket_success_score.sql).
Duplicate classid 907015504 — был дважды: 'Опора металлическая (столб)' + 'Опора железобетонная (столб)'. Второй ON CONFLICT DO UPDATE молча перетёр бы первый → NSI код металлической опоры терялся бы навсегда. Fix: 907015504 ← металлическая (lower-numbered per Минстрой NSI), 907015507 ← железобетонная (заполняет gap 907015506 → 907015508). Comment-метка verify with Минстрой NSI table оставлена для pre-deploy verification.
Untyped geometry — geom GEOMETRY (no SRID constraint) разрешил бы insert mix MSK-66 (srid=100000) и WGS84 (srid=4326) → silent ST_DWithin corruption. Fix: geom GEOMETRY(Geometry, 4326). Capital Geometry allows mixed Point/LineString/Polygon, SRID 4326 жёстко фиксирует WGS84.

Why

Per code review critical #2 (May 14): without these three fixes, applying the migration would silently lose data (dup classid), break spatial queries (mixed SRID), and risk applying out-of-order against 85_pzz_zones_ekb.sql.

Pre-deploy checklist

Verify 907015507 classid assignment against official Минстрой NSI table — если на самом деле 907015507 имеет другую семантику, переназначить (см. Engineering_Networks_Schema_191FZ_May12.md в vault для контекста).
ETL loader transform check — любой ETL пишущий в engineering_networks должен делать ST_Transform(geom_msk66_raw, 4326) перед insert. См. backend/app/core/crs.py (untracked сейчас — отдельный PR).
Test apply на staging DB: psql -f data/sql/87_engineering_networks_191fz.sql — все CREATE/INSERT idempotent (BEGIN/COMMIT, IF NOT EXISTS, ON CONFLICT DO UPDATE).
GIST index on geom существует и валиден после SRID constraint (был на line 134, должен пережить ALTER без re-create).

Day-1 critical #2 from code review audit (May 14, 2026-05-14)
Vault: Engineering_Networks_Schema_191FZ_May12.md (24 feature layers + 73 справочника + NSI classifier)
Vault: Bug_NSPD_WMS_NotBulk_2026_May14.md (motivating scrape architecture for engineering networks)

## Summary Three critical pre-deploy fixes on untracked `data/sql/85_engineering_networks_191fz.sql` (per code review audit May 14) — file is renumbered to `87_` and corrected before its first prod application. 1. **NN collision** — `85_pzz_zones_ekb.sql` уже занимает 85_ prefix. Alphabetic deploy order применил бы `85_engineering_*` ДО `85_pzz_*`. Renumber → 87_ (no dep на 85_pzz, applies after `86_v_bucket_success_score.sql`). 2. **Duplicate classid `907015504`** — был дважды: `'Опора металлическая (столб)'` + `'Опора железобетонная (столб)'`. Второй `ON CONFLICT DO UPDATE` молча перетёр бы первый → NSI код металлической опоры терялся бы навсегда. **Fix:** `907015504` ← металлическая (lower-numbered per Минстрой NSI), `907015507` ← железобетонная (заполняет gap `907015506` → `907015508`). Comment-метка `verify with Минстрой NSI table` оставлена для pre-deploy verification. 3. **Untyped geometry** — `geom GEOMETRY` (no SRID constraint) разрешил бы insert mix MSK-66 (srid=100000) и WGS84 (srid=4326) → silent `ST_DWithin` corruption. **Fix:** `geom GEOMETRY(Geometry, 4326)`. Capital `Geometry` allows mixed Point/LineString/Polygon, SRID 4326 жёстко фиксирует WGS84. ## Why Per code review critical #2 (May 14): without these three fixes, applying the migration would silently lose data (dup classid), break spatial queries (mixed SRID), and risk applying out-of-order against `85_pzz_zones_ekb.sql`. ## Pre-deploy checklist - [ ] **Verify `907015507` classid assignment** against official Минстрой NSI table — если на самом деле `907015507` имеет другую семантику, переназначить (см. `Engineering_Networks_Schema_191FZ_May12.md` в vault для контекста). - [ ] **ETL loader transform check** — любой ETL пишущий в `engineering_networks` должен делать `ST_Transform(geom_msk66_raw, 4326)` перед insert. См. `backend/app/core/crs.py` (untracked сейчас — отдельный PR). - [ ] **Test apply** на staging DB: `psql -f data/sql/87_engineering_networks_191fz.sql` — все CREATE/INSERT idempotent (BEGIN/COMMIT, IF NOT EXISTS, ON CONFLICT DO UPDATE). - [ ] **GIST index on `geom`** существует и валиден после SRID constraint (был на line 134, должен пережить ALTER без re-create). ## Related - Day-1 critical #2 from code review audit (May 14, 2026-05-14) - Vault: `Engineering_Networks_Schema_191FZ_May12.md` (24 feature layers + 73 справочника + NSI classifier) - Vault: `Bug_NSPD_WMS_NotBulk_2026_May14.md` (motivating scrape architecture for engineering networks)

lekss361 commented

2026-05-14 19:42:49 +00:00

(Migrated from github.com)

Bot review (не аппрув)

SQL код корректен — BEGIN/COMMIT, idempotent (IF NOT EXISTS, ON CONFLICT DO UPDATE), ENUM создаются через DO $$ EXCEPTION WHEN duplicate_object. Три заявленных фикса (renumber 85→87, dedup 907015504, geom GEOMETRY(Geometry, 4326)) — все по делу.

Мерджить пока не могу — один блокер + один минор:

🔴 Блокер: `classid '907015507'` для "Опора железобетонная (столб)" провизорный

Inline-комментарий прямо говорит: assigned by review; 907015507 fills gap between 906 and 508; verify with Минстрой NSI table. Pre-deploy checklist пункт «Verify 907015507 classid assignment against official Минстрой NSI table» — unchecked.

Это canonical NSI-классификатор, который попадает в reference table через ON CONFLICT DO UPDATE name=EXCLUDED.name. Если у Минстроя 907015507 имеет другую семантику, после применения миграции мы навсегда перетрём правильное значение своим guess'ом, и все ETL-loaders начнут писать в engineering_networks.classid='907015507' с неверной маппой на "железобетонная опора".

Два варианта для разблока мерджа:

Подтвердить classid из официальной NSI-таблицы (Минстрой Свердловск или федеральная), убрать verify with... комментарий, отметить чеклист — re-review и аппрув.
Если нет доступа к официальной таблице сейчас — закомментировать строку с 907015507 (или поставить classid_name='Опора железобетонная (столб) [unmapped]' и не вставлять в eng_ref_classid через ON CONFLICT) + завести follow-up issue. Это позволяет смерджить корректные части PR без риска корраптить NSI lookup.

🟡 Минор (раз file ещё не применялся — самое время поправить)

Опечатка в seed строке 907015602: 'Фонати электрические (на столбах)' → должно быть 'Фонари'.

Прочие пункты pre-deploy checklist

ETL ST_Transform check, staging test apply, GIST validity — это deploy gate, не code review блокеры. Их разумно держать как pre-apply verification, но они не мешают мерджу самой миграции (она idempotent и не выполняется автоматически на CI).

Минор (не блокирующий)

Порядок INSERT в seed-блоке classid: 907015504 → 907015507 → 907015505 → 907015506 → 907015508 ломает alphabetic readability. Тривиально, но если будете править провизорный 507 — заодно отсортируйте численно.

После фикса 907015507 (или закомментирования) — пингани (push новый коммит), на следующей итерации перепроверю на новом SHA.

## Bot review (не аппрув) SQL код корректен — `BEGIN/COMMIT`, idempotent (`IF NOT EXISTS`, `ON CONFLICT DO UPDATE`), ENUM создаются через `DO $$ EXCEPTION WHEN duplicate_object`. Три заявленных фикса (renumber 85→87, dedup `907015504`, `geom GEOMETRY(Geometry, 4326)`) — все по делу. **Мерджить пока не могу** — один блокер + один минор: ### 🔴 Блокер: `classid '907015507'` для "Опора железобетонная (столб)" провизорный Inline-комментарий прямо говорит: `assigned by review; 907015507 fills gap between 906 and 508; verify with Минстрой NSI table`. Pre-deploy checklist пункт **«Verify 907015507 classid assignment against official Минстрой NSI table»** — unchecked. Это canonical NSI-классификатор, который попадает в reference table через `ON CONFLICT DO UPDATE name=EXCLUDED.name`. Если у Минстроя `907015507` имеет другую семантику, после применения миграции мы навсегда перетрём правильное значение своим guess'ом, и все ETL-loaders начнут писать в `engineering_networks.classid='907015507'` с неверной маппой на "железобетонная опора". **Два варианта для разблока мерджа:** 1. **Подтвердить classid из официальной NSI-таблицы** (Минстрой Свердловск или федеральная), убрать `verify with...` комментарий, отметить чеклист — re-review и аппрув. 2. **Если нет доступа к официальной таблице сейчас** — закомментировать строку с `907015507` (или поставить `classid_name='Опора железобетонная (столб) [unmapped]'` и не вставлять в `eng_ref_classid` через `ON CONFLICT`) + завести follow-up issue. Это позволяет смерджить корректные части PR без риска корраптить NSI lookup. ### 🟡 Минор (раз file ещё не применялся — самое время поправить) Опечатка в seed строке `907015602`: `'Фонати электрические (на столбах)'` → должно быть `'Фонари'`. ### Прочие пункты pre-deploy checklist ETL `ST_Transform` check, staging test apply, GIST validity — это deploy gate, не code review блокеры. Их разумно держать как pre-apply verification, но они не мешают мерджу самой миграции (она idempotent и не выполняется автоматически на CI). ### Минор (не блокирующий) Порядок INSERT в seed-блоке classid: `907015504 → 907015507 → 907015505 → 907015506 → 907015508` ломает alphabetic readability. Тривиально, но если будете править провизорный 507 — заодно отсортируйте численно. --- После фикса `907015507` (или закомментирования) — пингани (push новый коммит), на следующей итерации перепроверю на новом SHA.

lekss361 commented

2026-05-14 19:46:58 +00:00

(Migrated from github.com)

Bot re-review (SHA `3835be6`)

Оба замечания закрыты безупречно:

✅ 907015507 закомментирован с TODO(NSI-verify) — выбран безопасный вариант 2: classid удалён из seed-блока, в eng_ref_classid не попадёт, ON CONFLICT DO UPDATE не может перетереть правильное значение. TODO-комментарий чётко описывает, почему и когда возвращать строку. Численный порядок сохранён (504→505→506→507(comment)→508).

✅ Фонати → Фонари — поправлено на строке 907015602.

Жду CI — backend + frontend ещё IN_PROGRESS (запущены 19:45:51Z). Как только зелёные — мерджу squash.

(Не пишу маркер аппрува пока CI не подтвердит, чтобы не запутать loop на следующей итерации.)

## Bot re-review (SHA `3835be6`) Оба замечания закрыты безупречно: ✅ **`907015507` закомментирован с TODO(NSI-verify)** — выбран безопасный вариант 2: classid удалён из seed-блока, в `eng_ref_classid` не попадёт, `ON CONFLICT DO UPDATE` не может перетереть правильное значение. TODO-комментарий чётко описывает, почему и когда возвращать строку. Численный порядок сохранён (504→505→506→507(comment)→508). ✅ **`Фонати → Фонари`** — поправлено на строке `907015602`. **Жду CI** — backend + frontend ещё `IN_PROGRESS` (запущены 19:45:51Z). Как только зелёные — мерджу squash. (Не пишу маркер аппрува пока CI не подтвердит, чтобы не запутать loop на следующей итерации.)

lekss361 commented

2026-05-14 19:51:08 +00:00

(Migrated from github.com)

Bot final review (SHA `3835be6`, CI ✅)

Backend check: SUCCESS (19:47:12Z)
Frontend check: SUCCESS (19:47:34Z)
Все замечания из предыдущего ревью закрыты (907015507 commented out + TODO, Фонати→Фонари)

approve merge

## Bot final review (SHA `3835be6`, CI ✅) - Backend check: SUCCESS (19:47:12Z) - Frontend check: SUCCESS (19:47:34Z) - Все замечания из предыдущего ревью закрыты (`907015507` commented out + TODO, `Фонати→Фонари`) approve merge