malloy
malloy copied to clipboard
https://github.com/malloydata/malloy/pull/1325
Cleaned up and normalized aggregate expressions which hadn't been touched in a while ... this includes some things which will require docs changes ...
source
is now a keyword , this is a breaking change.
Will warn when m4warnings are enabled ...
-
COUNT(DISTINCT expression)
is gone, will warn and suggestcount(expression)
-
COUNT(*)
is gone, will warn and suggestcount()
-
SUM/AVG(path)
will warn and suggestpath.SUM/AVG()
-
SUM/AVG(expr)
will warn and suggestsource.SUM/AVG()
Here's the full matrix of support ...
Function | f() | f(expr) | source.f() | source.f(expr) | path.f() | path.f(expr) |
---|---|---|---|---|---|---|
min/max | err | ok | err | ok | ok | err |
sum/avg | err | warn | err | ok | ok | err |
count | ok | ok | ok | ok | ok | err |
In your matrix, can we clarify path
with source_path
and field_path
? That is, I'm not sure with the current matrix how some_join.avg(some_field)
behaves in comparison tosome_join.some_field.avg()
. Also, it seems like the grammar doesn't have support for source.stddev(foo)
, which is a "custom" aggregate function. Should we have that? Maybe that can be a separate PR.
Function | f() | f(expr) | source.f() | source.f(expr) | srcpath.f() | srcpath.f(expr) | fldpath.f() |
---|---|---|---|---|---|---|---|
min/max | err | ok | err | ok | ? | ? | ? |
sum/avg | err | warn | err | ok | ? | ? | ? |
count | ok | ok | ok | ok | ? | ? | ? |
stddev | ? | ? | ? | ? | ? | ? | ? |
@christopherswenson i don't understand what a "source path" is as opposed to a "field path"
Yes, this totally ignores custom aggregate functions. We can delay fixing this until we have a solution for all things. This was a quick-ish attempt to address something @lloydtabb said was M4 blocking. We can for sure discuss if it makes sense to do this in two parts or wait for it all at once.
@christopherswenson is doing some new work based on this, closing this PR as his work starts from this branch
re-opening just for the checklist