fix(eap): sort absent order by attributes deterministically when paging - #8246
Draft
JoshuaKGoldberg wants to merge 2 commits into
Draft
fix(eap): sort absent order by attributes deterministically when paging#8246JoshuaKGoldberg wants to merge 2 commits into
JoshuaKGoldberg wants to merge 2 commits into
Conversation
A flextime page token encodes the last row's ORDER BY values, and the next page asks for the rows less than that tuple. Both halves of that break when an ORDER BY column can be NULL: `get_filters` raised `AttributeError: None` reading a null bookmark value, since `is_null` is not part of the `value` oneof, and a NULL tuple element makes the comparison NULL, which drops the row rather than paging past it. Absent map-backed keys now sort as their type's zero value in both the ORDER BY and the page boundary, so any nullable attribute can take part in a flextime ORDER BY. Sentry needs that to break log ordering ties on `sentry.timestamp.sequence`, which older SDKs do not emit.
…ision `time_window is not None` says the routing strategy chose to page by keyset, but `get_routing_decision` swallows an estimation failure and rebuilds the decision with no time window, while the client's page token still compares against the ORDER BY of the page before it. Ordering absent keys on that signal could put the two out of step, silently skipping or repeating rows at a page boundary. The requested flextime mode is stable across a pagination session, so key off that. Also stop stamping a type on normalized page-token columns: those are never NULL and their ORDER BY compares the raw column, so the sentinel belongs only on the map-backed columns the resolver wraps.
JoshuaKGoldberg
changed the title
k-fish
approved these changes
Aug 13, 2026
JoshuaKGoldberg
marked this pull request as ready for review
August 13, 2026 16:43
JoshuaKGoldberg
marked this pull request as draft
August 13, 2026 16:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Posting this as a reference & problem statement for review, please forgive what may or may not be idiomatic EAP code. We're happy to close this out if there's a better way.
Tl;dr: we want to add
sentry.timestamp.sequencefor logs to paginate on. But, unlikesentry.timestamp_precise, it's nullable, and nulls are causing crashes in pages >=2 of paginated queries.Flextime pagination (
FlexibleTimeWindowPageWithFilters, added in #7445) encodes the last row's ORDER BY values in the page token, and the next page asks for the rowsless(tuple(order by columns), tuple(bookmark values)). That assumes every ORDER BY column is non-nullable.Map-backed attributes are not.
add_existence_check_to_subscriptable_referencesselects them asif(mapContains(...), attributes_x[...], NULL), andconvert_resultsturns an absent key intoAttributeValue(is_null=True).So putting any sparse attribute in a flextime ORDER BY breaks in two ways:
get_filtersread the bookmark value withgetattr(value, str(value.WhichOneof("value"))).is_nullis field 11 and is not part of thevalueoneof, soWhichOneofreturnsNoneand the request dies withAttributeError: None...so you get a 500 on page 2 onward for nullable attributes.
Logs use
sentry.timestamp.sequenceas the ordering tiebreaker aftertimestampandtimestamp_precise. SDKs that predate the counter don't emit it, so that attribute is sparse and therefore nullable.Sentry side: getsentry/sentry#120451 <- LOGS-569.