gregfelice commented on PR #2435: URL: https://github.com/apache/age/pull/2435#issuecomment-4763338241
### Design check before this lands — desugar-to-aggregate as the foundation for `reduce()` Flagging the core design decision for lazy consensus, since it'll likely become the template for future folding/comprehension work. `reduce()` here is implemented entirely at transform time — desugared into a correlated scalar subquery over a new ordered aggregate (`age_reduce`), with the fold body serialized via `nodeToString()` and re-evaluated per element through a standalone `ExecInitExpr`/`ExecEvalExpr` bound to `PARAM_EXEC` 0/1 — so **no PostgreSQL core changes** are needed. List order comes from the aggregate's `ORDER BY`; init is evaluated once via `CASE WHEN ordinality = 1`. I've reviewed it closely and it's correct (the empty-list-vs-fold-to-null ambiguity and the param-isolation traps are both handled) and green under cassert PG18. Two things worth a second committer's eyes before merge: 1. **Is desugar-to-aggregate the foundation we want** for `reduce()` (and as a precedent for `reduce`-adjacent features), versus a dedicated executor node? 2. **The body-scope restriction** (accumulator + element only; outer vars / `$params` / nested `reduce` / aggregates rejected with `FEATURE_NOT_SUPPORTED`) — acceptable as a first cut given the author's documented path to relax it? No objection from me on either; raising for lazy consensus rather than landing a design-setting change on a lone review. -- 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]
