feat(tradein/db): derived price-history trigger (avito/yandex) + last_seen_at deactivate index #1559

Merged
lekss361 merged 1 commit from feat/price-history-trigger-and-lastseen-index into main 2026-06-16 16:48:59 +00:00
2 changed files with 135 additions and 0 deletions

View file

@ -0,0 +1,100 @@
-- Migration 116: derived price-change trigger on listings → offer_price_history
--
-- Problem:
-- The listings upsert in base.py::save_listings uses ON CONFLICT (dedup_hash)
-- DO UPDATE SET price_rub = EXCLUDED.price_rub (and other fields). This
-- silently overwrites the price with no history record. avito and yandex
-- price changes are therefore lost on every re-scrape.
--
-- Only cian has offer_price_history data today (source-provided, scraped
-- from cian's embedded priceChanges JSON by cian_detail.py and backfilled
-- by cian_price_history.py).
--
-- Fix:
-- An AFTER UPDATE OF price_rub trigger on listings that writes a derived
-- history row when price_rub actually changes value.
--
-- Cian-exclusion rationale (CRITICAL):
-- cian_price_history.py::backfill_cian_price_history selects cian listings
-- to process via:
-- LEFT JOIN offer_price_history oph ON oph.listing_id = l.id
-- WHERE l.source = 'cian' AND oph.listing_id IS NULL
-- i.e. it ONLY picks cian listings with zero existing history rows.
-- If this trigger were to insert a NOW() row for a cian listing, that
-- listing would be permanently excluded from the backfill, losing the full
-- source-provided timeline with accurate historical timestamps. Therefore
-- the trigger MUST fire only for non-cian sources.
--
-- Other no-ops:
-- deactivate_stale_listings runs UPDATE listings SET is_active=false —
-- it does not touch price_rub, so AFTER UPDATE OF price_rub means the
-- trigger function is not even called for those updates.
-- Geocode backfill (SET geom=...) similarly never touches price_rub.
--
-- Dependencies: listings table, offer_price_history table,
-- offer_price_history_listing_change_uq unique constraint (listing_id, change_time)
--
-- Deploy order: standalone; no other migration must precede or follow.
-- Idempotent: safe to run multiple times (CREATE OR REPLACE + DROP IF EXISTS).
BEGIN;
CREATE OR REPLACE FUNCTION record_listing_price_change()
RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
-- Skip if price did not actually change (same-value upsert write).
IF NEW.price_rub IS NOT DISTINCT FROM OLD.price_rub THEN
RETURN NEW;
END IF;
-- Skip if the new price is NULL (defensive; price_rub is NOT NULL in practice).
IF NEW.price_rub IS NULL THEN
RETURN NEW;
END IF;
-- Skip cian: its history is source-provided via cian_price_history.py backfill.
-- That service selects listings WHERE oph.listing_id IS NULL (zero rows in
-- offer_price_history). Inserting a trigger row here would permanently
-- exclude those listings from the backfill, losing accurate historical timestamps.
IF NEW.source = 'cian' THEN
RETURN NEW;
END IF;
INSERT INTO offer_price_history (
listing_id,
change_time,
price_rub,
source,
diff_percent,
recorded_at
) VALUES (
NEW.id,
now(),
NEW.price_rub,
NEW.source,
CASE
WHEN OLD.price_rub IS NOT NULL AND OLD.price_rub > 0
THEN round(
(NEW.price_rub::numeric - OLD.price_rub::numeric)
/ OLD.price_rub::numeric * 100,
2
)
ELSE NULL
END,
now()
)
ON CONFLICT ON CONSTRAINT offer_price_history_listing_change_uq
DO NOTHING;
RETURN NEW;
END;
$$;
DROP TRIGGER IF EXISTS listings_price_change_trg ON listings;
CREATE TRIGGER listings_price_change_trg
AFTER UPDATE OF price_rub ON listings
FOR EACH ROW
EXECUTE FUNCTION record_listing_price_change();
COMMIT;

View file

@ -0,0 +1,35 @@
-- Migration 117: partial composite index to accelerate deactivate_stale_listings
--
-- Problem:
-- deactivate_stale_listings executes:
-- UPDATE listings SET is_active = false
-- WHERE source = :x
-- AND is_active = true
-- AND last_seen_at < NOW() - interval '...'
-- [AND listing_segment = ANY(...)]
-- With ~39k rows and no index on last_seen_at or is_active, every call
-- performs a sequential scan.
--
-- Fix:
-- A partial B-tree index on (source, last_seen_at) WHERE is_active.
-- The WHERE is_active partial predicate eliminates already-deactivated rows
-- (typically the majority after steady-state), keeping the index small.
-- The (source, last_seen_at) composite matches the equality+range predicates
-- in the deactivate query so the planner can use an index scan.
--
-- Note on CONCURRENTLY:
-- NOT used here. The deploy migration runner wraps each file in an explicit
-- transaction (BEGIN/COMMIT). CREATE INDEX CONCURRENTLY cannot run inside
-- a transaction block. Plain CREATE INDEX holds an ACCESS SHARE lock;
-- at ~39k rows the lock duration is sub-second and acceptable.
--
-- Idempotent: IF NOT EXISTS makes repeated runs safe.
-- Deploy order: standalone.
BEGIN;
CREATE INDEX IF NOT EXISTS listings_active_source_lastseen_idx
ON listings (source, last_seen_at)
WHERE is_active;
COMMIT;