fix(tradein/dedup): ФИАС-проход схлопывания домов больше не сливает дома за сотни километров (#2690)
ФИАС-проход house_dedup_merge шёл без гео-стража (#2187: «общий ФИАС-UUID и есть здание»). Посылка неверна: house_fias_id — ответ DaData на НАШУ строку адреса, и «улица Кирова, 4» без города получает тот же UUID, что и дом в другом городе. Прогон 05.09 (house_merge_log, merge_pass='fias'): 408 слияний, 129 дальше 250 м, 53 дальше 3 км, максимум 362 км, у 67 нет координат на одной из сторон. Теперь у обоих проходов один и тот же страж (250 м, координаты у обеих сторон), параметр apply_geo_guard удалён — асимметрию нельзя вернуть флагом. Тест по значению на живой Postgres: тот же ФИАС в 5 км и с пустыми координатами не сливается, тот же ФИАС в 10 м сливается, журнал пишет geo_guard=true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
34642e1dd5
commit
ceb2e5972d
3 changed files with 93 additions and 137 deletions
|
|
@ -25,14 +25,17 @@ WHAT this is:
|
||||||
«улица Вайнера, 66» share one cluster_key. Rows whose canon is NULL/blank are never clustered
|
«улица Вайнера, 66» share one cluster_key. Rows whose canon is NULL/blank are never clustered
|
||||||
(cluster_key NULL → ignored). Only canons shared by >1 house_id form a cluster.
|
(cluster_key NULL → ignored). Only canons shared by >1 house_id form a cluster.
|
||||||
|
|
||||||
GEO GUARD (anti over-merge, CANON PASS ONLY — #2187): because the canon strips город/район,
|
GEO GUARD (anti over-merge, BOTH PASSES — #2690): because the canon strips город/район,
|
||||||
two different buildings with the same street+number in different region-66 towns (e.g. «Ленина
|
two different buildings with the same street+number in different region-66 towns (e.g. «Ленина
|
||||||
5») would share a canon. To avoid merging them, within a canon-cluster a house is a LOSER only
|
5») would share a canon. To avoid merging them, within a cluster a house is a LOSER only
|
||||||
if it is within 250 m of the keeper (ST_DistanceSphere on the WGS84 geom). Same-canon houses
|
if it is within 250 m of the keeper (ST_DistanceSphere on the WGS84 geom). Same-key houses
|
||||||
>250 m away — or with NULL geom on either side — are left as separate rows (conservative). The
|
>250 m away — or with NULL geom on either side — are left as separate rows (conservative).
|
||||||
FIAS pass deliberately SKIPS this guard: a shared ФИАС/ГАР UUID IS the building identity and
|
The FIAS pass used to skip this guard (#2187, «a shared ФИАС UUID IS the building identity»).
|
||||||
strictly outranks geo-proximity, so same-fias rows merge even with NULL geom on a side or
|
That premise is false: house_fias_id is DaData's answer to OUR address string (KEY section
|
||||||
>250 m apart (the geom-first keeper rule simultaneously repairs the broken coordinate).
|
below), so a street+number without a town gets the same UUID as the same street+number in
|
||||||
|
another town. Prod run 2026-09-05 (house_merge_log, merge_pass='fias'): 408 merges, 129 of
|
||||||
|
them >250 m apart, 53 >3 km, worst 362 km («р-н Синарский, улица Кирова, 4» of
|
||||||
|
Каменск-Уральский into Екатеринбург), plus 67 with no coordinates on a side.
|
||||||
|
|
||||||
TWO PASSES (2026-07-02 follow-up): the SAME cluster→keeper→re-point→carry-identity→delete
|
TWO PASSES (2026-07-02 follow-up): the SAME cluster→keeper→re-point→carry-identity→delete
|
||||||
pipeline now runs TWICE inside one transaction, parametrised by the cluster-key expression
|
pipeline now runs TWICE inside one transaction, parametrised by the cluster-key expression
|
||||||
|
|
@ -45,10 +48,8 @@ TWO PASSES (2026-07-02 follow-up): the SAME cluster→keeper→re-point→carry-
|
||||||
2. CANON pass — the canonical-address clustering above, now with a CROSS-FIAS GUARD: within a
|
2. CANON pass — the canonical-address clustering above, now with a CROSS-FIAS GUARD: within a
|
||||||
canon cluster a loser is NOT merged when it AND the keeper both carry a non-null but
|
canon cluster a loser is NOT merged when it AND the keeper both carry a non-null but
|
||||||
DIFFERENT house_fias_id — provably different buildings the canon collapsed (the
|
DIFFERENT house_fias_id — provably different buildings the canon collapsed (the
|
||||||
slash-collapse class). Critical anti-over-merge fix. The 250 m geo guard applies to the
|
slash-collapse class). Critical anti-over-merge fix. The 250 m geo guard applies to
|
||||||
CANON pass ONLY (#2187): the FIAS pass merges on UUID identity regardless of geom — a
|
BOTH passes (#2690; the fias-pass exemption of #2187 is reverted — see GEO GUARD).
|
||||||
боевой прогон left 130 same-fias groups (335 houses, 1 529 listings) split because the
|
|
||||||
guard blocked them (NULL geom on a side, or >250 m from a broken coordinate).
|
|
||||||
|
|
||||||
IDENTITY CARRY-OVER (each pass, BEFORE deleting losers): the keeper's NULL identity / geo-QC
|
IDENTITY CARRY-OVER (each pass, BEFORE deleting losers): the keeper's NULL identity / geo-QC
|
||||||
fields are filled from its losers with COALESCE semantics (keeper value wins; donor = the
|
fields are filled from its losers with COALESCE semantics (keeper value wins; donor = the
|
||||||
|
|
@ -114,9 +115,9 @@ MERGE JOURNAL — the merge is REVERSIBLE (#2690, migration 230):
|
||||||
keeper rule and no guard. It only makes whatever the pass decides reversible — which is the
|
keeper rule and no guard. It only makes whatever the pass decides reversible — which is the
|
||||||
precondition for revisiting those decisions at all (#2690, #1772).
|
precondition for revisiting those decisions at all (#2690, #1772).
|
||||||
|
|
||||||
distance_m is recorded on BOTH passes, including the fias pass whose geo guard is off. That
|
distance_m is recorded on BOTH passes. It is what exposed the fias pass merging without a
|
||||||
asymmetry — merge allowed without a proximity check — was invisible in data before; now
|
proximity check (#2690, 2026-09-05): «how many merges happened beyond N metres, on which key»
|
||||||
«how many merges happened beyond N metres, on which key» is one query.
|
is one query.
|
||||||
|
|
||||||
KEY — there is no second, address-independent observation. Measured on prod 2026-08-10 (#2690):
|
KEY — there is no second, address-independent observation. Measured on prod 2026-08-10 (#2690):
|
||||||
#2690 asked for a cluster key that does not come from the normalized address, so that two
|
#2690 asked for a cluster key that does not come from the normalized address, so that two
|
||||||
|
|
@ -311,35 +312,15 @@ def _ranked_cte(cluster_key_case: str) -> str:
|
||||||
)"""
|
)"""
|
||||||
|
|
||||||
|
|
||||||
def _mapping_sql(cluster_key_case: str, *, apply_geo_guard: bool = True) -> str:
|
def _mapping_sql(cluster_key_case: str) -> str:
|
||||||
"""Render the loser→keeper mapping SQL for one pass, given its cluster-key CASE expression.
|
"""Render the loser→keeper mapping SQL for one pass, given its cluster-key CASE expression.
|
||||||
|
|
||||||
Only cluster keys shared by >1 house_id form a cluster; the keeper is rn=1 per cluster, losers
|
Only cluster keys shared by >1 house_id form a cluster; the keeper is rn=1 per cluster, losers
|
||||||
are rn>1. The CROSS-FIAS guard always applies (a no-op for the fias pass, where every clustered
|
are rn>1. The CROSS-FIAS guard always applies (a no-op for the fias pass, where every clustered
|
||||||
row shares one fias by construction).
|
row shares one fias by construction). So does the 250 m GEO GUARD — on BOTH passes (#2690):
|
||||||
|
neither key is independent of our address string, so neither may merge what proximity rejects.
|
||||||
apply_geo_guard (#2187): the 250 m ST_DistanceSphere guard is emitted ONLY when True.
|
|
||||||
- CANON pass → True: the canon strips город/район, so same-street-number buildings in
|
|
||||||
different region-66 towns share a canon; the guard stops the cross-town over-merge.
|
|
||||||
- FIAS pass → False: a shared ФИАС/ГАР UUID IS the building identity and strictly outranks
|
|
||||||
proximity, so same-fias rows merge even with NULL geom on a side or >250 m apart (the
|
|
||||||
geom-first keeper rule simultaneously repairs the broken coordinate).
|
|
||||||
`cluster_key_case` is a STATIC module constant (never runtime data) — no value injection.
|
`cluster_key_case` is a STATIC module constant (never runtime data) — no value injection.
|
||||||
"""
|
"""
|
||||||
geo_guard = (
|
|
||||||
"""
|
|
||||||
-- GEO GUARD (canon pass only — #2187). tradein_canon_addr strips город/район, so two
|
|
||||||
-- different buildings sharing a street+number canon («Ленина 5» in different region-66
|
|
||||||
-- towns) collapse to one cluster_key. A loser merges only when geographically next to the
|
|
||||||
-- keeper (<=250 m — covers one building's geocode spread, prod: Мраморская 34к4 dupes at
|
|
||||||
-- 222 m; region-66 towns are km+ apart → 250 m is safe from cross-town). >250 m, or NULL
|
|
||||||
-- geom on either side, → left as separate rows (conservative — never over-merges).
|
|
||||||
AND keeper_geom IS NOT NULL
|
|
||||||
AND loser_geom IS NOT NULL
|
|
||||||
AND ST_DistanceSphere(loser_geom, keeper_geom) <= 250"""
|
|
||||||
if apply_geo_guard
|
|
||||||
else ""
|
|
||||||
)
|
|
||||||
return f"""
|
return f"""
|
||||||
CREATE TEMP TABLE _1772_dup_mapping ON COMMIT DROP AS
|
CREATE TEMP TABLE _1772_dup_mapping ON COMMIT DROP AS
|
||||||
{_ranked_cte(cluster_key_case)}
|
{_ranked_cte(cluster_key_case)}
|
||||||
|
|
@ -350,16 +331,22 @@ def _mapping_sql(cluster_key_case: str, *, apply_geo_guard: bool = True) -> str:
|
||||||
--
|
--
|
||||||
-- cluster_key / distance_m are carried out of the mapping for the MERGE JOURNAL (#2690):
|
-- cluster_key / distance_m are carried out of the mapping for the MERGE JOURNAL (#2690):
|
||||||
-- cluster_key records WHICH key value fired, distance_m how far apart the two rows were.
|
-- cluster_key records WHICH key value fired, distance_m how far apart the two rows were.
|
||||||
-- distance_m is computed even when the geo guard is OFF for this pass — that is precisely
|
-- distance_m is what showed the fias pass merging 362 km apart while it had no guard (#2690).
|
||||||
-- the case where nothing else records the distance, and #2690 had no way to ask
|
|
||||||
-- «how many merges happened at distances the guard would have blocked» from data.
|
|
||||||
SELECT id AS loser_id, keeper_id, norm_address, cluster_key,
|
SELECT id AS loser_id, keeper_id, norm_address, cluster_key,
|
||||||
CASE WHEN keeper_geom IS NOT NULL AND loser_geom IS NOT NULL
|
CASE WHEN keeper_geom IS NOT NULL AND loser_geom IS NOT NULL
|
||||||
THEN ST_DistanceSphere(loser_geom, keeper_geom)
|
THEN ST_DistanceSphere(loser_geom, keeper_geom)
|
||||||
END AS distance_m
|
END AS distance_m
|
||||||
FROM ranked
|
FROM ranked
|
||||||
WHERE rn > 1
|
WHERE rn > 1
|
||||||
AND id <> keeper_id{geo_guard}
|
AND id <> keeper_id
|
||||||
|
-- GEO GUARD (both passes — #2690). Neither cluster key tells towns apart: the canon strips
|
||||||
|
-- город/район, and house_fias_id is DaData's answer to that same town-less address. A loser
|
||||||
|
-- merges only when geographically next to the keeper (<=250 m — covers one building's
|
||||||
|
-- geocode spread, prod: Мраморская 34к4 dupes at 222 m; region-66 towns are km+ apart).
|
||||||
|
-- >250 m, or NULL geom on either side, → left as separate rows (conservative).
|
||||||
|
AND keeper_geom IS NOT NULL
|
||||||
|
AND loser_geom IS NOT NULL
|
||||||
|
AND ST_DistanceSphere(loser_geom, keeper_geom) <= 250
|
||||||
AND NOT (
|
AND NOT (
|
||||||
NULLIF(loser_fias, '') IS NOT NULL
|
NULLIF(loser_fias, '') IS NOT NULL
|
||||||
AND NULLIF(keeper_fias, '') IS NOT NULL
|
AND NULLIF(keeper_fias, '') IS NOT NULL
|
||||||
|
|
@ -368,12 +355,10 @@ def _mapping_sql(cluster_key_case: str, *, apply_geo_guard: bool = True) -> str:
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
# Canon-pass mapping — geo guard ON (cross-town over-merge protection for street+number canons).
|
# Canon-pass mapping.
|
||||||
_BUILD_MAPPING_SQL = text(_mapping_sql(_CANON_KEY_EXPR))
|
_BUILD_MAPPING_SQL = text(_mapping_sql(_CANON_KEY_EXPR))
|
||||||
# Fias-pass mapping — same pipeline, clustered by the ФИАС building UUID (runs first). Geo guard
|
# Fias-pass mapping — same pipeline and same guards, clustered by the ФИАС UUID (runs first).
|
||||||
# OFF (#2187): a shared ГАР UUID IS the building identity and outranks proximity — same-fias rows
|
_BUILD_MAPPING_SQL_FIAS = text(_mapping_sql(_FIAS_KEY_EXPR))
|
||||||
# merge even with NULL geom or >250 m apart (the geom-first keeper rule fixes broken coords).
|
|
||||||
_BUILD_MAPPING_SQL_FIAS = text(_mapping_sql(_FIAS_KEY_EXPR, apply_geo_guard=False))
|
|
||||||
|
|
||||||
# ── RESIDUAL CENSUS (#2690 п.2/п.4) ───────────────────────────────────────────
|
# ── RESIDUAL CENSUS (#2690 п.2/п.4) ───────────────────────────────────────────
|
||||||
#
|
#
|
||||||
|
|
@ -870,7 +855,6 @@ def _run_merge_pass(
|
||||||
*,
|
*,
|
||||||
build_sql: Any,
|
build_sql: Any,
|
||||||
pass_label: str,
|
pass_label: str,
|
||||||
geo_guard: bool,
|
|
||||||
batch_id: str,
|
batch_id: str,
|
||||||
run_id: int | None,
|
run_id: int | None,
|
||||||
initiator: str,
|
initiator: str,
|
||||||
|
|
@ -911,7 +895,7 @@ def _run_merge_pass(
|
||||||
"run_id": run_id,
|
"run_id": run_id,
|
||||||
"initiator": initiator,
|
"initiator": initiator,
|
||||||
"merge_pass": pass_label,
|
"merge_pass": pass_label,
|
||||||
"geo_guard": geo_guard,
|
"geo_guard": True, # both passes are guarded since #2690
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -1032,8 +1016,8 @@ def merge_duplicate_houses(
|
||||||
Re-implements migration 108's proven collision-safe pipeline as a RECURRING TWO-PASS job:
|
Re-implements migration 108's proven collision-safe pipeline as a RECURRING TWO-PASS job:
|
||||||
1. FIAS pass — cluster by lower(NULLIF(house_fias_id, '')) (catches slash-collapse /
|
1. FIAS pass — cluster by lower(NULLIF(house_fias_id, '')) (catches slash-collapse /
|
||||||
посёлок canon bugs the address canon misses).
|
посёлок canon bugs the address canon misses).
|
||||||
2. CANON pass — cluster by canonical address (250 m geo guard + cross-fias anti-over-merge
|
2. CANON pass — cluster by canonical address.
|
||||||
guard).
|
Both passes carry the 250 m geo guard and the cross-fias anti-over-merge guard.
|
||||||
Each pass: pick keeper → re-point children (UNIQUE-collision-safe) → carry identity onto the
|
Each pass: pick keeper → re-point children (UNIQUE-collision-safe) → carry identity onto the
|
||||||
keeper → delete losers → backfill sources/aliases. BOTH passes run in ONE transaction.
|
keeper → delete losers → backfill sources/aliases. BOTH passes run in ONE transaction.
|
||||||
dry_run=True computes counts then ROLLS BACK (no writes). Idempotent: a clean table yields an
|
dry_run=True computes counts then ROLLS BACK (no writes). Idempotent: a clean table yields an
|
||||||
|
|
@ -1057,7 +1041,6 @@ def merge_duplicate_houses(
|
||||||
db,
|
db,
|
||||||
build_sql=_BUILD_MAPPING_SQL_FIAS,
|
build_sql=_BUILD_MAPPING_SQL_FIAS,
|
||||||
pass_label="fias",
|
pass_label="fias",
|
||||||
geo_guard=False,
|
|
||||||
batch_id=batch_id,
|
batch_id=batch_id,
|
||||||
run_id=run_id,
|
run_id=run_id,
|
||||||
initiator=initiator,
|
initiator=initiator,
|
||||||
|
|
@ -1068,7 +1051,6 @@ def merge_duplicate_houses(
|
||||||
db,
|
db,
|
||||||
build_sql=_BUILD_MAPPING_SQL,
|
build_sql=_BUILD_MAPPING_SQL,
|
||||||
pass_label="canon",
|
pass_label="canon",
|
||||||
geo_guard=True,
|
|
||||||
batch_id=batch_id,
|
batch_id=batch_id,
|
||||||
run_id=run_id,
|
run_id=run_id,
|
||||||
initiator=initiator,
|
initiator=initiator,
|
||||||
|
|
|
||||||
|
|
@ -35,7 +35,7 @@ tests/test_gar_flats_loader.py::test_upsert_and_canon_match_populates_gar_flat_c
|
||||||
tests/test_gar_flats_loader.py::test_no_city_filter_ambiguous_canon_not_matched
|
tests/test_gar_flats_loader.py::test_no_city_filter_ambiguous_canon_not_matched
|
||||||
tests/test_house_dedup_merge.py::test_real_canon_clusterkey_and_geo_guard_merge_semantics
|
tests/test_house_dedup_merge.py::test_real_canon_clusterkey_and_geo_guard_merge_semantics
|
||||||
tests/test_house_dedup_merge.py::test_real_fias_pass_cross_guard_and_identity_carryover
|
tests/test_house_dedup_merge.py::test_real_fias_pass_cross_guard_and_identity_carryover
|
||||||
tests/test_house_dedup_merge.py::test_real_fias_pass_ignores_geo_guard
|
tests/test_house_dedup_merge.py::test_real_fias_pass_keeps_geo_guard
|
||||||
tests/test_house_dedup_merge.py::test_real_merge_is_reversible_via_journal
|
tests/test_house_dedup_merge.py::test_real_merge_is_reversible_via_journal
|
||||||
tests/test_house_dedup_merge.py::test_real_merge_repoints_dedups_deletes_and_is_idempotent
|
tests/test_house_dedup_merge.py::test_real_merge_repoints_dedups_deletes_and_is_idempotent
|
||||||
tests/test_user_events.py::test_real_record_event_inserts_row
|
tests/test_user_events.py::test_real_record_event_inserts_row
|
||||||
|
|
|
||||||
|
|
@ -121,7 +121,7 @@ def test_keeper_listing_count_puts_nulls_last() -> None:
|
||||||
assert "listing_cnt DESC NULLS LAST" in order
|
assert "listing_cnt DESC NULLS LAST" in order
|
||||||
# Оба места, где применяется порядок (ROW_NUMBER-ранг и first_value-выбор keeper'а),
|
# Оба места, где применяется порядок (ROW_NUMBER-ранг и first_value-выбор keeper'а),
|
||||||
# берут одну и ту же константу — иначе ранг и keeper разъедутся построчно.
|
# берут одну и ту же константу — иначе ранг и keeper разъедутся построчно.
|
||||||
mapping = _flat(hdm._mapping_sql(hdm._FIAS_KEY_EXPR, apply_geo_guard=False))
|
mapping = _flat(hdm._mapping_sql(hdm._FIAS_KEY_EXPR))
|
||||||
assert mapping.count("listing_cnt DESC NULLS LAST") >= 2
|
assert mapping.count("listing_cnt DESC NULLS LAST") >= 2
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -274,56 +274,31 @@ def test_fias_pass_clusters_by_house_fias_id() -> None:
|
||||||
assert "'addr:'" not in flat
|
assert "'addr:'" not in flat
|
||||||
|
|
||||||
|
|
||||||
def test_fias_pass_drops_geo_guard_canon_pass_keeps_it() -> None:
|
def test_both_passes_carry_the_geo_guard() -> None:
|
||||||
"""#2187: ФИАС identity strictly outranks geo-proximity, so the fias-pass mapping must NOT
|
"""#2690: the fias pass carries the same 250 m guard as the canon pass (#2187 is reverted).
|
||||||
carry the 250 m distance guard (same-fias rows merge even with NULL geom on a side or >250 m
|
|
||||||
apart), while the canon-pass mapping MUST keep it (cross-town over-merge protection)."""
|
house_fias_id is DaData's answer to our own address string, so it does not tell towns apart
|
||||||
fias = _flat(_FIAS_MAPPING_SQL)
|
any better than the canon does — prod 2026-09-05 the unguarded fias pass merged 53 pairs
|
||||||
canon = _flat(_MAPPING_SQL)
|
more than 3 km apart.
|
||||||
# canon pass keeps the full geo guard (distance + NULL-geom safety on both sides).
|
The value-level proof is test_real_fias_pass_keeps_geo_guard; this pins the rendered SQL.
|
||||||
assert "ST_DistanceSphere(loser_geom, keeper_geom) <= 250" in canon
|
"""
|
||||||
assert "keeper_geom IS NOT NULL" in canon
|
|
||||||
assert "loser_geom IS NOT NULL" in canon
|
|
||||||
# fias pass drops the distance guard AND the NULL-geom exclusions entirely.
|
|
||||||
#
|
|
||||||
# Asserted on the guard PREDICATE, not on the bare function name: since #2690 the mapping also
|
|
||||||
# MEASURES the keeper↔loser distance into `distance_m` for the merge journal, on both passes.
|
|
||||||
# Measuring is the opposite of guarding — the fias pass is precisely where nothing else records
|
|
||||||
# how far apart the merged rows were — so the name alone can no longer stand in for the guard.
|
|
||||||
assert "ST_DistanceSphere(loser_geom, keeper_geom) <= 250" not in fias
|
|
||||||
guard = (
|
guard = (
|
||||||
"AND keeper_geom IS NOT NULL AND loser_geom IS NOT NULL "
|
"AND keeper_geom IS NOT NULL AND loser_geom IS NOT NULL "
|
||||||
"AND ST_DistanceSphere(loser_geom, keeper_geom) <= 250"
|
"AND ST_DistanceSphere(loser_geom, keeper_geom) <= 250"
|
||||||
)
|
)
|
||||||
assert guard in canon
|
assert guard in _flat(_MAPPING_SQL)
|
||||||
assert guard not in fias
|
assert guard in _flat(_FIAS_MAPPING_SQL)
|
||||||
# the cross-fias anti-over-merge guard is untouched in the canon pass.
|
for sql in (_MAPPING_SQL, _FIAS_MAPPING_SQL):
|
||||||
assert "lower(loser_fias) <> lower(keeper_fias)" in canon
|
# the journal's distance MEASUREMENT and the cross-fias guard stay on both passes.
|
||||||
|
assert "AS distance_m" in sql
|
||||||
|
assert "lower(loser_fias) <> lower(keeper_fias)" in _flat(sql)
|
||||||
def test_mapping_sql_geo_guard_param_toggles_only_distance_filter() -> None:
|
|
||||||
"""_mapping_sql(apply_geo_guard=...) toggles ONLY the 250 m distance filter; the cross-fias
|
|
||||||
guard is emitted regardless, and the default is True (canon-safe)."""
|
|
||||||
guard = "ST_DistanceSphere(loser_geom, keeper_geom) <= 250"
|
|
||||||
with_guard = _flat(hdm._mapping_sql(hdm._CANON_KEY_EXPR, apply_geo_guard=True))
|
|
||||||
without_guard = _flat(hdm._mapping_sql(hdm._CANON_KEY_EXPR, apply_geo_guard=False))
|
|
||||||
assert guard in with_guard
|
|
||||||
assert guard not in without_guard
|
|
||||||
# default = True (the canon pass must never lose its guard by omission).
|
|
||||||
assert guard in _flat(hdm._mapping_sql(hdm._CANON_KEY_EXPR))
|
|
||||||
# ...while the journal's distance MEASUREMENT is emitted either way (#2690).
|
|
||||||
assert "AS distance_m" in with_guard and "AS distance_m" in without_guard
|
|
||||||
# cross-fias guard present in BOTH renderings (independent of the geo guard).
|
|
||||||
assert "lower(loser_fias) <> lower(keeper_fias)" in with_guard
|
|
||||||
assert "lower(loser_fias) <> lower(keeper_fias)" in without_guard
|
|
||||||
|
|
||||||
|
|
||||||
def test_both_passes_share_one_pipeline_no_copy_paste() -> None:
|
def test_both_passes_share_one_pipeline_no_copy_paste() -> None:
|
||||||
"""The two mappings differ ONLY in the cluster key + the (canon-only) geo guard — no copy-paste.
|
"""The two mappings differ ONLY in the cluster key — no copy-paste.
|
||||||
|
|
||||||
#2187: the 250 m distance guard is emitted for the canon pass ONLY; the fias pass drops it (a
|
The CTE skeleton, keeper rule, per-partition geom/fias exposure, the cross-fias guard and (since
|
||||||
shared ГАР UUID IS the building identity and outranks proximity). Everything else — the CTE
|
#2690) the 250 m geo guard are identical.
|
||||||
skeleton, keeper rule, per-partition geom/fias exposure and the cross-fias guard — is identical.
|
|
||||||
"""
|
"""
|
||||||
canon = _flat(_MAPPING_SQL)
|
canon = _flat(_MAPPING_SQL)
|
||||||
fias = _flat(_FIAS_MAPPING_SQL)
|
fias = _flat(_FIAS_MAPPING_SQL)
|
||||||
|
|
@ -338,11 +313,8 @@ def test_both_passes_share_one_pipeline_no_copy_paste() -> None:
|
||||||
"lower(loser_fias) <> lower(keeper_fias)",
|
"lower(loser_fias) <> lower(keeper_fias)",
|
||||||
):
|
):
|
||||||
assert token in canon and token in fias
|
assert token in canon and token in fias
|
||||||
# the 250 m distance guard is CANON-ONLY (#2187) — fias identity outranks proximity.
|
# the rendered SQL is the same text once the cluster key is swapped back.
|
||||||
assert "ST_DistanceSphere(loser_geom, keeper_geom) <= 250" in canon
|
assert fias.replace(_flat(hdm._FIAS_KEY_EXPR), _flat(hdm._CANON_KEY_EXPR)) == canon
|
||||||
assert "ST_DistanceSphere(loser_geom, keeper_geom) <= 250" not in fias
|
|
||||||
# ...but the journal's distance MEASUREMENT is on both — measuring is not guarding.
|
|
||||||
assert "AS distance_m" in canon and "AS distance_m" in fias
|
|
||||||
|
|
||||||
|
|
||||||
def test_cross_fias_guard_blocks_slash_collapse_over_merge() -> None:
|
def test_cross_fias_guard_blocks_slash_collapse_over_merge() -> None:
|
||||||
|
|
@ -1130,19 +1102,17 @@ def test_real_fias_pass_cross_guard_and_identity_carryover() -> None:
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(_live_session() is None, reason="no reachable Postgres test DB")
|
@pytest.mark.skipif(_live_session() is None, reason="no reachable Postgres test DB")
|
||||||
def test_real_fias_pass_ignores_geo_guard() -> None:
|
def test_real_fias_pass_keeps_geo_guard() -> None:
|
||||||
"""End-to-end on a real DB for #2187 — the FIAS pass drops the 250 m geo guard:
|
"""End-to-end on a real DB for #2690 — the FIAS pass obeys the same 250 m geo guard:
|
||||||
|
|
||||||
A. same house_fias_id, loser has NULL geom (no lat/lon) → MERGED (fias identity outranks
|
A. same house_fias_id, loser has NULL geom (no lat/lon) → NOT merged (guard cannot judge).
|
||||||
the missing coordinate; the geom-first keeper survives + the loser's listing re-points).
|
B. same house_fias_id, ~5 km apart (>250 m) → NOT merged (prod 2026-09-05: the
|
||||||
B. same house_fias_id, ~5 km apart (>250 m) → MERGED (fias identity outranks
|
unguarded pass merged «р-н Синарский, улица Кирова, 4» into Екатеринбург, 362 km).
|
||||||
proximity — a scattered/broken coordinate no longer blocks the merge).
|
C. same house_fias_id, DIFFERENT canon, ~10 m apart → MERGED (the pass still works —
|
||||||
C. same canon, NO fias, ~5 km apart (>250 m) → NOT merged (the canon pass STILL
|
without C, «nothing merged» would pass even if the fias pass were dead).
|
||||||
enforces the geo guard — the fix is fias-only, canon behaviour is unchanged).
|
|
||||||
|
|
||||||
A/B use distinct synthetic streets so ONLY the shared fias groups them (the canon never does);
|
Distinct synthetic streets «*2690» so ONLY the shared fias groups each pair (never the canon).
|
||||||
C shares one address so only the canon pass can group it. Streets «*2187» avoid prod aliases.
|
Latitude delta 0.045° at ~56.84° ≈ 5 km; 0.00009° ≈ 10 m.
|
||||||
Latitude delta 0.045° at ~56.84° ≈ 5 km (>250 m).
|
|
||||||
"""
|
"""
|
||||||
from sqlalchemy import text as _t
|
from sqlalchemy import text as _t
|
||||||
|
|
||||||
|
|
@ -1153,49 +1123,53 @@ def test_real_fias_pass_ignores_geo_guard() -> None:
|
||||||
_t(
|
_t(
|
||||||
"INSERT INTO houses "
|
"INSERT INTO houses "
|
||||||
"(id, source, ext_house_id, url, address, lat, lon, house_fias_id) VALUES "
|
"(id, source, ext_house_id, url, address, lat, lon, house_fias_id) VALUES "
|
||||||
# A — same fias, loser NULL geom → fias pass merges despite the missing coordinate
|
"(900030,'avito','EXT-2690-A-K','u','ФиасГеоA2690, 1',56.84,60.6,'F-A-2690'),"
|
||||||
"(900030,'avito','EXT-2187-A-K','u','ФиасГеоA2187, 1',56.84,60.6,'F-A-2187'),"
|
"(900031,'cian', 'EXT-2690-A-L','u','ФиасГеоAL2690, 2',NULL,NULL,'F-A-2690'),"
|
||||||
"(900031,'cian', 'EXT-2187-A-L','u','ФиасГеоAL2187, 2',NULL,NULL,'F-A-2187'),"
|
"(900032,'avito','EXT-2690-B-K','u','ФиасГеоB2690, 3',56.84,60.6,'F-B-2690'),"
|
||||||
# B — same fias, ~5 km apart (>250 m) → fias pass merges despite the distance
|
"(900033,'cian', 'EXT-2690-B-L','u','ФиасГеоBL2690, 4',56.885,60.6,'F-B-2690'),"
|
||||||
"(900032,'avito','EXT-2187-B-K','u','ФиасГеоB2187, 3',56.84,60.6,'F-B-2187'),"
|
"(900034,'avito','EXT-2690-C-K','u','ФиасГеоC2690, 5',56.84,60.6,'F-C-2690'),"
|
||||||
"(900033,'cian', 'EXT-2187-B-L','u','ФиасГеоBL2187, 4',56.885,60.6,'F-B-2187'),"
|
"(900035,'cian', 'EXT-2690-C-L','u','ФиасГеоCL2690, 6',56.84009,60.6,'F-C-2690')"
|
||||||
# C — same canon, NO fias, ~5 km apart → canon pass STILL blocks (guard unchanged)
|
|
||||||
"(900034,'avito','EXT-2187-C-1','u','КанонГео2187, 5', 56.84000,60.60000,NULL),"
|
|
||||||
"(900035,'cian', 'EXT-2187-C-2','u','КанонГео2187, 5', 56.88500,60.60000,NULL)"
|
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
# A loser gets a listing so we prove the re-point still fires with a NULL-geom loser.
|
|
||||||
db.execute(
|
db.execute(
|
||||||
_t(
|
_t(
|
||||||
"INSERT INTO listings "
|
"INSERT INTO listings "
|
||||||
"(id, source, source_url, source_id, dedup_hash, price_rub, house_id_fk) VALUES "
|
"(id, source, source_url, source_id, dedup_hash, price_rub, house_id_fk) VALUES "
|
||||||
"(910031,'cian','http://t/2187/al','L-2187-AL','dh-2187-al',6000000,900031)"
|
"(910031,'cian','http://t/2690/al','L-2690-AL','dh-2690-al',6000000,900031),"
|
||||||
|
"(910035,'cian','http://t/2690/cl','L-2690-CL','dh-2690-cl',6000000,900035)"
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
out = hdm.merge_duplicate_houses(db, dry_run=False)
|
out = hdm.merge_duplicate_houses(db, dry_run=False)
|
||||||
assert out["losers_deleted"] >= 2 # A + B both merged by the fias pass
|
|
||||||
|
|
||||||
ids = [
|
ids = {
|
||||||
r.id
|
r.id
|
||||||
for r in db.execute(
|
for r in db.execute(
|
||||||
_t("SELECT id FROM houses WHERE id BETWEEN 900030 AND 900035 ORDER BY id")
|
_t("SELECT id FROM houses WHERE id BETWEEN 900030 AND 900035")
|
||||||
).all()
|
).all()
|
||||||
|
}
|
||||||
|
assert ids == {900030, 900031, 900032, 900033, 900035}, sorted(ids)
|
||||||
|
assert out["losers_deleted"] == 1
|
||||||
|
# A: the NULL-geom row keeps its own listing; C: the row WITH the listing is the keeper.
|
||||||
|
lst = dict(
|
||||||
|
db.execute(
|
||||||
|
_t("SELECT id, house_id_fk FROM listings WHERE id IN (910031, 910035)")
|
||||||
|
).all()
|
||||||
|
)
|
||||||
|
assert lst == {910031: 900031, 910035: 900035}
|
||||||
|
journal = db.execute(
|
||||||
|
_t(
|
||||||
|
"SELECT merge_pass, geo_guard, loser_id, keeper_id, round(distance_m) AS d "
|
||||||
|
"FROM house_merge_log WHERE loser_id BETWEEN 900030 AND 900035"
|
||||||
|
)
|
||||||
|
).all()
|
||||||
|
assert [(j.merge_pass, j.geo_guard, j.loser_id, j.keeper_id, j.d) for j in journal] == [
|
||||||
|
("fias", True, 900034, 900035, 10)
|
||||||
]
|
]
|
||||||
# A: same-fias, loser NULL geom → MERGED (the geo guard would have blocked on NULL geom).
|
|
||||||
assert 900030 in ids and 900031 not in ids, "same-fias NULL-geom loser must merge"
|
|
||||||
# B: same-fias, >250 m apart → MERGED (the geo guard would have blocked on distance).
|
|
||||||
assert 900032 in ids and 900033 not in ids, "same-fias >250 m apart must merge"
|
|
||||||
# C: same-canon, no fias, >250 m apart → NOT merged (canon geo guard unchanged by #2187).
|
|
||||||
assert 900034 in ids and 900035 in ids, "canon-only pair >250 m apart must NOT merge"
|
|
||||||
|
|
||||||
# A: the NULL-geom loser's listing re-pointed onto the surviving geom-first keeper.
|
|
||||||
repointed = db.execute(_t("SELECT house_id_fk FROM listings WHERE id = 910031")).scalar()
|
|
||||||
assert repointed == 900030
|
|
||||||
finally:
|
finally:
|
||||||
db.rollback()
|
db.rollback()
|
||||||
db.execute(_t("DELETE FROM listings WHERE id = 910031"))
|
db.execute(_t("DELETE FROM listings WHERE id IN (910031, 910035)"))
|
||||||
db.execute(_t("DELETE FROM house_sources WHERE house_id BETWEEN 900030 AND 900035"))
|
db.execute(_t("DELETE FROM house_sources WHERE house_id BETWEEN 900030 AND 900035"))
|
||||||
db.execute(_t("DELETE FROM house_address_aliases WHERE house_id BETWEEN 900030 AND 900035"))
|
db.execute(_t("DELETE FROM house_address_aliases WHERE house_id BETWEEN 900030 AND 900035"))
|
||||||
# journal rows have no FK and are never cascaded away — sweep them explicitly,
|
# journal rows have no FK and are never cascaded away — sweep them explicitly,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue