Skip to content

fix(rtdb): Match backend key ordering in order_by_key() query results - #984

Closed
Sanjays2402 wants to merge 1 commit into
firebase:mainfrom
Sanjays2402:astro-order-by-key-sort
Closed

fix(rtdb): Match backend key ordering in order_by_key() query results#984
Sanjays2402 wants to merge 1 commit into
firebase:mainfrom
Sanjays2402:astro-order-by-key-sort

Conversation

@Sanjays2402

Copy link
Copy Markdown

Fixes #677

Problem

db.reference(path).order_by_key().get() returned children in plain lexicographic key order, while the Realtime Database backend orders keys that parse as 32-bit integers first (ascending numeric order) and the remaining keys lexicographically. The client/server order mismatch breaks key-based pagination: using the last key of a page in start_at() can return the same page repeatedly.

Solution

Taught the client-side query sorter (_SortEntry in firebase_admin/db.py) the backend key ordering rules, mirroring the key comparator used by the other Firebase SDKs:

  • keys parseable as signed 32-bit integers sort first, by numeric value ascending (numeric ties broken by key length, shorter first),
  • all other keys follow in lexicographic order.

The change is scoped to order_by == '$key' with string keys; $value, child-path, and $priority ordering are untouched.

Testing

  • Reproduced the exact scenario from Order diff between backend and client with order_by_key() #677 before the fix (client returned ['100001', ..., '123'] vs backend ['123', '100001', ...]); after the fix the client order matches the backend order.
  • Extended TestSorter::test_order_by_key in tests/test_db.py with cases covering numeric ordering ('2' < '10'), negative keys, leading-zero ties, integer-before-string ordering, and keys outside the 32-bit range.
  • pytest tests/test_db.py: 486 passed. ./lint.sh (pylint): 10.00/10 on changed files.
  • Full suite: one pre-existing, unrelated failure in tests/test_http_client.py::TestHttpxAsyncClient::test_request (httpx version issue in this environment; fails identically on the pristine tree).

Context Sources Used:

  • id: firebase-admin-python

The client-side query sorter ordered keys purely lexicographically, while
the Realtime Database backend orders keys that parse as 32-bit integers
first (ascending numeric order), followed by the remaining keys
lexicographically. This made order_by_key() results disagree with the
backend ordering, breaking key-based pagination (start_at/end_at).

Teach _SortEntry the backend key ordering rules so order_by_key() query
results match the server order. Fixes firebase#677.
@Sanjays2402

Copy link
Copy Markdown
Author

Closing as a duplicate of #982 — same fix for #677, opened in error. Please review #982 instead.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces backend-matching key sorting for the Firebase Realtime Database, where keys parseable as 32-bit integers are sorted numerically before other keys. The review feedback correctly identifies that keys with leading zeros (such as '01') are treated as strings rather than canonical integers by the backend. To align with this behavior, the implementation of _parse_int_key needs to be updated to ensure only canonical integer representations are parsed as integers, and the corresponding test cases should be corrected.

Comment thread firebase_admin/db.py
Comment on lines +728 to +734
def _parse_int_key(cls, key):
"""Returns key parsed as a 32-bit integer, or None if not parseable."""
if isinstance(key, str) and cls._INTEGER_KEY_PATTERN.match(key):
value = int(key)
if -2147483648 <= value <= 2147483647:
return value
return None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In the Firebase Realtime Database, keys with leading zeros (except for '0' itself) are not treated as 32-bit integers; they are treated as strings and sorted lexicographically after all integer keys.

Currently, _parse_int_key parses '01' as the integer 1 because _INTEGER_KEY_PATTERN matches it and int('01') succeeds. To ensure only canonical integer representations are treated as integers (matching the backend behavior), we should verify that str(value) == key.

    @classmethod
    def _parse_int_key(cls, key):
        """Returns key parsed as a 32-bit integer, or None if not parseable."""
        if isinstance(key, str) and cls._INTEGER_KEY_PATTERN.match(key):
            try:
                value = int(key)
                if str(value) == key and -2147483648 <= value <= 2147483647:
                    return value
            except ValueError:
                pass
        return None

Comment thread tests/test_db.py
['123', '100001', '100002', '100003']),
({'2' : 1, '10' : 2, '1' : 3}, ['1', '2', '10']),
({'-5' : 1, '3' : 2, '-10' : 3}, ['-10', '-5', '3']),
({'01' : 1, '1' : 2, '2' : 3}, ['1', '01', '2']),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since keys with leading zeros (like '01') are treated as strings by the RTDB backend, they should be sorted lexicographically after all valid 32-bit integer keys (like '1' and '2'). Therefore, the expected sorted order for {'01' : 1, '1' : 2, '2' : 3} should be ['1', '2', '01'] instead of ['1', '01', '2'].

Suggested change
({'01' : 1, '1' : 2, '2' : 3}, ['1', '01', '2']),
({'01' : 1, '1' : 2, '2' : 3}, ['1', '2', '01']),

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.

Order diff between backend and client with order_by_key()

1 participant