Skip to content

Commit c49e614

Browse files
committed
Type coord-array indices past the 32-bit range on the DuckDB fast path
The DuckDB fast write path derived the new_idx column types by inferring from the remapped indices (arrowType(new_indices)) rather than using the pre-computed, max_dim-aware idxtypes. With a > 2^31 along-axis offset the remapped indices are a double, which inferred float64 and degraded to a DuckDB INTEGER temp column that overflowed; and an append part could narrow differently from part 0. .buildIndexMappings() now takes idxtypes and types new_idx from idxtypes[[j]] (falling back to inference only when idxtypes is absent), so the index type matches the R write path and is consistent across parts. .validateMaxDim() also accepts a whole-number max_dim beyond the 32-bit range: it validated integrality with `max_dim == as.integer(max_dim)`, overflowing such a dimension to NA. It now checks integrality via round() without coercion and keeps within-32-bit dims as integer, so a coordinate axis larger than ~2.1e9 can be declared. Tests: fast path types the index from a pinned max_dim (int32, not the uint8 the 50 present values would infer) and from a > 2^31 max_dim (uint32, no float64/overflow). Small/within-range writes are unchanged.
1 parent 2c72514 commit c49e614

5 files changed

Lines changed: 83 additions & 8 deletions

File tree

DESCRIPTION

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: DuckDBArray
2-
Version: 0.99.00
3-
Date: 2026-07-18
2+
Version: 0.99.1
3+
Date: 2026-07-22
44
Title: DuckDB-Backed DelayedArray Implementation
55
Description:
66
Provides DuckDB-backed implementations of DelayedArray and DelayedMatrix

NEWS.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
# DuckDBArray 0.99.1
2+
3+
## Bug fixes
4+
5+
- The DuckDB fast write path now types the coordinate-index columns from the
6+
pre-computed, `max_dim`-aware `idxtypes` (`.buildIndexMappings()`) instead of
7+
inferring them from the remapped indices. Previously a `> 2^31` along-axis
8+
offset made the remapped indices a double, which inferred `float64` and
9+
degraded to a DuckDB `INTEGER` temp column that overflowed; and an append part
10+
could narrow differently from part 0. The index type now matches the R write
11+
path and is consistent across parts.
12+
13+
- `writeCoordArray()` accepts a whole-number `max_dim` beyond the 32-bit integer
14+
range (a coordinate axis larger than ~2.1e9). The validator previously coerced
15+
`max_dim` via `as.integer()`, overflowing such a dimension to `NA`; it now
16+
validates integrality without coercion and keeps within-32-bit dims as integer.
17+
118
# DuckDBArray 0.9.20
219

320
## New features

R/writeCoordArray-duckdb.R

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,14 +236,22 @@ function(x, path, indexcols, datacol, arrowtype, ...)
236236
#' @importFrom DuckDBDataFrame arrowType
237237
.buildIndexMappings <-
238238
function(tbl, indexcols, keycols, grid = NULL, along = NULL, offset = 0L,
239-
group_offset = 0L)
239+
group_offset = 0L, idxtypes = NULL)
240240
{
241241
# For each dimension, create temp table: old_key → new_idx → grid_group
242242
# Returns list(mapping_tables = c(names), cleanup_sql = c(DROP statements))
243243

244244
# Extract connection from tbl
245245
conn <- tbl$src$con
246246

247+
# Per-axis new_idx types resolved up front (max_dim-aware). Using these
248+
# instead of inferring from the remapped indices keeps the new_idx column
249+
# consistent with the R write path and, on a > 2^31 (append) offset, avoids
250+
# the double offset degrading to float64 -> INTEGER and overflowing.
251+
if (!is.null(idxtypes)) {
252+
idxtypes <- .resolveArrowTypeList(idxtypes)
253+
}
254+
247255
mapping_tables <- character(length(indexcols))
248256
cleanup_sql <- character(length(indexcols))
249257

@@ -267,9 +275,17 @@ function(tbl, indexcols, keycols, grid = NULL, along = NULL, offset = 0L,
267275
temp_name <- sprintf("temp_idxmap_%s_%s", col, temp_suffix)
268276
mapping_tables[j] <- temp_name
269277

270-
# Determine optimal integer types using existing helpers
278+
# Determine optimal integer types using existing helpers. The new_idx
279+
# type is taken from the pre-resolved per-axis idxtypes when available
280+
# (max_dim-aware, and correct for a > 2^31 offset that makes the
281+
# remapped indices a double); otherwise inferred from the data.
271282
old_key_type <- arrowType(old_keys)
272-
new_idx_type <- arrowType(new_indices)
283+
new_idx_type <- if (!is.null(idxtypes) && length(idxtypes) >= j &&
284+
!is.null(idxtypes[[j]])) {
285+
idxtypes[[j]]
286+
} else {
287+
arrowType(new_indices)
288+
}
273289
grid_group_type <- arrowType(grid_groups)
274290

275291
# Convert Arrow types to DuckDB type names
@@ -315,7 +331,8 @@ function(x, path, indexcols, datacol, grid, grid_suffix, idxtypes, arrowtype,
315331
# The grid_group column is computed using S4Arrays::mapToGrid
316332
mappings_result <- .buildIndexMappings(tbl, indexcols, keycols, grid = grid,
317333
along = along, offset = offset,
318-
group_offset = group_offset)
334+
group_offset = group_offset,
335+
idxtypes = idxtypes)
319336
mappings <- mappings_result[["mapping_tables"]]
320337

321338
# Setup cleanup handler for mapping tables

R/writeCoordArray-methods.R

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,12 @@ function(max_dim, x, append, along, offset, ndim)
6565
}
6666
if (!is.numeric(max_dim) || length(max_dim) != ndim ||
6767
anyNA(max_dim) ||
68-
!all(max_dim == as.integer(max_dim))) {
68+
!all(max_dim == round(max_dim))) {
6969
stop("'max_dim' must be NULL or an integer vector of length ", ndim)
7070
}
71-
max_dim <- as.integer(max_dim)
71+
if (all(max_dim <= .Machine$integer.max)) {
72+
max_dim <- as.integer(max_dim)
73+
}
7274
if (any(max_dim < dim(x))) {
7375
j <- which(max_dim < dim(x))[1L]
7476
stop("'max_dim[", j, "]' (", max_dim[j],

tests/testthat/test-writeCoordArray-DuckDBArray.R

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,42 @@ test_that("writeCoordArray,DuckDBArray infers schema from DuckDB column types",
7878
}, character(1L))
7979
expect_true(all(types == "double"))
8080
})
81+
82+
# the DuckDB fast path must type the index columns from the pre-computed
83+
# idxtypes (max_dim-aware) rather than inferring from the remapped indices, so a
84+
# pinned-wide index (and, on a > 2^31 offset, a double index) does not narrow /
85+
# overflow differently from the R write path.
86+
.f4IndexType <- function(path, column) {
87+
f <- list.files(path, pattern = "parquet$", recursive = TRUE,
88+
full.names = TRUE)[1L]
89+
arrow::ParquetFileReader$create(f)$GetSchema()$
90+
GetFieldByName(column)$type$ToString()
91+
}
92+
93+
test_that("fast path types the index from max_dim (idxtypes honored, not inferred)", {
94+
skip_if_not_installed("arrow")
95+
names(dimnames(state.x77)) <- c("index1", "index2")
96+
pqmat <- DuckDBMatrix(state_path, datacol = "value",
97+
keycols = lapply(dimnames(state.x77),
98+
function(x) setNames(seq_along(x), x)))
99+
path <- .newCoordPath("f4-maxdim")
100+
# Row axis pinned to 70000 (int32) though its 50 values alone infer uint8.
101+
writeCoordArray(pqmat, path,
102+
grid = RegularArrayGrid(dim(state.x77), c(10L, 4L)),
103+
max_dim = c(70000L, 8L))
104+
expect_identical(.f4IndexType(path, "index1"), "int32")
105+
})
106+
107+
test_that("fast path types the index from a > 2^31 max_dim (no float64/overflow)", {
108+
skip_if_not_installed("arrow")
109+
names(dimnames(state.x77)) <- c("index1", "index2")
110+
pqmat <- DuckDBMatrix(state_path, datacol = "value",
111+
keycols = lapply(dimnames(state.x77),
112+
function(x) setNames(seq_along(x), x)))
113+
path <- .newCoordPath("f4-bigdim")
114+
# Declare axis 2 larger than the 32-bit range.
115+
writeCoordArray(pqmat, path,
116+
grid = RegularArrayGrid(dim(state.x77), c(10L, 4L)),
117+
max_dim = c(nrow(state.x77), 3e9 + ncol(state.x77)))
118+
expect_identical(.f4IndexType(path, "index2"), "uint32")
119+
})

0 commit comments

Comments
 (0)