Skip to content

Commit 282d637

Browse files
zacdavZac Davies
andauthored
Use connection disposition for DBI queries (#218)
Co-authored-by: Zac Davies <zachary.davies+data@databricks.com>
1 parent f08dcd5 commit 282d637

7 files changed

Lines changed: 137 additions & 15 deletions

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# brickster 0.2.13
22

33
- Enabled `db_request()` retries for transient low-level HTTP request failures, improving resilience to intermittent curl/HTTP2 framing errors (#215)
4+
- Added a DBI connection-level `disposition` setting so `dbSendQuery()` and default `dbGetQuery()` calls can use `INLINE` results when direct cloud-storage downloads are blocked (#205)
45
- Added Azure AD service principal OAuth M2M support (`ARM_CLIENT_ID`, `ARM_CLIENT_SECRET`, `ARM_TENANT_ID`) with optional `DATABRICKS_AUTH_TYPE` override (`oauth-m2m`, `azure-client-secret`, `oauth-u2m`); default auth resolution now prefers Azure M2M over U2M when ARM credentials are present (#185)
56
- Marked DBFS REST wrappers (`db_dbfs_*`) as deprecated and moved them to internal-only, guiding users towards using volumes (`db_volume_*`)
67
- Added `db_volume_download_dir()` for parallel directory downloads from Unity Catalog volumes to local directories

R/databricks-dbi.R

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ setClass(
2828
catalog = "character",
2929
schema = "character",
3030
staging_volume = "character",
31+
disposition = "character",
3132
max_active_connections = "numeric",
3233
fetch_timeout = "numeric"
3334
)
@@ -83,6 +84,9 @@ setMethod("show", "DatabricksDriver", function(object) {
8384
#' @param catalog Optional catalog name to use as default
8485
#' @param schema Optional schema name to use as default
8586
#' @param staging_volume Optional volume path for large dataset staging
87+
#' @param disposition Query disposition mode to use by default for DBI query
88+
#' results. Use `"EXTERNAL_LINKS"` for large results or `"INLINE"` for
89+
#' smaller results that must avoid direct cloud-storage result downloads.
8690
#' @param max_active_connections Maximum number of concurrent download
8791
#' connections when fetching query results (default: 30)
8892
#' @param fetch_timeout Timeout in seconds for downloading each result chunk
@@ -102,12 +106,15 @@ setMethod(
102106
catalog = NULL,
103107
schema = NULL,
104108
staging_volume = NULL,
109+
disposition = c("EXTERNAL_LINKS", "INLINE"),
105110
max_active_connections = 30,
106111
fetch_timeout = 300,
107112
token = db_token(),
108113
host = db_host(),
109114
...
110115
) {
116+
disposition <- match.arg(disposition)
117+
111118
# Validate required parameters
112119
if (
113120
!is.null(warehouse_id) &&
@@ -168,6 +175,7 @@ setMethod(
168175
catalog = catalog %||% "",
169176
schema = schema %||% "",
170177
staging_volume = staging_volume %||% "",
178+
disposition = disposition,
171179
max_active_connections = max_active_connections,
172180
fetch_timeout = fetch_timeout
173181
)
@@ -218,6 +226,7 @@ setMethod("show", "DatabricksConnection", function(object) {
218226
if (!is.null(object@staging_volume) && nzchar(object@staging_volume)) {
219227
cat(" Staging Volume:", object@staging_volume, "\n")
220228
}
229+
cat(" Disposition:", object@disposition, "\n")
221230
cat(" Max Active Connections:", object@max_active_connections, "\n")
222231
cat(" Fetch Timeout (s):", object@fetch_timeout, "\n")
223232
})
@@ -227,23 +236,26 @@ setMethod("show", "DatabricksConnection", function(object) {
227236
#' Send query to Databricks (asynchronous)
228237
#' @param conn A DatabricksConnection object
229238
#' @param statement SQL statement to execute
239+
#' @param disposition Query disposition mode. Defaults to the connection's
240+
#' `disposition` setting.
230241
#' @param ... Additional arguments (ignored)
231242
#' @returns A DatabricksResult object
232243
#' @export
233244
setMethod(
234245
"dbSendQuery",
235246
signature = c(conn = "DatabricksConnection", statement = "character"),
236-
function(conn, statement, ...) {
247+
function(conn, statement, disposition = conn@disposition, ...) {
237248
db_assert_statement(statement)
249+
disposition <- match.arg(disposition, c("EXTERNAL_LINKS", "INLINE"))
238250

239251
# Execute query asynchronously
240252
resp <- db_sql_exec_query(
241253
warehouse_id = conn@warehouse_id,
242254
statement = statement,
243255
catalog = if (nzchar(conn@catalog)) conn@catalog else NULL,
244256
schema = if (nzchar(conn@schema)) conn@schema else NULL,
245-
disposition = "EXTERNAL_LINKS",
246-
format = "ARROW_STREAM",
257+
disposition = disposition,
258+
format = if (disposition == "INLINE") "JSON_ARRAY" else "ARROW_STREAM",
247259
wait_timeout = "0s", # Async execution
248260
host = conn@host,
249261
token = conn@token
@@ -265,8 +277,8 @@ setMethod(
265277
#'
266278
#' @param conn A DatabricksConnection object
267279
#' @param statement SQL statement to execute
268-
#' @param disposition Query disposition mode: "EXTERNAL_LINKS" (default) for large results,
269-
#' "INLINE" for small metadata queries (automatically chooses appropriate format)
280+
#' @param disposition Query disposition mode. Defaults to the connection's
281+
#' `disposition` setting.
270282
#' @param show_progress If `TRUE`, show progress updates during query execution (default: `TRUE`)
271283
#' @param ... Additional arguments passed to underlying query execution
272284
#' @returns A data.frame with query results
@@ -277,10 +289,12 @@ setMethod(
277289
function(
278290
conn,
279291
statement,
280-
disposition = "EXTERNAL_LINKS",
292+
disposition = conn@disposition,
281293
show_progress = TRUE,
282294
...
283295
) {
296+
disposition <- match.arg(disposition, c("EXTERNAL_LINKS", "INLINE"))
297+
284298
# Detect schema discovery queries (LIMIT 0) and optimize them
285299
if (endsWith(trimws(statement), "LIMIT 0")) {
286300
# Force INLINE disposition and disable progress for schema queries
@@ -397,7 +411,11 @@ setMethod("dbFetch", "DatabricksResult", function(res, n = -1, ...) {
397411
}
398412

399413
# Check if we need to poll for completion and start executing step
400-
initial_status <- db_sql_exec_status(statement_id = res@statement_id)
414+
initial_status <- db_sql_exec_status(
415+
statement_id = res@statement_id,
416+
host = res@connection@host,
417+
token = res@connection@token
418+
)
401419
if (initial_status$status$state %in% c("RUNNING", "PENDING")) {
402420
cli::cli_progress_step("Executing query")
403421
status <- db_sql_exec_poll_for_success(
@@ -415,6 +433,15 @@ setMethod("dbFetch", "DatabricksResult", function(res, n = -1, ...) {
415433
# Use total_row_count to detect empty result sets
416434
if (status$manifest$total_row_count == 0) {
417435
results <- db_sql_create_empty_result(status$manifest)
436+
} else if (
437+
identical(status$manifest$format, "JSON_ARRAY") ||
438+
!is.null(status$result$data_array)
439+
) {
440+
results <- db_sql_process_inline(
441+
result_data = status$result,
442+
manifest = status$manifest,
443+
row_limit = if (n > 0) n else NULL
444+
)
418445
} else {
419446
# Use helper function to fetch results with progress
420447
results <- db_sql_fetch_results(
@@ -871,7 +898,8 @@ setMethod("dbGetInfo", "DatabricksConnection", function(dbObj, ...) {
871898
username = NA_character_,
872899
host = dbObj@host,
873900
port = NA_integer_,
874-
warehouse_id = dbObj@warehouse_id
901+
warehouse_id = dbObj@warehouse_id,
902+
disposition = dbObj@disposition
875903
)
876904
})
877905

man/dbConnect-DatabricksDriver-method.Rd

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

man/dbGetQuery-DatabricksConnection-character-method.Rd

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

man/dbSendQuery-DatabricksConnection-character-method.Rd

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/test-databricks-dbi-offline-helpers.R

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
make_dbi_test_con <- function(staging_volume = "") {
1+
make_dbi_test_con <- function(staging_volume = "", disposition = "EXTERNAL_LINKS") {
22
new(
33
"DatabricksConnection",
44
warehouse_id = "test_warehouse",
@@ -7,6 +7,7 @@ make_dbi_test_con <- function(staging_volume = "") {
77
catalog = "test_catalog",
88
schema = "test_schema",
99
staging_volume = staging_volume,
10+
disposition = disposition,
1011
max_active_connections = 30,
1112
fetch_timeout = 300
1213
)
@@ -31,12 +32,14 @@ test_that("dbConnect validates tuning inputs and persists connection settings",
3132
warehouse_id = "wh-1",
3233
host = "mock_host",
3334
token = "mock_token",
35+
disposition = "INLINE",
3436
max_active_connections = 12,
3537
fetch_timeout = 45
3638
)
3739

3840
expect_s4_class(con, "DatabricksConnection")
3941
expect_identical(con@warehouse_id, "wh-1")
42+
expect_identical(con@disposition, "INLINE")
4043
expect_identical(con@max_active_connections, 12)
4144
expect_identical(con@fetch_timeout, 45)
4245
expect_true(state$opened)
@@ -62,6 +65,17 @@ test_that("dbConnect validates tuning inputs and persists connection settings",
6265
),
6366
"`fetch_timeout` must be a positive numeric value"
6467
)
68+
69+
expect_error(
70+
dbConnect(
71+
drv,
72+
warehouse_id = "wh-1",
73+
host = "mock_host",
74+
token = "mock_token",
75+
disposition = "invalid"
76+
),
77+
"'arg' should be one of"
78+
)
6579
})
6680

6781
test_that("dbWriteTable validates user input", {
@@ -363,10 +377,61 @@ test_that("query execution DBI methods dispatch expected options", {
363377
expect_identical(rows_unknown, 0L)
364378

365379
expect_identical(state$query_calls[[1]]$wait_timeout, "0s")
380+
expect_identical(state$query_calls[[1]]$disposition, "EXTERNAL_LINKS")
381+
expect_identical(state$query_calls[[1]]$format, "ARROW_STREAM")
366382
expect_identical(state$query_calls[[2]]$wait_timeout, "0s")
367383
expect_identical(state$query_calls[[3]]$disposition, "INLINE")
368384
expect_false(state$query_calls[[3]]$show_progress)
369385
expect_identical(state$query_calls[[4]]$disposition, "EXTERNAL_LINKS")
386+
387+
con_inline <- make_dbi_test_con(disposition = "INLINE")
388+
res_inline <- dbSendQuery(con_inline, "SELECT 2")
389+
expect_s4_class(res_inline, "DatabricksResult")
390+
expect_identical(state$query_calls[[5]]$disposition, "INLINE")
391+
expect_identical(state$query_calls[[5]]$format, "JSON_ARRAY")
392+
393+
out_inline <- dbGetQuery(con_inline, "SELECT * FROM inline_table")
394+
expect_identical(out_inline$ok, TRUE)
395+
expect_identical(state$query_calls[[6]]$disposition, "INLINE")
396+
})
397+
398+
test_that("dbFetch processes inline results from dbSendQuery", {
399+
con <- make_dbi_test_con(disposition = "INLINE")
400+
res <- new(
401+
"DatabricksResult",
402+
statement_id = "stmt-inline",
403+
statement = "SELECT 1 UNION ALL SELECT 2",
404+
connection = con,
405+
completed = FALSE,
406+
rows_fetched = 0
407+
)
408+
409+
local_mocked_bindings(
410+
db_sql_exec_status = function(...) {
411+
list(
412+
statement_id = "stmt-inline",
413+
status = list(state = "SUCCEEDED"),
414+
manifest = list(
415+
format = "JSON_ARRAY",
416+
total_chunk_count = 1L,
417+
total_row_count = 2L,
418+
schema = list(columns = list(
419+
list(name = "id", type_name = "INT")
420+
))
421+
),
422+
result = list(data_array = list(list(1L), list(2L)))
423+
)
424+
},
425+
db_sql_fetch_results = function(...) {
426+
stop("external result fetcher should not be called")
427+
},
428+
.package = "brickster"
429+
)
430+
431+
out <- dbFetch(res)
432+
433+
expect_s3_class(out, "tbl_df")
434+
expect_identical(out$id, c(1L, 2L))
370435
})
371436

372437
test_that("volume-method selection warns/errors at size thresholds", {

tests/testthat/test-databricks-dbplyr-offline-helpers.R

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
make_test_con <- function() {
1+
make_test_con <- function(disposition = "EXTERNAL_LINKS") {
22
new(
33
"DatabricksConnection",
44
warehouse_id = "test_warehouse",
55
host = "test_host",
66
token = "test_token",
77
catalog = "",
88
schema = "",
9-
staging_volume = ""
9+
staging_volume = "",
10+
disposition = disposition
1011
)
1112
}
1213

@@ -84,6 +85,25 @@ test_that("sql_query_save covers temporary and persistent branches", {
8485
expect_match(table_sql, "`already_quoted`")
8586
})
8687

88+
test_that("db_collect uses connection query disposition", {
89+
con <- make_test_con(disposition = "INLINE")
90+
state <- new.env(parent = emptyenv())
91+
state$disposition <- NULL
92+
93+
local_mocked_bindings(
94+
dbGetQuery = function(conn, statement, show_progress = TRUE, ...) {
95+
state$disposition <- conn@disposition
96+
data.frame(v = c(1L, 2L, 3L))
97+
},
98+
.package = "brickster"
99+
)
100+
101+
out <- dbplyr::db_collect(con, "SELECT * FROM tbl")
102+
103+
expect_identical(state$disposition, "INLINE")
104+
expect_identical(out$v, c(1L, 2L, 3L))
105+
})
106+
87107
test_that("sql_query_save validates connection", {
88108
con <- make_test_con()
89109

0 commit comments

Comments
 (0)