ARRAY_AGG's inline LIMIT is dropped when rendering, and replaced by the statement LIMIT on the next parse

[fupelaqu]

Summary

ARRAY_AGG(x) OVER (... LIMIT n) parses, keeps n on the AST, and drops it when rendering back to SQL. Any consumer that round-trips SQL through the AST loses the per-partition bound — and, because of the fallback below, can silently get a different one.

Found while adding WindowFunctionCoverageSpec (#246). The spec pins the current behaviour in a named test so this issue is visible rather than quietly green.

Reproduction

Measured against 0.21.0:

IN : SELECT p, ARRAY_AGG(tag) OVER (PARTITION BY p ORDER BY ts ASC LIMIT 10) AS t FROM s LIMIT 100
OUT: SELECT p, ARRAY_AGG(tag) OVER (PARTITION BY p ORDER BY ts ASC)          AS t FROM s LIMIT 100

The statement-level LIMIT 100 survives; the inline LIMIT 10 does not.

Why it is not just a cosmetic loss

ArrayAgg.update (sql/src/main/scala/app/softnetwork/elastic/sql/function/aggregate/package.scala:260):

limit = limit.orElse(request.limit)

After a round-trip the inline limit is None, so the fallback takes the statement's LIMIT. The example above therefore goes from "10 tags per partition" to "100 tags per partition" — the bound is not merely lost, it is replaced by an unrelated number that happens to be in scope. A silently different answer, not an error.

Cause

WindowFunction.sql (.../function/aggregate/package.scala:174) emits the inline limit only when emitsLimitInOver is true. That flag (:156) defaults to false and is overridden only by RankingWindow (:575).

The current scaladoc calls this deliberate — "Defaults to false so existing windows (FIRST_VALUE / LAST_VALUE / ARRAY_AGG / aggregate-style) keep the bare round-trip". For FIRST_VALUE / LAST_VALUE that is sound: their limit is a synthetic Limit(1, None) (:209, :229), never user-written, and emitting it would produce SQL the user did not type. ARRAY_AGG is the odd one out — its limit is a real user-written clause (:248), documented as meaningful in documentation/sql/dql_statements.md ("LIMIT inside ARRAY_AGG restricts the collected values"), and it is the only non-ranking window where that is true.

So the flag is right in principle and wrong for exactly one subclass.

Who is affected

Anything that re-renders a parsed statement rather than passing the original string through — the REPL, JOIN reconstruction (the family of #158 and #218), and any tool that normalises SQL before sending it on.

Proposed fix

Override emitsLimitInOver to true in ArrayAgg, matching RankingWindow. Then flip the pinned expectation in WindowFunctionCoverageSpec — the test named "drop ARRAY_AGG's inline LIMIT when rendering (known asymmetry)" exists precisely to fail when this is fixed — and fold the form back into the main table as a same(...) entry.

A narrower alternative — keep the flag false and make update not fall back to request.limit for ArrayAgg — removes the substitution but keeps the loss, so a round-tripped query would collect every value instead of n. Less wrong, still wrong.

Adjacent, probably its own issue

ORDER BY inside a non-ranking OVER is accepted and then discarded entirely:

IN : SELECT c, COUNT(x) OVER (PARTITION BY c ORDER BY ts) AS n FROM t
OUT: SELECT c, COUNT(x) OVER (PARTITION BY c)             AS n FROM t

count_agg ignores the parsed order-by. That is silent acceptance of a clause that never had an effect — a different defect from this one (nothing is lost, because nothing was ever honoured), and worth filing separately if it is not intended. #246 works around it by documenting these windows as OVER (PARTITION BY ...).