I noticed that slice_max currently defaults to a ROW_NUMBER() window function. While that's a standard way to do it, it forces a sort phase that can get pretty expensive on large datasets. BigQuery has a native MAX_BY(value, key) aggregate that handles this in a single-pass hash aggregate (no sort needed). We recently had a pipeline hitting shuffle limits (200M+ rows) that failed with Resources exceeded using the window version, but ran cleanly once we manually swapped it for MAX_BY.
When n = 1 and with_ties = FALSE, we can translate the query to a GROUP BY with MAX_BY instead of a window function.
Current translation:
SELECT * FROM (
SELECT *, ROW_NUMBER() OVER (PARTITION BY group_id ORDER BY score DESC) AS rn
FROM df
) WHERE rn <= 1
Proposed:
SELECT group_id, MAX_BY(val, score) as val, MAX(score) as score
FROM df
GROUP BY group_id
Reprex:
library(dplyr)
library(dbplyr)
con <- bigrquery:::simulate_bigrquery()
df <- lazy_frame(g = "g1", v = 1L, score = 1.5, con = con)
df |>
slice_max(score, n = 1, with_ties = FALSE, by = g) |>
show_query()
I’m thinking of a narrow S3 method for tbl_BigQueryConnection. If it's anything other than the simple n=1 case, it just falls back to NextMethod() so we don't break existing behavior:
#' @importFrom dplyr slice_max
#' @export
slice_max.tbl_BigQueryConnection <- function(.data, order_by, ..., n,
prop, by = NULL,
with_ties = TRUE,
na_rm = FALSE) {
if (!missing(prop) || missing(n) || n != 1 || isTRUE(with_ties)) {
return(NextMethod())
}
order_quo <- rlang::enquo(order_by)
if (!is_simple_column(order_quo)) {
return(NextMethod())
}
slice_via_max_by(.data, order_quo, rlang::enquo(by), direction = "max")
}
slice_min being symmetric... The optimization fires only when n == 1, with_ties = FALSE, prop unset, and order_by is a bare column — anything else falls through to the existing ROW_NUMBER translation byte-for-byte. The narrow detection is the main "won't break anyone" argument.
Tests would sit in tests/testthat/test-dplyr.R with simulate_bigrquery(): narrow case emits MAX_BY()/MIN_BY(), each fall-through (n=2, with_ties=TRUE, prop=..., multi-column order_by) still emits ROW_NUMBER.
I'm happy to put together a PR if this looks like a good fit for the BigQuery translations.. if yes, a few quick questions:
- Tie determinism: Both the current window method and MAX_BY are non-deterministic for ties. Is a doc note mentioning that sufficient?
- Expressions: Should we start with "bare columns" only? (e.g.,
slice_max(-x) would just fall through to the default)
- Structs: MAX_BY(STRUCT(*), key) is more efficient but harder to implement in dbplyr. I'd start with N MAX_BY calls unless you'd prefer the struct approach.
Thanks!
I noticed that slice_max currently defaults to a
ROW_NUMBER()window function. While that's a standard way to do it, it forces a sort phase that can get pretty expensive on large datasets. BigQuery has a nativeMAX_BY(value, key)aggregate that handles this in a single-pass hash aggregate (no sort needed). We recently had a pipeline hitting shuffle limits (200M+ rows) that failed with Resources exceeded using the window version, but ran cleanly once we manually swapped it for MAX_BY.When n = 1 and with_ties = FALSE, we can translate the query to a GROUP BY with MAX_BY instead of a window function.
Current translation:
Proposed:
Reprex:
I’m thinking of a narrow S3 method for tbl_BigQueryConnection. If it's anything other than the simple n=1 case, it just falls back to NextMethod() so we don't break existing behavior:
slice_minbeing symmetric... The optimization fires only whenn == 1,with_ties = FALSE,propunset, andorder_byis a bare column — anything else falls through to the existingROW_NUMBERtranslation byte-for-byte. The narrow detection is the main "won't break anyone" argument.Tests would sit in
tests/testthat/test-dplyr.Rwithsimulate_bigrquery(): narrow case emitsMAX_BY()/MIN_BY(), each fall-through (n=2,with_ties=TRUE,prop=..., multi-columnorder_by) still emitsROW_NUMBER.I'm happy to put together a PR if this looks like a good fit for the BigQuery translations.. if yes, a few quick questions:
slice_max(-x)would just fall through to the default)Thanks!