Decompose topic filters into ORs of ANDs#578
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors topic filtering in event queries by decomposing the flattened topic filter structure into a disjunction of conjunctions (OR of ANDs). Previously, all topic filters were OR'd together at each topic position, which unnecessarily included extra rows that required client-side filtering. The new approach preserves the complete filter structure at the SQL level, resulting in approximately 50% reduction in CPU and memory usage as demonstrated by the included benchmark.
Changes:
- Introduced new type definitions (
TopicCondition,TopicFilter,TopicFilters) to represent topic filters as OR of ANDs - Refactored
combineTopicsfunction to build the new structured filter format - Updated database query builder to construct SQL with nested AND/OR conditions
- Added benchmark test to demonstrate performance improvements
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/stellar-rpc/internal/db/event.go | Introduced new type definitions for topic filtering and updated GetEvents query builder to support OR of ANDs structure |
| cmd/stellar-rpc/internal/methods/get_events.go | Refactored combineTopics to decompose topic filters into conjunctive clauses within a disjunction |
| cmd/stellar-rpc/internal/methods/get_events_test.go | Added BenchmarkGetEventsTopicFilters to measure performance improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Each topic is an OR... | ||
| for _, topicFilter := range filter.Topics { | ||
| conditions := make(db.TopicFilter, 0, len(topicFilter)) | ||
| // ...but each segment within a topic is an AND. |
There was a problem hiding this comment.
The comment 'Each topic is an OR...' is misleading. According to the protocol structure, each element in filter.Topics (which is a TopicFilter) represents a single topic pattern that should match all its segments (AND), and multiple TopicFilters in the array are OR'd together. The comment should clarify this more accurately, for example: 'Each TopicFilter is OR'd together...'
| // Each topic is an OR... | |
| for _, topicFilter := range filter.Topics { | |
| conditions := make(db.TopicFilter, 0, len(topicFilter)) | |
| // ...but each segment within a topic is an AND. | |
| // Each TopicFilter (topic pattern) in filter.Topics is OR'd together... | |
| for _, topicFilter := range filter.Topics { | |
| conditions := make(db.TopicFilter, 0, len(topicFilter)) | |
| // ...but each segment within a TopicFilter is ANDed. |
sreuland
left a comment
There was a problem hiding this comment.
lgtm. could update the title of the pr to mention this is for db specifics such as Decompose db topic filter query into ... just to discern it's an internal sql model thing and not related to external rpc request model.
What
Decomposes the existing, flattened
NestedTopicArraywhich is simply a list of topic filters that are OR'd together into a disjunction of conjunctions, meaning each topic filter is kept together as anANDclause and only the total set of filters isORd together.You can see the differences in SQL as follows:
Details
which demonstrates that the topic filter lists themselves are preserved at the SQL level rather than filtered out at the service level after the fact.
Why
By OR'ing everything together, we unnecessarily include extra rows in complex filtering cases.
The included benchmark proves the efficacy of this method, run on this branch vs.
main:Details
The top run is on
mainwhile the second one is on this branch.You can see there is around a 50% reduction in both CPU and memory usage across the board due to most of the filtering happening in the database rather than client-side.
Known limitations
n/a