Skip to content

Read a materialized top-K by naming its entity and dimension values (Step 5 of #386) - #497

Merged
zipdoki merged 12 commits into
mainfrom
feat/topk-query
Aug 31, 2026
Merged

zipdoki merged 12 commits into
mainfrom
feat/topk-query

Conversation

@zipdoki

@zipdoki zipdoki commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rank rows are written and kept fresh, but nothing can read them. Getting a ranking out means knowing the rank table's name, its index and how its row key is composed, then scanning it yourself. This exposes the read.

POST /aggregations/v1/databases/{database}/tables/{table}/topks/{topk}

The path names the source table and the declaration on it, never the rank table. RankScan.from(...) reads the declaration off the schema and resolves the rest — which table the rows live in, the metric index to walk, and the start key from (database, table, topk, entity, dimensionValues). I kept the rank table out of the request on
purpose: a service that renames it should not have to tell its callers.

{
  "entity": "user1",
  "dimensionValues": { "category": "fruit" },
  "limit": 10
}
{
  "topks": [
    { "value": "item9", "metric": 42, "properties": { "brand": "acme" } },
    { "value": "item3", "metric": 17, "properties": { "brand": "acme" } }
  ],
  "count": 2
}

When the declaration is global, entity in the request is ignored — a global ranking is one row for everyone, so there is nothing to narrow. An unknown topk is a 400 naming it.

I put the read on POST, not GET. A ranking is picked by naming values, and a query string cannot carry those onto an immutable request. The downside is that a read stops looking like one to anything in front of the server — no caching, and ReadOnlyRequestFilter has to know about it by hand. It now matches on a path segment (/topks/)
as well as on a suffix, since this path ends in a variable. If the exception list grows, the filter should read the annotation instead of a name list.

Dimension values are positional against the declaration, and a value the request omits reads back as an empty string rather than as an error. So a partially specified request quietly reads the ranking split on empty values instead of the one the caller meant. I'll leave that validation for a follow-up.

Stacked on #496 — that needs to land first.

Part of #386.

Test plan

  • ./gradlew :engine:test --tests '*RankScanTest*' — rank table resolution, key composition, global entity, unknown topk
  • ./gradlew :server:test --tests '*MetadataAggQueryControllerE2ETest*' — the endpoint over HTTP, per-entity and global rankings, dimension splits, paging
  • ./gradlew :server:test --tests '*ReadOnlyRequestFilterTest*' — the read passes in read-only mode
  • ./gradlew spotlessCheck build — formatting and full build

AI Assistance

  • This PR was written largely with AI assistance.
    • Tool / model: Claude Code (Opus 5)

@zipdoki zipdoki self-assigned this Aug 20, 2026
@zipdoki
zipdoki requested a review from em3s as a code owner August 20, 2026 03:48
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 20, 2026
@zipdoki
zipdoki removed the request for review from em3s August 20, 2026 03:48
@dosubot dosubot Bot added the enhancement New feature or request label Aug 20, 2026
@zipdoki zipdoki changed the title Read a materialized top-K by naming its entity and dimension values Read a materialized top-K as a step in a multi-hop query (Step 6 of #386) Aug 20, 2026
@zipdoki zipdoki changed the title Read a materialized top-K as a step in a multi-hop query (Step 6 of #386) Read a materialized top-K by naming its entity and dimension values (Step 5 of #386) Aug 20, 2026
@zipdoki zipdoki mentioned this pull request Aug 20, 2026
6 tasks done
zipdoki and others added 10 commits August 27, 2026 17:08
…t's name

A declaration may name a date bucket for the value it yields rather than for the field it reads, and a
`ranges` bound refers to it by that name. `refreshAt` looked the event's value up under the bucket's name,
found nothing whenever the two names differ, and fell back to counting from now — so an order that left
the window a year ago was scheduled to expire a year from ingest instead.

Nothing reported it. The aggregation answered SUCCESS and the message carried the wrong due time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`refreshQueue` landed on the declaration in #493 with nothing reading it. The enqueue still went to the
constant, so naming a queue had no effect.

Existing tests here name no queue and assert the shared one, which pins the default; the new test names one
and also asserts nothing reaches the shared queue.
The sweep design moved from an _expire table scanned by expiredAt to a
refresh queue, and these three payloads never got wired to anything.
AggregationExpireItemRequest has no references at all, and
AggregationExpireResult is only read by AggregationsExpireResponse.from,
which nothing calls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Base automatically changed from feat/topk-sweep to main August 27, 2026 10:04
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Aug 27, 2026
zipdoki and others added 2 commits August 31, 2026 09:06
Steps 3 and 4 landed on main as squash merges (#494, #496) carrying review
changes this branch never saw: `AggregationConstants.Topk.REFRESH_TABLE` is now
`REFRESH_QUEUE`, the aggregate and sweep dispatchers emit in request order via
`flatMapSequential`, and both test classes gained cases for that ordering and for
a window that never slides. Took main's side wherever the two overlapped, since
this branch's copies are the pre-review versions of the same work. Kept the
`AggregationQueryService` bean in `GraphConfiguration`, which is Step 5 and
exists only here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ller`

The auto-merge kept a blank line the branch had between `PostMapping` and
`RequestBody`, splitting the `org.springframework` import group in two.
`:server:spotlessKotlinCheck` fails on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zipdoki
zipdoki merged commit c83480e into main Aug 31, 2026
3 checks passed
@zipdoki
zipdoki deleted the feat/topk-query branch August 31, 2026 00:50
@zipdoki

zipdoki commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Optimistic Merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant