Advanced-mode SELECT: aggregation functions (sum/avg/count/min/max) execute correctly but are highlighted red and the line marked [ERR] #6

Closed
opened 2026-05-28 14:44:13 +01:00 by oliversturm · 1 comment
oliversturm commented 2026-05-28 14:44:13 +01:00 (Migrated from github.com)

Observed

select sum(Age) from Customers (advanced mode):

  • Returns the correct numeric result in the output panel.
  • But sum is highlighted red in the echoed input, and the line is marked [ERR].

Same behaviour expected for avg, count, min, max.

Cause (likely)

Aggregation function calls are not yet covered by the SELECT expression grammar / validator (ADR-0031 / ADR-0033 §3). The engine runs the SQL (SQLite accepts it), but the in-app static analysis flags the call as unknown.

Expected

Aggregations should be a recognised expression form in advanced-mode SELECT — no red highlight, no [ERR] marker on a statement that ran successfully.

Scope decision

  • Just the five standard SQL aggregates above? Or wider (e.g. group_concat, total, coalesce)?
  • Plain calls only, or also sum(distinct …), count(*)?
  • GROUP BY / HAVING support is a separate question — out of scope here unless raised.
### Observed `select sum(Age) from Customers` (advanced mode): - Returns the correct numeric result in the output panel. - But `sum` is highlighted red in the echoed input, and the line is marked `[ERR]`. Same behaviour expected for `avg`, `count`, `min`, `max`. ### Cause (likely) Aggregation function calls are not yet covered by the SELECT expression grammar / validator (ADR-0031 / ADR-0033 §3). The engine runs the SQL (SQLite accepts it), but the in-app static analysis flags the call as unknown. ### Expected Aggregations should be a recognised expression form in advanced-mode SELECT — no red highlight, no `[ERR]` marker on a statement that ran successfully. ### Scope decision - Just the five standard SQL aggregates above? Or wider (e.g. `group_concat`, `total`, `coalesce`)? - Plain calls only, or also `sum(distinct …)`, `count(*)`? - `GROUP BY` / `HAVING` support is a separate question — out of scope here unless raised.
oliversturm commented 2026-05-28 23:07:49 +01:00 (Migrated from github.com)

Resolved in commit 24c2685.

Two layered fixes in the same bug class:

  1. Walker (schema_existence_diagnostics): the bare-column check on sql_expr_ident items now skips when the ident is immediately followed by ( — it's a function-call name, not a column reference. New helper is_followed_by_call_args mirrors the existing is_followed_by_qualified_ref guard. Cascades to the [ERR] indicator, the red highlight overlay, and the diagnostic-driven ambient hint.

  2. Typing-time (invalid_ident_at_cursor_in_mode): early-return at sql_expr_ident positions — the partial could resolve to either a column or a function name; without lookahead for a trailing ( we can't tell. Submit-time still catches genuine column typos via the schema-existence diagnostic and pick_hint_diagnostic.

Scope question in the original body turned out to be moot — the fix is all function calls, not an enumerated aggregate list. Matches ADR-0031 §1's stated posture ("the grammar admits the call shape structurally; it does not know which names are aggregates"). Tests added: 5 (walker positives covering every standard aggregate + count(*) + count(distinct …) + nested + WHERE + non-aggregate functions; walker negatives for unknown column inside args and inside DISTINCT-shielded args; typing-time positive; typing-time trade-off lockdown).

Two adjacent enhancement trackers filed for points outside the bug-fix scope:

  • #15 — Tab completion: offer common SQL function names
  • #16 — Restore typing-time column-typo hint via known-function list

Full suite 2040 passed / 0 failed / 0 unexpected skips. Clippy clean.

Resolved in commit 24c2685. Two layered fixes in the same bug class: 1. Walker (`schema_existence_diagnostics`): the bare-column check on `sql_expr_ident` items now skips when the ident is immediately followed by `(` — it's a function-call name, not a column reference. New helper `is_followed_by_call_args` mirrors the existing `is_followed_by_qualified_ref` guard. Cascades to the `[ERR]` indicator, the red highlight overlay, and the diagnostic-driven ambient hint. 2. Typing-time (`invalid_ident_at_cursor_in_mode`): early-return at `sql_expr_ident` positions — the partial could resolve to either a column or a function name; without lookahead for a trailing `(` we can't tell. Submit-time still catches genuine column typos via the schema-existence diagnostic and `pick_hint_diagnostic`. Scope question in the original body turned out to be moot — the fix is **all function calls**, not an enumerated aggregate list. Matches ADR-0031 §1's stated posture ("the grammar admits the call shape structurally; it does not know which names are aggregates"). Tests added: 5 (walker positives covering every standard aggregate + `count(*)` + `count(distinct …)` + nested + WHERE + non-aggregate functions; walker negatives for unknown column inside args and inside DISTINCT-shielded args; typing-time positive; typing-time trade-off lockdown). Two adjacent enhancement trackers filed for points outside the bug-fix scope: - #15 — Tab completion: offer common SQL function names - #16 — Restore typing-time column-typo hint via known-function list Full suite 2040 passed / 0 failed / 0 unexpected skips. Clippy clean.
Sign in to join this conversation.