Skip to content
This repository was archived by the owner on Mar 2, 2026. It is now read-only.

Commit 326315a

Browse files
committed
re-ordered sort
1 parent 5ea1410 commit 326315a

2 files changed

Lines changed: 99 additions & 2 deletions

File tree

google/cloud/firestore_v1/base_query.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,8 +1193,6 @@ def _build_pipeline(self, source: "PipelineSource"):
11931193
if self._limit_to_last:
11941194
actual_orderings = _reverse_orderings(orderings)
11951195
ppl = ppl.sort(*actual_orderings)
1196-
else:
1197-
ppl = ppl.sort(*orderings)
11981196

11991197
# Apply cursor conditions.
12001198
# Cursors are translated into filter conditions (e.g., field > value)
@@ -1213,6 +1211,9 @@ def _build_pipeline(self, source: "PipelineSource"):
12131211
)
12141212
)
12151213

1214+
if not self._limit_to_last:
1215+
ppl = ppl.sort(*orderings)
1216+
12161217
if self._limit is not None:
12171218
ppl = ppl.limit(self._limit)
12181219

tests/unit/v1/test_base_query.py

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,3 +2337,99 @@ def _make_snapshot(docref, values):
23372337
from google.cloud.firestore_v1 import document
23382338

23392339
return document.DocumentSnapshot(docref, values, True, None, None, None)
2340+
2341+
2342+
def test__build_pipeline_limit_to_last_ordering():
2343+
from google.cloud.firestore_v1 import pipeline_expressions as expr
2344+
2345+
# Verify that for limit_to_last=True:
2346+
# 1. Sort (reversed)
2347+
# 2. Where (cursor condition)
2348+
2349+
client = make_client()
2350+
# Query: Order by 'a' ASC, StartAt(10), LimitToLast(5)
2351+
query = (
2352+
client.collection("my_col").order_by("a").start_at({"a": 10}).limit_to_last(5)
2353+
)
2354+
2355+
pipeline = query._build_pipeline(client.pipeline())
2356+
2357+
# Expected stages:
2358+
# 0. Collection
2359+
# 1. Exists (for 'a')
2360+
# 2. Sort (DESCENDING) -> This must come BEFORE the cursor filter
2361+
# 3. Where (a > 10 condition or similar)
2362+
# 4. Limit (5)
2363+
# 5. Sort (ASCENDING)
2364+
2365+
assert len(pipeline.stages) >= 4
2366+
2367+
# Find indices
2368+
sort_reversed_idx = -1
2369+
cursor_where_idx = -1
2370+
2371+
for i, stage in enumerate(pipeline.stages):
2372+
if isinstance(stage, stages.Sort):
2373+
# Check if it is the reversed sort (DESCENDING)
2374+
if (
2375+
len(stage.orders) > 0
2376+
and stage.orders[0].order_dir == expr.Ordering.Direction.DESCENDING
2377+
):
2378+
if sort_reversed_idx == -1:
2379+
sort_reversed_idx = i
2380+
2381+
if isinstance(stage, stages.Where):
2382+
# Check if this is the cursor condition.
2383+
# Cursor condition for start_at({"a": 10}) should be related to 'a' and 10.
2384+
# usually an OR or Comparison.
2385+
# The Exists filter is also a Where, but it's usually `exists(a)`.
2386+
2387+
# Simple check: The condition is not just an 'exists' function call.
2388+
cond = stage.condition
2389+
if not (hasattr(cond, "name") and cond.name == "exists"):
2390+
# Assume this is the cursor filter
2391+
cursor_where_idx = i
2392+
2393+
assert sort_reversed_idx != -1, "Reversed sort stage not found"
2394+
assert cursor_where_idx != -1, "Cursor filter stage not found"
2395+
2396+
# Reversed Sort must happen BEFORE Cursor Filter
2397+
assert sort_reversed_idx < cursor_where_idx
2398+
2399+
2400+
def test__build_pipeline_normal_ordering():
2401+
from google.cloud.firestore_v1 import pipeline_expressions as expr
2402+
2403+
# Verify that for limit_to_last=False (Normal):
2404+
# 1. Where (cursor condition)
2405+
# 2. Sort
2406+
2407+
client = make_client()
2408+
# Query: Order by 'a' ASC, StartAt(10)
2409+
query = client.collection("my_col").order_by("a").start_at({"a": 10})
2410+
2411+
pipeline = query._build_pipeline(client.pipeline())
2412+
2413+
# Expected stages:
2414+
# 0. Collection
2415+
# 1. Exists (for 'a')
2416+
# 2. Where (cursor condition)
2417+
# 3. Sort (ASCENDING)
2418+
2419+
sort_idx = -1
2420+
cursor_where_idx = -1
2421+
2422+
for i, stage in enumerate(pipeline.stages):
2423+
if isinstance(stage, stages.Sort):
2424+
sort_idx = i
2425+
2426+
if isinstance(stage, stages.Where):
2427+
cond = stage.condition
2428+
if not (hasattr(cond, "name") and cond.name == "exists"):
2429+
cursor_where_idx = i
2430+
2431+
assert sort_idx != -1, "Sort stage not found"
2432+
assert cursor_where_idx != -1, "Cursor filter stage not found"
2433+
2434+
# Cursor Filter must happen BEFORE Sort
2435+
assert cursor_where_idx < sort_idx

0 commit comments

Comments
 (0)