fix(ptica): фильтр класса в velocity ссылался на алиас, которого нет в CTE
Some checks failed
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m18s
CI / backend-tests (pull_request) Failing after 16m26s
Some checks failed
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m18s
CI / backend-tests (pull_request) Failing after 16m26s
class_filter подставляется ВНУТРЬ latest_obj, где FROM — голый domrf_kn_objects, а алиас `o` появляется только во внешнем SELECT. Прод-EXPLAIN 13.08: `missing FROM-clause entry for table "o"`. Ветка мёртвая — единственный вызывающий (analyze_parcel) obj_class не передаёт, поэтому в проде это не стреляло. Стрельнуло бы тихо: исключение глотает except в compute_velocity, функция возвращает None, и блок velocity просто исчезает из отчёта с одной строкой в логе. SQL вынесен в модульную константу _COMPETITORS_SQL_TMPL — чтобы integration-тест мог прогнать EXPLAIN по ОБЕИМ подстановкам, а не только по той, что сегодня исполняется. Один хунк в тесте — не мой: pre-commit ruff v0.7.4 против 0.15.12 в venv (#2864). Refs #2464
This commit is contained in:
parent
9e83eb4a53
commit
e7d112b028
2 changed files with 94 additions and 45 deletions
|
|
@ -36,6 +36,48 @@ from sqlalchemy.orm import Session
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# Конкуренты в радиусе — модульная константа (а не inline f-string), чтобы
|
||||||
|
# integration-тест мог прогнать EXPLAIN по обеим подстановкам `{class_filter}`.
|
||||||
|
# Ветка с фильтром до #2464-G не парсилась вообще: ссылалась на алиас `o`,
|
||||||
|
# которого внутри CTE нет (`missing FROM-clause entry for table "o"`).
|
||||||
|
_COMPETITORS_SQL_TMPL = """
|
||||||
|
WITH latest_obj AS (
|
||||||
|
SELECT DISTINCT ON (obj_id)
|
||||||
|
obj_id,
|
||||||
|
comm_name,
|
||||||
|
dev_name,
|
||||||
|
-- #38: эффективный класс — реальный, иначе fallback
|
||||||
|
COALESCE(obj_class, obj_class_fallback) AS obj_class,
|
||||||
|
latitude,
|
||||||
|
longitude,
|
||||||
|
district_name
|
||||||
|
FROM domrf_kn_objects
|
||||||
|
WHERE latitude IS NOT NULL
|
||||||
|
AND longitude IS NOT NULL
|
||||||
|
AND region_cd = 66
|
||||||
|
{class_filter}
|
||||||
|
ORDER BY obj_id, snapshot_date DESC NULLS LAST
|
||||||
|
)
|
||||||
|
SELECT
|
||||||
|
o.obj_id,
|
||||||
|
o.comm_name,
|
||||||
|
o.dev_name,
|
||||||
|
o.obj_class,
|
||||||
|
o.district_name,
|
||||||
|
ST_Distance(
|
||||||
|
ST_SetSRID(ST_MakePoint(o.longitude, o.latitude), 4326)::geography,
|
||||||
|
ST_Centroid(ST_GeomFromText(:parcel_wkt, 4326))::geography
|
||||||
|
) AS distance_m
|
||||||
|
FROM latest_obj o
|
||||||
|
WHERE ST_DWithin(
|
||||||
|
ST_SetSRID(ST_MakePoint(o.longitude, o.latitude), 4326)::geography,
|
||||||
|
ST_Centroid(ST_GeomFromText(:parcel_wkt, 4326))::geography,
|
||||||
|
:radius_m
|
||||||
|
)
|
||||||
|
ORDER BY distance_m ASC
|
||||||
|
LIMIT 200
|
||||||
|
"""
|
||||||
|
|
||||||
# Fallback если в БД нет данных за окно months_window (DB-error / пустой _get_ekb_median).
|
# Fallback если в БД нет данных за окно months_window (DB-error / пустой _get_ekb_median).
|
||||||
# Источник (audit #1871): реальная медиана monthly velocity по ЕКБ — 593-766 м²/мес на
|
# Источник (audit #1871): реальная медиана monthly velocity по ЕКБ — 593-766 м²/мес на
|
||||||
# один ЖК. Берём верхнюю границу 750.0 — консервативно (безопаснее переоценки рынка:
|
# один ЖК. Берём верхнюю границу 750.0 — консервативно (безопаснее переоценки рынка:
|
||||||
|
|
@ -173,9 +215,17 @@ def compute_velocity(
|
||||||
# только если явно передан. #38: при NULL реального класса используем
|
# только если явно передан. #38: при NULL реального класса используем
|
||||||
# obj_class_fallback (yandex_match / price_inference) — реальный obj_class
|
# obj_class_fallback (yandex_match / price_inference) — реальный obj_class
|
||||||
# в приоритете (COALESCE), поведение для размеченных ЖК не меняется.
|
# в приоритете (COALESCE), поведение для размеченных ЖК не меняется.
|
||||||
class_filter = (
|
# Колонки БЕЗ алиаса: фильтр подставляется ВНУТРЬ latest_obj, где FROM —
|
||||||
"AND COALESCE(o.obj_class, o.obj_class_fallback) = :obj_class" if obj_class else ""
|
# голый domrf_kn_objects. Алиас `o` появляется только во внешнем SELECT,
|
||||||
)
|
# и `o.obj_class` здесь давал `missing FROM-clause entry for table "o"`
|
||||||
|
# (#2464-G, прод-EXPLAIN 13.08). Ошибку глотал except ниже → velocity
|
||||||
|
# молча выпадал из отчёта. Не срабатывало только потому, что единственный
|
||||||
|
# вызывающий (parcels.py) obj_class не передаёт.
|
||||||
|
# NB для первого, кто ветку включит: сравнение точное и регистрозависимое, а
|
||||||
|
# в проде классы с большой буквы и словарь шире ожидаемого — «Комфорт» 870,
|
||||||
|
# «Типовой» 224, «Бизнес» 95, «Премиум» 13, «Элит» 12, «Стандарт» 9,
|
||||||
|
# «Элитный» 4 объекта (замер 13.08). Передавать нужно ровно эти строки.
|
||||||
|
class_filter = "AND COALESCE(obj_class, obj_class_fallback) = :obj_class" if obj_class else ""
|
||||||
# SAVEPOINT per query: failure rollbacks ТОЛЬКО savepoint, не outer tx.
|
# SAVEPOINT per query: failure rollbacks ТОЛЬКО savepoint, не outer tx.
|
||||||
# db.rollback() здесь НЕЛЬЗЯ — он orphan'ит outer SessionTransaction
|
# db.rollback() здесь НЕЛЬЗЯ — он orphan'ит outer SessionTransaction
|
||||||
# (см. PR #155 bot review — SQLAlchemy 2.0 begin_nested context cleanup).
|
# (см. PR #155 bot review — SQLAlchemy 2.0 begin_nested context cleanup).
|
||||||
|
|
@ -183,45 +233,7 @@ def compute_velocity(
|
||||||
with db.begin_nested():
|
with db.begin_nested():
|
||||||
comp_rows = (
|
comp_rows = (
|
||||||
db.execute(
|
db.execute(
|
||||||
text(
|
text(_COMPETITORS_SQL_TMPL.format(class_filter=class_filter)),
|
||||||
f"""
|
|
||||||
WITH latest_obj AS (
|
|
||||||
SELECT DISTINCT ON (obj_id)
|
|
||||||
obj_id,
|
|
||||||
comm_name,
|
|
||||||
dev_name,
|
|
||||||
-- #38: эффективный класс — реальный, иначе fallback
|
|
||||||
COALESCE(obj_class, obj_class_fallback) AS obj_class,
|
|
||||||
latitude,
|
|
||||||
longitude,
|
|
||||||
district_name
|
|
||||||
FROM domrf_kn_objects
|
|
||||||
WHERE latitude IS NOT NULL
|
|
||||||
AND longitude IS NOT NULL
|
|
||||||
AND region_cd = 66
|
|
||||||
{class_filter}
|
|
||||||
ORDER BY obj_id, snapshot_date DESC NULLS LAST
|
|
||||||
)
|
|
||||||
SELECT
|
|
||||||
o.obj_id,
|
|
||||||
o.comm_name,
|
|
||||||
o.dev_name,
|
|
||||||
o.obj_class,
|
|
||||||
o.district_name,
|
|
||||||
ST_Distance(
|
|
||||||
ST_SetSRID(ST_MakePoint(o.longitude, o.latitude), 4326)::geography,
|
|
||||||
ST_Centroid(ST_GeomFromText(:parcel_wkt, 4326))::geography
|
|
||||||
) AS distance_m
|
|
||||||
FROM latest_obj o
|
|
||||||
WHERE ST_DWithin(
|
|
||||||
ST_SetSRID(ST_MakePoint(o.longitude, o.latitude), 4326)::geography,
|
|
||||||
ST_Centroid(ST_GeomFromText(:parcel_wkt, 4326))::geography,
|
|
||||||
:radius_m
|
|
||||||
)
|
|
||||||
ORDER BY distance_m ASC
|
|
||||||
LIMIT 200
|
|
||||||
"""
|
|
||||||
),
|
|
||||||
{
|
{
|
||||||
"parcel_wkt": parcel_geom_wkt,
|
"parcel_wkt": parcel_geom_wkt,
|
||||||
"radius_m": radius_km * 1000.0,
|
"radius_m": radius_km * 1000.0,
|
||||||
|
|
|
||||||
|
|
@ -39,6 +39,7 @@ from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app.api.v1.parcels import _NEIGHBORS_SUMMARY_SQL
|
from app.api.v1.parcels import _NEIGHBORS_SUMMARY_SQL
|
||||||
from app.services.site_finder.ird_overlay_lookup import _IRD_OVERLAP_SQL
|
from app.services.site_finder.ird_overlay_lookup import _IRD_OVERLAP_SQL
|
||||||
|
from app.services.site_finder.velocity import _COMPETITORS_SQL_TMPL
|
||||||
from tests.integration.conftest import requires_test_db
|
from tests.integration.conftest import requires_test_db
|
||||||
|
|
||||||
# NB: ``pytestmark`` НЕ ставим на модуль — здесь два класса compile-time
|
# NB: ``pytestmark`` НЕ ставим на модуль — здесь два класса compile-time
|
||||||
|
|
@ -103,9 +104,9 @@ class TestNeighborsSummarySql:
|
||||||
for kw in forbidden_aliases:
|
for kw in forbidden_aliases:
|
||||||
# ищем паттерн ``WITH <kw> AS (`` или ``, <kw> AS (`` — оба
|
# ищем паттерн ``WITH <kw> AS (`` или ``, <kw> AS (`` — оба
|
||||||
# формы CTE-биндинга.
|
# формы CTE-биндинга.
|
||||||
assert f"with {kw} as (" not in raw_sql and f", {kw} as (" not in raw_sql, (
|
assert (
|
||||||
f"CTE alias '{kw}' пересекается с PG keyword (см. incident #1195)"
|
f"with {kw} as (" not in raw_sql and f", {kw} as (" not in raw_sql
|
||||||
)
|
), f"CTE alias '{kw}' пересекается с PG keyword (см. incident #1195)"
|
||||||
|
|
||||||
|
|
||||||
# ── parcel_ird_overlaps SQL ──────────────────────────────────────────────────
|
# ── parcel_ird_overlaps SQL ──────────────────────────────────────────────────
|
||||||
|
|
@ -167,3 +168,39 @@ class TestPsycopg3CastAntipattern:
|
||||||
f"{name} содержит psycopg v3 antipattern: {matches}. "
|
f"{name} содержит psycopg v3 antipattern: {matches}. "
|
||||||
f"Используй CAST(:bind AS type) — см. .claude/rules/backend.md."
|
f"Используй CAST(:bind AS type) — см. .claude/rules/backend.md."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── velocity: конкуренты в радиусе (#2464-G) ─────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
class TestVelocityCompetitorsSql:
|
||||||
|
"""``_COMPETITORS_SQL_TMPL`` из ``app.services.site_finder.velocity``.
|
||||||
|
|
||||||
|
Шаблон подставляется в двух видах, и **вторая подстановка до #2464-G
|
||||||
|
не парсилась вообще**: фильтр класса ссылался на алиас ``o``, который
|
||||||
|
существует только во внешнем SELECT, а подставляется фильтр ВНУТРЬ CTE
|
||||||
|
``latest_obj`` (FROM domrf_kn_objects, без алиаса) →
|
||||||
|
``missing FROM-clause entry for table "o"`` (прод-EXPLAIN 13.08).
|
||||||
|
|
||||||
|
Почему это не падало в проде: единственный вызывающий
|
||||||
|
(``analyze_parcel``) ``obj_class`` не передаёт → ветка мёртвая.
|
||||||
|
Падало бы молча — исключение глотает ``except`` в ``compute_velocity``,
|
||||||
|
и блок velocity просто исчезал бы из отчёта с одной строкой в логе.
|
||||||
|
|
||||||
|
Тест закрывает обе ветки, а не только ту, что сегодня исполняется.
|
||||||
|
"""
|
||||||
|
|
||||||
|
@requires_test_db
|
||||||
|
@pytest.mark.integration
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"class_filter",
|
||||||
|
["", "AND COALESCE(obj_class, obj_class_fallback) = :obj_class"],
|
||||||
|
ids=["no_class_filter", "with_class_filter"],
|
||||||
|
)
|
||||||
|
def test_explain_competitors(self, phantom_check_session: Session, class_filter: str) -> None:
|
||||||
|
"""Обе подстановки шаблона парсятся и планируются против реальной схемы."""
|
||||||
|
_explain_text(
|
||||||
|
phantom_check_session,
|
||||||
|
_COMPETITORS_SQL_TMPL.format(class_filter=class_filter),
|
||||||
|
{"parcel_wkt": _EKB_WKT, "radius_m": 3000.0, "obj_class": "комфорт"},
|
||||||
|
)
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue