Skip to content

Commit b142044

Browse files
nealrichardsonArgusLi
authored andcommitted
apacheGH-34437: [R] Use FetchNode and OrderByNode (apache#34685)
### Rationale for this change See also apache#32991. By using the new nodes, we're closer to having all dplyr query business happening inside the ExecPlan. Unfortunately, there are still two cases where we have to apply operations in R after running a query: * apache#34941: Taking head/tail on unordered data, which has non-deterministic results but that should be possible, in the case where the user wants to see a slice of the result, any slice * apache#34942: Implementing tail in the FetchNode or similar would enable removing more hacks and workarounds. Once those are resolved, we can simply further and then move to the new Declaration class. ### What changes are included in this PR? This removes the use of different SinkNodes and many R-specific workarounds to support sorting and head/tail, so *almost* everything we do in a query should be represented in an ExecPlan. ### Are these changes tested? Yes. This is mostly an internal refactor, but behavior changes are accompanied by test updates. ### Are there any user-facing changes? The `show_query()` method will print slightly different ExecPlans. In many cases, they will be more informative. `tail()` now actually returns the tail of the data in cases where the data has an implicit order (currently only in-memory tables). Previously it was non-deterministic (and would return the head or some other slice of the data). When printing query objects that include `summarize()` when the `arrow.summarize.sort = TRUE` option is set, the sorting is correctly printed. It's unclear if there should be changes in performance; running benchmarks would be good but it's also not clear that our benchmarks cover all affected scenarios. * Closes: apache#34437 * Closes: apache#31980 * Closes: apache#31982 Authored-by: Neal Richardson <neal.p.richardson@gmail.com> Signed-off-by: Nic Crane <thisisnic@gmail.com>
1 parent ab00eed commit b142044

10 files changed

Lines changed: 246 additions & 228 deletions

r/R/arrowExports.R

Lines changed: 14 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

r/R/dplyr-summarize.R

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,11 @@ do_arrow_summarize <- function(.data, ..., .groups = NULL) {
287287
stop(paste("Invalid .groups argument:", .groups))
288288
}
289289
out$drop_empty_groups <- .data$drop_empty_groups
290+
if (getOption("arrow.summarise.sort", FALSE)) {
291+
# Add sorting instructions for the rows to match dplyr
292+
out$arrange_vars <- .data$selected_columns[.data$group_by_vars]
293+
out$arrange_desc <- rep(FALSE, length(.data$group_by_vars))
294+
}
290295
}
291296
out
292297
}

r/R/dplyr.R

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -284,17 +284,7 @@ tail.arrow_dplyr_query <- function(x, n = 6L, ...) {
284284
#' mutate(x = gear / carb) %>%
285285
#' show_exec_plan()
286286
show_exec_plan <- function(x) {
287-
adq <- as_adq(x)
288-
289-
# do not show the plan if we have a nested query (as this will force the
290-
# evaluation of the inner query/queries)
291-
# TODO see if we can remove after ARROW-16628
292-
if (is_collapsed(x) && has_head_tail(x$.data)) {
293-
warn("The `ExecPlan` cannot be printed for a nested query.")
294-
return(invisible(x))
295-
}
296-
297-
result <- as_record_batch_reader(adq)
287+
result <- as_record_batch_reader(as_adq(x))
298288
plan <- result$Plan()
299289
on.exit({
300290
plan$.unsafe_delete()
@@ -419,6 +409,25 @@ query_can_stream <- function(x) {
419409

420410
is_collapsed <- function(x) inherits(x$.data, "arrow_dplyr_query")
421411

422-
has_head_tail <- function(x) {
423-
!is.null(x$head) || !is.null(x$tail) || (is_collapsed(x) && has_head_tail(x$.data))
412+
has_unordered_head <- function(x) {
413+
if (is.null(x$head %||% x$tail)) {
414+
# no head/tail
415+
return(FALSE)
416+
}
417+
!has_order(x)
418+
}
419+
420+
has_order <- function(x) {
421+
length(x$arrange_vars) > 0 ||
422+
has_implicit_order(x) ||
423+
(is_collapsed(x) && has_order(x$.data))
424+
}
425+
426+
has_implicit_order <- function(x) {
427+
# Approximate what ExecNode$has_ordered_batches() would return (w/o building ExecPlan)
428+
# An in-memory table has an implicit order
429+
# TODO(GH-34698): FileSystemDataset and RecordBatchReader will have implicit order
430+
inherits(x$.data, "ArrowTabular") &&
431+
# But joins, aggregations, etc. will result in non-deterministic order
432+
is.null(x$aggregations) && is.null(x$join) && is.null(x$union_all)
424433
}

r/R/query-engine.R

Lines changed: 77 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ ExecPlan <- R6Class("ExecPlan",
7474

7575
if (is_collapsed(.data)) {
7676
# We have a nested query.
77-
if (has_head_tail(.data$.data)) {
78-
# head and tail are not ExecNodes; at best we can handle them via
79-
# SinkNode, so if there are any steps done after head/tail, we need to
80-
# evaluate the query up to then and then do a new query for the rest.
81-
# as_record_batch_reader() will build and run an ExecPlan
77+
if (has_unordered_head(.data$.data)) {
78+
# TODO(GH-34941): FetchNode should do non-deterministic fetch
79+
# Instead, we need to evaluate the query up to here,
80+
# and then do a new query for the rest.
81+
# as_record_batch_reader() will build and run an ExecPlan and do head() on it
8282
reader <- as_record_batch_reader(.data$.data)
8383
on.exit(reader$.unsafe_delete())
8484
node <- self$SourceNode(reader)
@@ -126,15 +126,6 @@ ExecPlan <- R6Class("ExecPlan",
126126
options = .data$aggregations,
127127
key_names = group_vars
128128
)
129-
130-
if (grouped && getOption("arrow.summarise.sort", FALSE)) {
131-
# Add sorting instructions for the rows too to match dplyr
132-
# (see below about why sorting isn't itself a Node)
133-
node$extras$sort <- list(
134-
names = group_vars,
135-
orders = rep(0L, length(group_vars))
136-
)
137-
}
138129
} else {
139130
# If any columns are derived, reordered, or renamed we need to Project
140131
# If there are aggregations, the projection was already handled above.
@@ -166,82 +157,81 @@ ExecPlan <- R6Class("ExecPlan",
166157
}
167158
}
168159

169-
# Apply sorting: this is currently not an ExecNode itself, it is a
170-
# sink node option.
171-
# TODO: handle some cases:
172-
# (1) arrange > summarize > arrange
173-
# (2) ARROW-13779: arrange then operation where order matters (e.g. cumsum)
160+
# Apply sorting and head/tail
161+
head_or_tail <- .data$head %||% .data$tail
174162
if (length(.data$arrange_vars)) {
175-
node$extras$sort <- list(
163+
if (!is.null(.data$tail)) {
164+
# Handle tail first: Reverse sort, take head
165+
# TODO(GH-34942): FetchNode support for tail
166+
node <- node$OrderBy(list(
167+
names = names(.data$arrange_vars),
168+
orders = as.integer(!.data$arrange_desc)
169+
))
170+
node <- node$Fetch(.data$tail)
171+
}
172+
# Apply sorting
173+
node <- node$OrderBy(list(
176174
names = names(.data$arrange_vars),
177-
orders = .data$arrange_desc,
178-
temp_columns = names(.data$temp_columns)
179-
)
180-
}
181-
# This is only safe because we are going to evaluate queries that end
182-
# with head/tail first, then evaluate any subsequent query as a new query
183-
if (!is.null(.data$head)) {
184-
node$extras$head <- .data$head
185-
}
186-
if (!is.null(.data$tail)) {
187-
node$extras$tail <- .data$tail
175+
orders = as.integer(.data$arrange_desc)
176+
))
177+
178+
if (length(.data$temp_columns)) {
179+
# If we sorted on ad-hoc derived columns, Project to drop them
180+
temp_schema <- node$schema
181+
cols_to_keep <- setdiff(names(temp_schema), names(.data$temp_columns))
182+
node <- node$Project(make_field_refs(cols_to_keep))
183+
}
184+
185+
if (!is.null(.data$head)) {
186+
# Take the head now
187+
node <- node$Fetch(.data$head)
188+
}
189+
} else if (!is.null(head_or_tail)) {
190+
# Unsorted head/tail
191+
# Handle a couple of special cases here:
192+
if (node$has_ordered_batches()) {
193+
# Data that has order, even implicit order from an in-memory table, is supported
194+
# in FetchNode
195+
if (!is.null(.data$head)) {
196+
node <- node$Fetch(.data$head)
197+
} else {
198+
# TODO(GH-34942): FetchNode support for tail
199+
# FetchNode currently doesn't support tail, but it has limit + offset
200+
# So if we know how many rows the query will result in, we can offset
201+
data_without_tail <- .data
202+
data_without_tail$tail <- NULL
203+
row_count <- nrow(data_without_tail)
204+
if (!is.na(row_count)) {
205+
node <- node$Fetch(.data$tail, offset = row_count - .data$tail)
206+
} else {
207+
# Workaround: non-deterministic tail
208+
node$extras$slice_size <- head_or_tail
209+
}
210+
}
211+
} else {
212+
# TODO(GH-34941): non-deterministic FetchNode
213+
# Data has non-deterministic order, so head/tail means "just show me any N rows"
214+
# FetchNode does not support non-deterministic scans, so we have to handle outside
215+
node$extras$slice_size <- head_or_tail
216+
}
188217
}
189218
node
190219
},
191220
Run = function(node) {
192221
assert_is(node, "ExecNode")
193-
194-
# Sorting and head/tail (if sorted) are handled in the SinkNode,
195-
# created in ExecPlan_build
196-
sorting <- node$extras$sort %||% list()
197-
select_k <- node$extras$head %||% -1L
198-
has_sorting <- length(sorting) > 0
199-
if (has_sorting) {
200-
if (!is.null(node$extras$tail)) {
201-
# Reverse the sort order and take the top K, then after we'll reverse
202-
# the resulting rows so that it is ordered as expected
203-
sorting$orders <- !sorting$orders
204-
select_k <- node$extras$tail
205-
}
206-
sorting$orders <- as.integer(sorting$orders)
207-
}
208-
209222
out <- ExecPlan_run(
210223
self,
211224
node,
212-
sorting,
213-
prepare_key_value_metadata(node$final_metadata()),
214-
select_k
225+
prepare_key_value_metadata(node$final_metadata())
215226
)
216227

217-
if (!has_sorting) {
218-
# Since ExecPlans don't scan in deterministic order, head/tail are both
228+
if (!is.null(node$extras$slice_size)) {
229+
# For non-deterministic scans, head/tail are
219230
# essentially taking a random slice from somewhere in the dataset.
220231
# And since the head() implementation is way more efficient than tail(),
221232
# just use it to take the random slice
222-
# TODO(ARROW-16628): handle limit in ExecNode
223-
slice_size <- node$extras$head %||% node$extras$tail
224-
if (!is.null(slice_size)) {
225-
out <- head(out, slice_size)
226-
}
227-
} else if (!is.null(node$extras$tail)) {
228-
# TODO(ARROW-16630): proper BottomK support
229-
# Reverse the row order to get back what we expect
230-
out <- as_arrow_table(out)
231-
out <- out[rev(seq_len(nrow(out))), , drop = FALSE]
232-
out <- as_record_batch_reader(out)
233-
}
234-
235-
# If arrange() created $temp_columns, make sure to omit them from the result
236-
# We can't currently handle this in ExecPlan_run itself because sorting
237-
# happens in the end (SinkNode) so nothing comes after it.
238-
# TODO(ARROW-16631): move into ExecPlan
239-
if (length(node$extras$sort$temp_columns) > 0) {
240-
tab <- as_arrow_table(out)
241-
tab <- tab[, setdiff(names(tab), node$extras$sort$temp_columns), drop = FALSE]
242-
out <- as_record_batch_reader(tab)
233+
out <- head(out, node$extras$slice_size)
243234
}
244-
245235
out
246236
},
247237
Write = function(node, ...) {
@@ -272,13 +262,8 @@ ExecNode <- R6Class("ExecNode",
272262
inherit = ArrowObject,
273263
public = list(
274264
extras = list(
275-
# `sort` is a slight hack to be able to keep around arrange() params,
276-
# which don't currently yield their own ExecNode but rather are consumed
277-
# in the SinkNode (in ExecPlan$run())
278-
sort = NULL,
279-
# Similar hacks for head and tail
280-
head = NULL,
281-
tail = NULL,
265+
# Workaround for non-deterministic head/tail
266+
slice_size = NULL,
282267
# `source_schema` is put here in Scan() so that at Run/Write, we can
283268
# extract the relevant metadata and keep it in the result
284269
source_schema = NULL
@@ -295,6 +280,7 @@ ExecNode <- R6Class("ExecNode",
295280
old_meta$r <- get_r_metadata_from_old_schema(self$schema, old_schema)
296281
old_meta
297282
},
283+
has_ordered_batches = function() ExecNode_has_ordered_batches(self),
298284
Project = function(cols) {
299285
if (length(cols)) {
300286
assert_is_list_of(cols, "Expression")
@@ -336,6 +322,16 @@ ExecNode <- R6Class("ExecNode",
336322
},
337323
Union = function(right_node) {
338324
self$preserve_extras(ExecNode_Union(self, right_node))
325+
},
326+
Fetch = function(limit, offset = 0L) {
327+
self$preserve_extras(
328+
ExecNode_Fetch(self, offset, limit)
329+
)
330+
},
331+
OrderBy = function(sorting) {
332+
self$preserve_extras(
333+
ExecNode_OrderBy(self, sorting)
334+
)
339335
}
340336
),
341337
active = list(

0 commit comments

Comments
 (0)