Skip to content

Apply chain filter before limit in product search, use trigram index - #107

Merged
senko merged 3 commits into
mainfrom
fix-106
Jul 24, 2026
Merged

senko merged 3 commits into
mainfrom
fix-106

Conversation

@senko

@senko senko commented Jul 24, 2026 •

Copy link
Copy Markdown
Owner

Fixes #106.

The bug

The chains filter on GET /v1/products/ was applied in Python after the SQL LIMIT: search_products() fetched the top-N matches globally, then prepare_product_response() dropped everything not in the requested chains. So ?q=mlijeko&limit=100&chains=konzum returned the Konzum subset of the global top 100 (76 products) instead of up to 100 Konzum products — even though 225 Konzum products match "mlijeko" on prod today.

Findings along the way

While analyzing the fix, EXPLAIN on prod showed the default (non-fuzzy) search couldn't use any index at all: its predicate was cp.name ILIKE '%word%', but the only text index (idx_chain_products_name_trgm) is a GIN trigram index over lower(immutable_unaccent(name || ' ' || COALESCE(brand, ''))) — a different expression. Every exact search was a parallel seq scan over ~540k chain_products rows: ~160 ms and 3 backends per request, CPU-bound (warm cache didn't help), plus a hash join materializing the entire products table.

Measured plans on prod (q=mlijeko):

Variant Time Notes
Current exact search (warm) 161.5 ms parallel seq scan, ILIKE over all rows
+ chain filter in SQL 20.6 ms bitmap scan on existing (chain_id, code) unique index
+ trigram-indexed predicate, chain-filtered 12.0 ms GIN bitmap scan, single backend
+ trigram-indexed predicate, unfiltered 54.9 ms dominated by 3.6k PK probes into products

The fix

Three coordinated changes to search_products() / fuzzy_search_products(), no schema or index changes:

  1. Chain filter before the limit (the Filter po lancu na /v1/products/ primjenjuje se nakon limita, ne prije #106 fix): both search functions accept chain_ids and filter with cp.chain_id = ANY($n) inside the query. The router resolves chain codes to IDs up front. Uses the existing UNIQUE (chain_id, code) index.
  2. Exact-search predicate rewritten onto the indexed expression: lower(immutable_unaccent(name || ' ' || brand)) LIKE '%' || immutable_unaccent($n) || '%' with the word lowercased app-side — same convention the fuzzy path already uses. This makes the trigram GIN index usable, eliminating the seq scan.
  3. Aggregate first, join after the limit: group by cp.product_id (equivalent to grouping by ean, which is unique per product) and join products only for the returned rows. Also removes the second round-trip through get_products_by_ean(), which didn't preserve ordering.

Intentional behavior changes

  • Exact search is now diacritic-insensitive ("casa" finds "čaša") and matches brand as well as name — consistent with fuzzy search.
  • Results are now returned in relevance order; previously the re-fetch by EAN discarded the ranking, so order was arbitrary.
  • With a chain filter, ranking counts matches within the filtered chains (a product's relevance no longer depends on chains excluded from the query).
  • Filtering by a chain means matching against that chain's product names: a product whose Konzum name doesn't contain the query no longer appears in Konzum-filtered results just because another chain's name for it matches.

Review notes

  • Plan shapes were validated on prod via EXPLAIN, but the final query form (subquery wrapper + parameterized pattern) has not been executed verbatim — there is no service test suite and no local Postgres was available.
  • Post-deploy smoke test: GET /v1/products/?q=mlijeko&limit=100&chains=konzum should return 100 products (was 76), and default searches should drop from ~160 ms to ~10–55 ms of DB time.
  • Words shorter than 3 characters produce no trigrams and fall back to a scan; unchanged worst case.

senko added 3 commits July 24, 2026 10:15
Fixes #106: the chains filter on /v1/products/ was applied after the SQL
LIMIT, so filtered searches returned fewer results than requested even
when more matches existed. The filter is now pushed into the search query.

Also rewrite the exact-search predicate onto the expression indexed by
idx_chain_products_name_trgm (previously an unindexable name ILIKE that
seq-scanned ~540k rows, ~160ms per search) and join products after the
limit. Exact search is now diacritic-insensitive, matches brand, and
returns results in relevance order.
Review follow-up: chain_products accumulates discontinued products
(~28% of all rows, >50% for konzum/studenac), and the price-availability
check in the response builder ran after the SQL limit, so searches could
return far fewer than 'limit' results even when more matches existed.

Both search queries now require a chain_prices row on the chain's
effective date (latest load <= requested date, same rule as the price
lookup), applied before grouping and limiting. The requested date is now
also threaded into the search, so historical queries match against that
date's availability. Measured on prod: +0-15ms vs the previous shape.
@senko

senko commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

Follow-up commit addressing the P1 review finding: price/date filtering also ran after the LIMIT, so the PR as originally submitted still couldn't guarantee full result pages.

Why this turned out to be material

chain_products never forgets: products discontinued since May 2025 stay in the table with no current prices. Measured on prod (July 2026):

  • ~148k of ~539k rows (27.5%) have no price on their chain's effective date;
  • for the most-searched chains it's worse: Konzum 54%, Studenac 62%, Žabac 66%;
  • widening the check to "any price in the past 7 days" barely moves the numbers — this is long-discontinued inventory, not day-to-day reporting flicker.

Worse, the interaction with the chain filter introduced in this PR would have amplified it: within a single chain nearly all candidates tie at match_count = 1, so the top-N selection is arbitrary and hits dead rows at the base rate. For q=mlijeko&chains=konzum&limit=100 (423 candidate rows, only 191 alive) the response would have been ~50 products — fewer than the 76 the pre-PR code returned, because the old global-popularity ranking accidentally favored still-active products.

The fix

Both search queries now require a chain_prices row on the chain's effective date — EXISTS against the UNIQUE (chain_product_id, price_date) index, with the same chains_dates CTE (latest load ≤ requested date) the price lookup uses. Applied per row before grouping, so:

  • dead products can't occupy result slots (under-fill is structurally impossible whenever enough live matches exist — the mlijeko/konzum query now fills 100/100);
  • dead rows no longer inflate match_count ranking;
  • search and response builder share the same criterion and can never disagree;
  • per-chain effective dates mean chains with stalled crawls (e.g. Brodokomerc, servers down for weeks) stay searchable with their last-known prices, consistent with how the API already serves their prices.

Cost (EXPLAIN ANALYZE on prod)

Query Before (this PR) With EXISTS
mlijeko, chains=konzum 20.6 ms 18.5 ms
mlijeko, unfiltered 54.9 ms* 40.0 ms

*pre-aggregate-first shape; the EXISTS probes (~0.9 ms filtered, ~18 ms for 3.6k probes unfiltered) are roughly offset by joining products only after the limit. Still no schema or index changes.

Behavior change to be aware of

date now affects which products match, not just displayed prices: ?date=2026-01-01 searches against that date's price availability. That's the consistent reading of the parameter (search and response criteria now match by construction), but historical queries will return different (correct) product sets than before.

Not addressed here, filed mentally as follow-ups: purging/flagging the ~148k dead rows (pure perf/disk win now, no longer a correctness issue), and the suspiciously high churn ratios for Studenac/Brodokomerc that hint at unstable product codes in those chains' feeds.

@senko
senko marked this pull request as ready for review July 24, 2026 10:57
@senko
senko merged commit 373b549 into main Jul 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter po lancu na /v1/products/ primjenjuje se nakon limita, ne prije

1 participant