Skip to content

Commit 51aea00

Browse files
committed
Implement lazy sub-row fetching for non-paginateSubRows grouped mode
Only fetch sub-rows for expanded groups instead of all visible groups. Collapsed groups get empty subRows with subRowCount so expander arrows show. Changes: - DuckDBBackend.js: buildGroupLevel() adds COUNT(*) to GROUP BY, skips sub-row fetch for collapsed groups, sets __state.subRowCount - Reactable.js: always pass expanded to DuckDB query when groupBy active, fix eslint missing deps warning (canSkipInitialDuckDBQuery, defaultSorted, paginateSubRows), extend subRowCount placeholder to all backend modes - backend-duckdb.R: duckdbGroupedQuery() accepts expanded/parentId, adds COUNT(*) AS _sub_row_count, skips sub-row fetch for collapsed groups - backend-df.R: dfGroupBy() accepts expanded/parentId, trims sub-rows for collapsed groups after computing subRowCount expanded=NULL/undefined means all groups expanded (fetch all sub-rows), matching pre-existing behavior. Fix nondeterministic unique aggregator: add ORDER BY 1 to STRING_AGG(DISTINCT ...) so unique values are sorted alphabetically. Affects both DuckDB WASM client (DuckDBBackend.js) and DuckDB R server (duckdb-sql.R). Document known issues in implementation plan: - Select-all has no fallback for custom backends - Refactor selectAll into separate S3 generic Tests: 4 JS tests, 4 df R tests, 3 DuckDB R tests. All pass.
1 parent 55a534b commit 51aea00

12 files changed

Lines changed: 790 additions & 84 deletions

File tree

R/backend-df.R

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ reactableServerData.reactable_backendDf <- function(
6060

6161
# Grouping and aggregation
6262
if (length(groupBy) > 0) {
63-
data <- dfGroupBy(data, groupBy, columns)
63+
data <- dfGroupBy(data, groupBy, columns, expanded = expanded)
6464
}
6565

6666
# Pagination
@@ -106,7 +106,7 @@ dfSortBy <- function(df, by) {
106106
df
107107
}
108108

109-
dfGroupBy <- function(df, by, columns = NULL, depth = 0) {
109+
dfGroupBy <- function(df, by, columns = NULL, depth = 0, expanded = NULL, parentId = NULL) {
110110
by <- unlist(by)
111111
if (length(by) == depth) {
112112
return(df)
@@ -166,28 +166,46 @@ dfGroupBy <- function(df, by, columns = NULL, depth = 0) {
166166
}), recursive = FALSE)
167167
}
168168

169-
df[[".subRows"]] <- lapply(values, function(x) {
169+
# Compute row state IDs before building .subRows (needed for expanded check and parentId)
170+
rowIds <- unname(vapply(values, function(x) {
171+
value <- if (is.list(x)) toJSON(x) else as.character(x)
172+
sprintf("%s:%s", groupedColumnId, value)
173+
}, character(1)))
174+
175+
df[[".subRows"]] <- lapply(seq_along(values), function(j) {
176+
x <- values[[j]]
170177
value <- if (is.list(x)) toJSON(x) else as.character(x)
171178
subGroup <- groups[[value]]
172-
dfGroupBy(subGroup, by, columns = columns, depth = depth + 1)
179+
childParentId <- if (!is.null(parentId)) paste0(parentId, ".", rowIds[j]) else rowIds[j]
180+
dfGroupBy(subGroup, by, columns = columns, depth = depth + 1,
181+
expanded = expanded, parentId = childParentId)
173182
})
174183

175184
# Add row state for grouped rows. This includes:
176185

177186
# - id: unique identifier for the row (format: "columnId:value")
178187
# - grouped: TRUE to mark this as a grouped row
179-
# - subRowCount: count of sub rows (for paginateSubRows)
180-
rowIds <- unname(vapply(values, function(x) {
181-
value <- if (is.list(x)) toJSON(x) else as.character(x)
182-
sprintf("%s:%s", groupedColumnId, value)
183-
}, character(1)))
188+
# - subRowCount: count of sub rows (for paginateSubRows and lazy sub-row fetching)
184189
subRowCounts <- vapply(df[[".subRows"]], nrow, integer(1))
185190
df[["__state"]] <- dataFrame(
186191
id = rowIds,
187192
grouped = rep(TRUE, length(rowIds)),
188193
subRowCount = subRowCounts
189194
)
190195

196+
# Trim sub-rows for collapsed groups (lazy sub-row fetching).
197+
# When expanded is provided, only expanded groups keep their sub-rows.
198+
# Collapsed groups get empty data frames; subRowCount is preserved in __state
199+
# so the expander arrow still shows.
200+
if (!is.null(expanded)) {
201+
for (i in seq_along(rowIds)) {
202+
fullRowId <- if (!is.null(parentId)) paste0(parentId, ".", rowIds[i]) else rowIds[i]
203+
if (!isTRUE(expanded[[fullRowId]])) {
204+
df[[".subRows"]][[i]] <- df[[".subRows"]][[i]][0, , drop = FALSE]
205+
}
206+
}
207+
}
208+
191209
df
192210
}
193211

R/backend-duckdb.R

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ reactableServerData.reactable_backendDuckdb <- function(
153153
pageIndex, pageSize, groupBy, expanded))
154154
}
155155
return(duckdbGroupedQuery(con, cols, filters, searchValue, sortBy,
156-
pageIndex, pageSize, groupBy))
156+
pageIndex, pageSize, groupBy, expanded))
157157
}
158158

159159
query <- buildDuckdbQuery(
@@ -187,15 +187,16 @@ reactableServerData.reactable_backendDuckdb <- function(
187187
# Execute a grouped query: GROUP BY for top level, then sub-rows for each group.
188188
# Returns resolvedData with nested .subRows matching the server-df format.
189189
duckdbGroupedQuery <- function(con, columns, filters, searchValue, sortBy,
190-
pageIndex, pageSize, groupBy,
191-
depth = 0, parentFilters = list(clauses = character(0), params = list())) {
190+
pageIndex, pageSize, groupBy, expanded = NULL,
191+
depth = 0, parentFilters = list(clauses = character(0), params = list()),
192+
parentId = NULL) {
192193
groupCol <- groupBy[[depth + 1]]
193194
groupedCols <- groupBy[seq_len(depth + 1)]
194195
baseWhere <- buildDuckdbWhere(columns, filters, searchValue)
195196

196-
# Build SELECT with group column + SQL aggregates
197+
# Build SELECT with group column + COUNT(*) for sub-row counts + SQL aggregates
197198
escapedGroupCol <- duckdbQuoteIdentifier(groupCol)
198-
selectParts <- escapedGroupCol
199+
selectParts <- c(escapedGroupCol, "COUNT(*) AS _sub_row_count")
199200
postComputeAggs <- list()
200201

201202
for (col in columns) {
@@ -249,29 +250,51 @@ duckdbGroupedQuery <- function(con, columns, filters, searchValue, sortBy,
249250
return(groupData)
250251
}
251252

252-
# Fetch sub-rows for each group
253+
# Extract sub-row counts and remove the helper column
254+
subRowCounts <- groupData[["_sub_row_count"]]
255+
groupData[["_sub_row_count"]] <- NULL
256+
257+
# Classify groups as expanded or collapsed.
258+
# expanded=NULL means all groups expanded (no lazy fetching, backward compat).
259+
# expanded=list() means nothing expanded (all collapsed).
253260
groupValues <- groupData[[groupCol]]
261+
stateIds <- vapply(groupValues, function(val) paste0(groupCol, ":", val), character(1))
262+
rowIds <- if (!is.null(parentId)) paste0(parentId, ".", stateIds) else stateIds
263+
if (is.null(expanded)) {
264+
isExpanded <- rep(TRUE, length(groupValues))
265+
} else {
266+
isExpanded <- vapply(rowIds, function(id) isTRUE(expanded[[id]]), logical(1))
267+
}
268+
269+
# Fetch sub-rows only for expanded groups
254270
subRowsList <- vector("list", length(groupValues))
255271

256272
for (i in seq_along(groupValues)) {
273+
if (!isExpanded[[i]]) {
274+
# Collapsed group: empty sub-rows (subRowCount set in __state below)
275+
subRowsList[[i]] <- data.frame()
276+
next
277+
}
278+
257279
childFilters <- list(
258280
clauses = c(parentFilters$clauses, paste0(escapedGroupCol, " = ?")),
259281
params = c(parentFilters$params, list(groupValues[[i]]))
260282
)
261283

262284
if (depth + 1 < length(groupBy)) {
263-
# More grouping levels recurse
285+
# More grouping levels - recurse only for expanded groups
264286
subResult <- duckdbGroupedQuery(con, columns, filters, searchValue, sortBy,
265287
pageIndex = 0, pageSize = .Machine$integer.max,
266-
groupBy, depth = depth + 1,
267-
parentFilters = childFilters)
288+
groupBy, expanded = expanded, depth = depth + 1,
289+
parentFilters = childFilters,
290+
parentId = rowIds[[i]])
268291
if (inherits(subResult, "reactable_resolvedData")) {
269292
subRowsList[[i]] <- subResult$data
270293
} else {
271294
subRowsList[[i]] <- subResult
272295
}
273296
} else {
274-
# Leaf level -- fetch individual rows
297+
# Leaf level - fetch individual rows
275298
subQuery <- buildDuckdbSubRowSql("reactable_data", baseWhere, childFilters, sortBy)
276299
subRows <- DBI::dbGetQuery(con, subQuery$sql, params = subQuery$params)
277300
# Extract _reactable_rowid into __state for stable row identification
@@ -297,10 +320,12 @@ duckdbGroupedQuery <- function(con, columns, filters, searchValue, sortBy,
297320

298321
groupData[[".subRows"]] <- subRowsList
299322

300-
# Add __state with group ID for group header rows
323+
# Add __state with group ID for group header rows.
324+
# Collapsed groups include subRowCount so the expander arrow shows.
301325
groupData[["__state"]] <- dataFrame(
302-
id = vapply(groupValues, function(val) paste0(groupCol, ":", val), character(1)),
303-
grouped = rep(TRUE, length(groupValues))
326+
id = unname(stateIds),
327+
grouped = rep(TRUE, length(stateIds)),
328+
subRowCount = ifelse(isExpanded, NA_integer_, as.integer(subRowCounts))
304329
)
305330

306331
if (depth == 0) {

R/duckdb-sql.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ duckdbAggregateSQL <- function(aggregate, columnId) {
202202
"min" = paste0("MIN(", col, ")"),
203203
"median" = paste0("MEDIAN(", col, ")"),
204204
"count" = paste0("COUNT(", col, ")"),
205-
"unique" = paste0("STRING_AGG(DISTINCT CAST(", col, " AS VARCHAR), ', ')"),
205+
"unique" = paste0("STRING_AGG(DISTINCT CAST(", col, " AS VARCHAR), ', ' ORDER BY 1)"),
206206
NULL # frequency and unknown aggregates are computed from sub-rows
207207
)
208208
}

R/shiny.R

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ getReactableState <- function(outputId, name = NULL, session = NULL) {
363363
#' @param groupBy The current grouped columns. `NULL` if empty.
364364
#' @param pagination Whether pagination is enabled, `TRUE` or `FALSE`.
365365
#' @param paginateSubRows Whether sub rows are paginated, `TRUE` or `FALSE`.
366-
#' @param selectAll Whether a select-all operation is being requested, `TRUE` or `NULL`.
366+
#' @param selectAll Whether a select-all operation is being requested. `TRUE` if the
367+
#' user clicked the select-all checkbox, or `NULL` for normal data requests.
367368
#' @param expanded The current expanded rows.
368369
#' @param ... Additional arguments passed to the S3 method.
369370
#' @return

design/duckdb-wasm-engine/implementation-plan.md

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,33 @@ integer vector via `setdiff(seq_len(rowCount), deselected)`. The payload is self
833833
selection state. This matches client-side semantics where select-all only applies to
834834
the currently filtered rows.
835835
836+
3. **~~`unique` aggregator is nondeterministic:~~** Fixed. Added `ORDER BY 1` inside the
837+
`STRING_AGG` in both DuckDB WASM client (`DuckDBBackend.js`) and DuckDB R server
838+
(`duckdb-sql.R`). Values are now sorted alphabetically, making the output deterministic.
839+
The `frequency` aggregator was also checked: it's computed from sub-rows using
840+
`Object.entries(counts)` in JS and `table()` in R. The R version sorts alphabetically
841+
(via `table()`); the JS version uses insertion order. Both are stable within each
842+
backend, so no fix needed (minor cross-backend ordering difference is acceptable).
843+
844+
4. **Select-all has no fallback for custom backends:** The `toggleAllRowsSelected` override
845+
in `Reactable.js` queries the backend for all matching row IDs (`selectAll: true`). If a
846+
custom backend doesn't implement `selectAll` (returns empty `rowIds` or errors), select-all
847+
effectively selects zero rows instead of falling back to current-page rows. Fix: save the
848+
original `toggleAllRowsSelected` before overriding it, and if `getMatchingRowIds()` returns
849+
an empty array, call through to the original (which enumerates `nonGroupedRowsById`, i.e.,
850+
current-page rows). This makes the documented behavior in `?reactableServerData` accurate:
851+
"selection will only work for rows on the current page" when the backend doesn't support it.
852+
853+
5. **Refactor `selectAll` out of `reactableServerData`:** The `selectAll` parameter overloads
854+
`reactableServerData()` to serve two unrelated purposes (return page data vs. return all
855+
matching row IDs) with a polymorphic return type. Refactor into a separate S3 generic,
856+
e.g. `reactableServerSelectAll(x, data, columns, filters, searchValue, ...)`. Provide a
857+
default method that calls `reactableServerData()` without pagination and extracts row IDs
858+
from `__state$id`, so custom backends get cross-page select-all for free without
859+
implementing anything extra. Keep the existing overloaded approach working for backward
860+
compatibility (if a backend handles `selectAll` in `reactableServerData`, use that;
861+
otherwise dispatch to the new generic).
862+
836863
#### 9E: Server-side expansion (lazy sub-row fetching)
837864
838865
**Goal:** Only fetch sub-rows for expanded groups, not all groups on the page. Currently, in
@@ -901,32 +928,32 @@ needed for that path.
901928
902929
**Steps:**
903930
904-
- [ ] **9E.1** **Reactable.js:** Always pass `expanded: state.expanded` (not just when
931+
- [x] **9E.1** **Reactable.js:** Always pass `expanded: state.expanded` (not just when
905932
`paginateSubRows`) in the DuckDB query effect when `groupBy` is active. Add
906-
`state.expanded` to the dependency array for grouped DuckDB queries. Extend the
907-
`subRowCount` placeholder pattern to apply in `(useDuckDB || useServerData) && !paginateSubRows`
908-
mode (currently only `paginateSubRows`).
909-
- [ ] **9E.2** **Reactable.js (server data):** The V8 server useEffect already sends `expanded`
910-
and has it in deps. For non-V8 server backends (df/duckdb-server), the expanded state
911-
needs to reach the backend. Verify that the server POST includes `expanded` and that the
912-
R `reactableServerData` methods pass it through to the grouping functions.
913-
- [ ] **9E.3** **DuckDB WASM (`DuckDBBackend.js`):** Update `queryGrouped()` to accept
914-
`expanded` param. At leaf level, split groups into expanded/collapsed. For expanded groups,
915-
fetch sub-rows via `IN()` as before. For collapsed groups, skip the sub-row query and set
916-
`.subRows = []` with `__state.subRowCount` from `COUNT(*)` (needs to be added to
917-
`buildGroupLevel()` SELECT; see `_sub_count` in `buildPaginatedGroupTree` for reference).
918-
For multi-level, only recurse into expanded groups.
919-
- [ ] **9E.4** **DuckDB R server (`backend-duckdb.R`):** Update `duckdbGroupedQuery()` to
920-
accept and use `expanded`. Add `COUNT(*) AS _sub_row_count` to the GROUP BY select if not
921-
already present. Skip sub-row fetch for collapsed groups. Set `__state$subRowCount`.
922-
Pass `expanded` from `reactableServerData` to `duckdbGroupedQuery()`.
923-
- [ ] **9E.5** **df backend (`backend-df.R`):** Update `dfGroupBy()` or add post-processing in
924-
`reactableServerData` to trim sub-rows for collapsed groups. Set `__state$subRowCount`
925-
for collapsed groups. Pass `expanded` through from `reactableServerData`.
926-
- [ ] **9E.6** **Tests:** JS tests: grouped table with DuckDB, expand group triggers re-query
927-
and shows sub-rows, collapse group triggers re-query and hides sub-rows. Verify collapsed
928-
groups show expander arrow (canExpand is true). R tests: verify `__state$subRowCount` in
929-
responses, verify sub-rows only returned for expanded groups.
933+
`state.expanded` to the dependency array for grouped DuckDB queries. Removed the
934+
`paginateSubRows` guard on the `subRowCount` placeholder pattern (the inner
935+
`subRowCount != null` check is sufficient since we're inside `manualPagination`).
936+
- [x] **9E.2** **Reactable.js (server data):** Verified: the V8 server useEffect already
937+
sends `expanded: state.expanded` and has it in deps. No changes needed; the server
938+
POST already includes `expanded` for all backends.
939+
- [x] **9E.3** **DuckDB WASM (`DuckDBBackend.js`):** `queryGrouped()` accepts `expanded`.
940+
`buildGroupLevel()` adds `COUNT(*) AS _sub_count` to GROUP BY SELECT, classifies
941+
groups as expanded/collapsed, only fetches sub-rows (IN() query) for expanded groups.
942+
Collapsed groups get `.subRows = []` with `__state.subRowCount`. Multi-level only
943+
recurses into expanded groups. `expanded=undefined` means all expanded (backward compat);
944+
`expanded={}` means all collapsed.
945+
- [x] **9E.4** **DuckDB R server (`backend-duckdb.R`):** `duckdbGroupedQuery()` accepts
946+
`expanded` and `parentId`. Adds `COUNT(*) AS _sub_row_count` to GROUP BY SELECT.
947+
Skips sub-row fetch for collapsed groups. Sets `__state$subRowCount` (NA for expanded,
948+
integer for collapsed). `expanded=NULL` is backward compat (all expanded).
949+
- [x] **9E.5** **df backend (`backend-df.R`):** `dfGroupBy()` accepts `expanded` and
950+
`parentId`. After building full sub-rows and computing `subRowCount`, trims sub-rows
951+
for collapsed groups to empty data frames. `expanded=NULL` is backward compat.
952+
Passes `expanded` and `parentId` through recursion for multi-level groupBy.
953+
- [x] **9E.6** **Tests:** 4 new JS tests (collapsed groups, expanded/collapsed mix,
954+
multi-level collapsed, multi-level expanded parent). 4 new df R tests (all collapsed,
955+
expanded/collapsed mix, backward compat, multi-level). 3 new DuckDB R tests (all
956+
collapsed, expanded/collapsed mix, backward compat). All 515 JS tests pass, all R tests pass.
930957
931958
#### 9F: Server-side data documentation
932959

0 commit comments

Comments
 (0)