james-willis opened a new pull request, #142:
URL: https://github.com/apache/sedona-spatialbench/pull/142

   ## The bug
   
   The Q5 example on the 
[Queries](https://sedona.apache.org/spatialbench/latest/queries/)
   page and in `notebooks/queries.ipynb` spells its grouped convex hull as
   
   ```sql
   ST_ConvexHull(ST_Collect(ST_GeomFromWKB(t.t_dropoffloc)))
   ```
   
   inside an `sd.sql(...)` call. SedonaDB has no `ST_Collect`, so the 
documented query
   fails at planning time:
   
   ```
   SedonaError: Error during planning: Invalid function 'st_collect'.
   ```
   
   The aggregate is spelled `ST_Collect_Agg`. The geography suite's Q5 already 
uses it in
   `docs/geography-queries.md` and `notebooks/geography-queries.ipynb`; this 
brings the
   geometry pages in line with them.
   
   ## I swept all twelve, not just Q5
   
   Every documented query was executed against SedonaDB 0.4.1 at SF1 over all 
six tables,
   both through the documented `import sedona.db` (`apache-sedona[db]` 1.9.1, 
per
   `docs/requirements.txt`) and through the standalone `sedonadb` package. 
Identical
   results both ways.
   
   | | before | after |
   |---|---|---|
   | Q1, Q2, Q3, Q4, Q6, Q7, Q8, Q9, Q10, Q11, Q12 | run | run |
   | **Q5** | **`Invalid function 'st_collect'`** | **runs** |
   
   **Q5 was the only failure, and no other documented query needed fixing.** 
Nothing else
   was a false positive either — the other eleven all executed on the first 
attempt, so
   there were no environmental failures to distinguish.
   
   ## Correctness
   
   All twelve results compared against `benchmark/answers/sf1/`, positionally 
and with the
   harness's own normalization (durations to seconds), at the CI tolerance 
`rtol=1e-6`:
   
   | query | verdict | max relative difference |
   |---|---|---|
   | Q1 | match | 2.7e-16 |
   | Q2, Q3, Q4, **Q5**, Q6, Q8, Q9, Q10, Q11, Q12 | match | 0.0 |
   | Q7 | match | 1.0e-6 (see below) |
   
   **All twelve match, Q5 exactly.** The eleven queries this PR does not touch 
return
   byte-identical results before and after, so nothing moved by accident.
   
   The documented Q5 has the same SELECT list, GROUP BY, HAVING, ORDER BY and 
LIMIT as the
   benchmark's Q5 — only whitespace and the aggregate spelling differ — so 
comparing it
   directly against `answers/sf1/q5.csv` is valid, and it reproduces all 100 
rows.
   
   ## Notebook outputs
   
   Left as they are, deliberately. Re-executing the corrected Q5 cell 
reproduces the stored
   output **byte for byte**, so the committed output was already the right data 
attached to
   code that could not produce it. It now matches code that runs. Nothing stale 
is being
   shipped, and no other cell was touched.
   
   ## Scope
   
   Documentation and notebook only — `spatialbench-queries/` and 
`benchmark/answers/` are
   untouched, and the base-class spelling in `print_queries.py` is deliberately 
left alone.
   The Chinese page gets the identical SQL edit with its translated comments 
left as they
   are. The "Spatial query characteristics tested" bullet is updated on both 
pages where it
   named the old function.
   
   Because this touches no `spatialbench-queries/**` path, the benchmark 
workflow will not
   run on this PR — hence the verification above, which I ran locally against 
the committed
   SF1 answers.
   
   ## Found but not fixed
   
   The documented Q7 converts degrees to metres with `* 111111` where the 
benchmark's Q7
   uses `/ 0.000009`. `1/0.000009 = 111111.11…`, so the two differ by exactly 
1e-6 relative
   — which lands right on the CI tolerance rather than inside it. It still 
passes, and it
   predates this PR, but it is a hair away from being a real mismatch. I left 
it alone as
   out of scope; happy to send a separate PR if maintainers want the two 
reconciled.
   
   ---
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to