Like mutate, summarise needs to create subqueries when necessary

[tidyverse]

Like mutate, summarise needs to create subqueries when necessary

[hadley]

I think it might make sense in R due to differing semantics. It is simply not feasible to make dplyr and a database agree for every possible input.

@AjarKeen commented on Apr 4, 2018, 7:54 PM UTC:

I think the problem isn't related to the tidyeval issue. It seems to just be that translation of the failing code into SQL results in a reference to the newly created column within the same statement, which doesn't work.

The code that fails is this:

flights_sdf %>%
  group_by(carrier) %>%
  summarize(count_num = n(), 
            mean_dep_delay = mean(dep_delay),
            ratio = mean_dep_delay / count_num) %>%
  collect()

Which is translated to:

SELECT `carrier`, 
       count(*) AS `count_num`, 
       AVG(`dep_delay`) AS `mean_dep_delay`, 
       `mean_dep_delay` / `count_num` AS `ratio`
FROM `flights`
GROUP BY `carrier`

We can test that query out on a toy table:

CREATE TABLE flights ( 
carrier VARCHAR(50), 
dep_delay FLOAT
); 

INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('UA', 0);
INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('AA', 5);
INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('DL', 10);
INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('UA', 5);
INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('AA', 10);
INSERT INTO `flights` (`carrier`, `dep_delay`) VALUES ('DL', 15);

SELECT `carrier`, 
        count(*) AS `count_num`, 
        AVG(`dep_delay`) AS `mean_dep_delay`, 
        `mean_dep_delay` / `count_num` AS `ratio`
FROM `flights`
GROUP BY `carrier`

Which fails: Unknown column 'mean_dep_delay'.

Instead, we need the R code to be translated into something like this:

SELECT `carrier`,
       `mean_dep_delay` / `count_num` AS `ratio`
FROM (
    SELECT `carrier`, 
            count(*) AS `count_num`, 
            AVG(`dep_delay`) AS `mean_dep_delay`
    FROM `flights`
    GROUP BY `carrier` 
    ) AS flights_grouped

Which works on the toy example:

carrier | ratio
AA      | 3.75
DL      | 6.25
UA      | 1.25

I'm not sure how to make such a change in dbplyr, but it would definitely be a useful feature.

@hadley commented on May 20, 2018, 2:38 PM UTC:

Minimal reprex

library(dplyr, warn.conflicts = FALSE)

lf1 <- dbplyr::lazy_frame(x = 1:5, src = dbplyr::simulate_dbi())

lf1 %>% 
  summarise(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE),
    x3 = x1 / x2
  ) %>% 
  show_query()
#> <SQL> SELECT AVG("x") AS "x1", SUM("x") AS "x2", "x1" / "x2" AS "x3"
#> FROM "df"

Created on 2018-05-20 by the reprex package (v0.2.0).

@hadley commented on May 20, 2018, 2:39 PM UTC:

Compare with mutate() which correctly creates the nested subquery:

library(dplyr, warn.conflicts = FALSE)

lf1 <- dbplyr::lazy_frame(x = 1:5, src = dbplyr::simulate_dbi())

lf1 %>% 
  mutate(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE),
    x3 = x1 / x2
  ) %>% 
  show_query()
#> <SQL> SELECT "x", "x1", "x2", "x1" / "x2" AS "x3"
#> FROM (SELECT "x", avg("x") OVER () AS "x1", sum("x") OVER () AS "x2"
#> FROM "df") "gdgrsnkldf"

[hadley]

This isn't quite as simple as applying the algorithm from mutate() because while mutate() automatically keeps previous variables, summarise() does not. This means it's the flip side of #193

[hadley]

Also need to make this work for transmute()

[hadley]

Updated reprex:

library(dplyr, warn.conflicts = FALSE)
lf <- dbplyr::lazy_frame(x = 1:5)

lf %>% 
  mutate(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE),
    x3 = x1 / x2
  )
#> <SQL>
#> SELECT `x`, `x1`, `x2`, `x1` / `x2` AS `x3`
#> FROM (SELECT `x`, AVG(`x`) OVER () AS `x1`, SUM(`x`) OVER () AS `x2`
#> FROM `df`) `dbplyr_ksmttkjckb`

lf %>% 
  summarise(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE),
    x3 = x1 / x2
  )
#> <SQL>
#> SELECT AVG(`x`) AS `x1`, SUM(`x`) AS `x2`, `x1` / `x2` AS `x3`
#> FROM `df`

^{Created on 2019-03-14 by the reprex package (v0.2.1.9000)}

[hadley]

Maybe it's enough to make this an error? It's not really clear what the above example means, and we really want to the user to rewrite to:

library(dplyr, warn.conflicts = FALSE)
lf <- dbplyr::lazy_frame(x = 1:5)

lf %>% 
  summarise(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE)
  ) %>% 
  mutate(x3 = x1 / x2)
#> <SQL>
#> SELECT `x1`, `x2`, `x1` / `x2` AS `x3`
#> FROM (SELECT AVG(`x`) AS `x1`, SUM(`x`) AS `x2`
#> FROM `df`) `dbplyr_xejwocofsh`

^{Created on 2019-03-14 by the reprex package (v0.2.1.9000)}

[doorisajar]

The only confusing thing about this fix is that the "wrong" version still works in a non-dbplyr context, so the same code will work or not work if the data source changes. This makes it hard to prototype locally and then execute on a larger dataset through a backend.

Local reprex:

library(dplyr)
lf <- tibble(x = 1:5)

lf %>% 
  summarise(
    x1 = mean(x, na.rm = TRUE), 
    x2 = sum(x, na.rm = TRUE),
    x3 = x1 / x2
  ) 
#> # A tibble: 1 x 3
#>      x1    x2    x3
#>   <dbl> <int> <dbl>
#> 1     3    15   0.2

I can file a dplyr issue if the right thing to do is have the error thrown in the local context as well.

Edit: or does this fix cover both cases? That wasn't clear from looking at the commit.