Skip to content

Commit 9692653

Browse files
hidekojiclaude
andcommitted
Support additional connection parameters for Oracle (OCI) (#35892)
The ODBC based Oracle data source lets you type extra connection options as key=value pairs. Oracle (OCI) had no equivalent, so options such as prefetch mode could not be set at all. ROracle::dbConnect silently ignores any argument it does not declare, so forwarding arbitrary text would make an unsupported option look like it took effect. parseOracleOCIAdditionalParams therefore accepts only the options dbConnect actually declares, converts each value to the type it expects (logical for prefetch/external_credentials/sysdba, integer for bulk_read/ bulk_write/stmt_cache), and rejects anything else with a message naming the supported keys. stmt_cache is only honoured in prefetch mode, so that dependency is checked here rather than failing silently at run time. bulk_read has its own field in the connection dialog, so a bulk_read given here is parsed for validation but dropped before dbConnect, leaving the dedicated field as the single source of truth. The parameters are part of the pool key: two connections that differ only by their additional parameters are different connections and must not share a pooled handle. Verified against a live Oracle database: no parameters, prefetch=TRUE, prefetch=TRUE;stmt_cache=20 and bulk_write=5000 all connect and return multibyte data as UTF-8, and an unsupported key is rejected before connecting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 54d01bf commit 9692653

3 files changed

Lines changed: 132 additions & 18 deletions

File tree

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Package: exploratory
22
Type: Package
33
Title: R package for Exploratory
4-
Version: 16.0.21
4+
Version: 16.0.22
55
Date: 2026-07-28
66
Authors@R: c(person("Hideaki", "Hayashi", email = "hideaki@exploratory.io", role = c("aut", "cre")), person("Hide", "Kojima", email = "hide@exploratory.io", role = c("aut")), person("Kan", "Nishida", email = "kan@exploratory.io", role = c("aut")), person("Kei", "Saito", email = "kei@exploratory.io", role = c("aut")), person("Yosuke", "Yasuda", email = "double.y.919.quick@gmail.com", role = c("aut")))
77
URL: https://github.com/exploratory-io/exploratory_func

R/system.R

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,64 @@ applyOracleClientEnv <- function(explicitDir = Sys.getenv("EXPLORATORY_ORACLE_CL
10461046
# *.AL32UTF8 -> correct UTF-8.
10471047
# So the character set has to be forced, not merely defaulted: a user who set NLS_LANG for the
10481048
# ODBC based Oracle data source would otherwise silently corrupt Oracle (OCI) results.
1049+
# Parses the Oracle (OCI) additional parameters field ("key1=value1;key2=value2") into a list
1050+
# of arguments for ROracle::dbConnect.
1051+
#
1052+
# Unlike the ODBC based Oracle data source, where unrecognized keys are still handed to the
1053+
# driver, ROracle's dbConnect swallows anything it does not declare: the method signature is
1054+
# (username, password, dbname, prefetch, bulk_read, bulk_write, stmt_cache,
1055+
# external_credentials, sysdba, ...) and the ... is dropped rather than forwarded. A typo would
1056+
# therefore be silently ignored, so unknown keys are rejected here instead.
1057+
#
1058+
# Types are converted because ROracle rejects strings outright ("argument 'prefetch' must be a
1059+
# single logical value and cannot be 'TRUE'"), and it also refuses stmt_cache unless prefetch is
1060+
# enabled ("prefetch should be enabled for statement cache"), so that pairing is checked here to
1061+
# give a clearer message than the driver's.
1062+
ORACLE_OCI_LOGICAL_PARAMS <- c("prefetch", "external_credentials", "sysdba")
1063+
ORACLE_OCI_INTEGER_PARAMS <- c("bulk_read", "bulk_write", "stmt_cache")
1064+
1065+
parseOracleOCIAdditionalParams <- function(additionalParams) {
1066+
if (is.null(additionalParams) || length(additionalParams) == 0 ||
1067+
is.na(additionalParams[[1]]) || trimws(additionalParams[[1]]) == "") {
1068+
return(list())
1069+
}
1070+
entries <- strsplit(as.character(additionalParams[[1]]), ";", fixed = TRUE)[[1]]
1071+
entries <- trimws(entries)
1072+
entries <- entries[entries != ""]
1073+
supported <- c(ORACLE_OCI_LOGICAL_PARAMS, ORACLE_OCI_INTEGER_PARAMS)
1074+
result <- list()
1075+
for (entry in entries) {
1076+
if (!grepl("=", entry, fixed = TRUE)) {
1077+
stop(paste0("Invalid Oracle (OCI) additional parameter '", entry,
1078+
"'. Use the form key=value, separated by semicolons."))
1079+
}
1080+
key <- trimws(sub("=.*$", "", entry))
1081+
value <- trimws(sub("^[^=]*=", "", entry))
1082+
if (!(key %in% supported)) {
1083+
stop(paste0("Unknown Oracle (OCI) additional parameter '", key, "'. Supported parameters are: ",
1084+
paste(supported, collapse = ", "), "."))
1085+
}
1086+
if (key %in% ORACLE_OCI_LOGICAL_PARAMS) {
1087+
upperValue <- toupper(value)
1088+
if (!(upperValue %in% c("TRUE", "FALSE", "T", "F"))) {
1089+
stop(paste0("Oracle (OCI) additional parameter '", key, "' must be TRUE or FALSE, but was '", value, "'."))
1090+
}
1091+
result[[key]] <- upperValue %in% c("TRUE", "T")
1092+
} else {
1093+
number <- suppressWarnings(as.integer(value))
1094+
if (is.na(number)) {
1095+
stop(paste0("Oracle (OCI) additional parameter '", key, "' must be a whole number, but was '", value, "'."))
1096+
}
1097+
result[[key]] <- number
1098+
}
1099+
}
1100+
# ROracle fails the connection outright when a statement cache is requested without prefetch.
1101+
if (!is.null(result$stmt_cache) && result$stmt_cache > 0 && !isTRUE(result$prefetch)) {
1102+
stop("Oracle (OCI) additional parameter 'stmt_cache' requires 'prefetch=TRUE'.")
1103+
}
1104+
result
1105+
}
1106+
10491107
oracleOCINlsLang <- function(currentNlsLang = Sys.getenv("NLS_LANG")) {
10501108
if (is.null(currentNlsLang) || length(currentNlsLang) == 0 || is.na(currentNlsLang[[1]]) ||
10511109
currentNlsLang[[1]] == "") {
@@ -1062,7 +1120,8 @@ oracleOCINlsLang <- function(currentNlsLang = Sys.getenv("NLS_LANG")) {
10621120
# Single source of truth for the Oracle (OCI) connection pool key so that getDBConnection
10631121
# and clearDBConnection can never drift apart. Every component is normalized here because
10641122
# paste() silently drops NULL, which would produce a key that no clear call can match.
1065-
oracleOCIPoolKey <- function(host, port, databaseName, username, timezone, bulkRead, connectionString) {
1123+
oracleOCIPoolKey <- function(host, port, databaseName, username, timezone, bulkRead, connectionString,
1124+
additionalParams = "") {
10661125
normalize <- function(value) {
10671126
if (is.null(value) || length(value) == 0 || is.na(value[[1]])) "" else as.character(value[[1]])
10681127
}
@@ -1071,7 +1130,8 @@ oracleOCIPoolKey <- function(host, port, databaseName, username, timezone, bulkR
10711130
timezone <- "UTC" # matches the default applied when connecting.
10721131
}
10731132
paste("oracleoci", normalize(host), normalize(port), normalize(databaseName),
1074-
normalize(username), timezone, normalize(bulkRead), normalize(connectionString), sep = ":")
1133+
normalize(username), timezone, normalize(bulkRead), normalize(connectionString),
1134+
normalize(additionalParams), sep = ":")
10751135
}
10761136

10771137
# Oracle rejects a trailing semicolon through OCI, unlike most SQL consoles, so drop one
@@ -1969,7 +2029,8 @@ getDBConnection <- function(type, host = NULL, port = "", databaseName = "", use
19692029
if (timezone == "") {
19702030
timezone <- "UTC" # if timezone is not provided use UTC as default, same as the ODBC based Oracle data source.
19712031
}
1972-
key <- oracleOCIPoolKey(host, port, databaseName, username, timezone, bulkRead, connectionString)
2032+
key <- oracleOCIPoolKey(host, port, databaseName, username, timezone, bulkRead, connectionString,
2033+
additionalParams)
19732034
# Only reuse a pooled connection when connection pooling is on (e.g. while the data source
19742035
# dialog is open), and always probe it first. Same reasoning as the oracle branch above. (tam#36429)
19752036
conn <- NULL
@@ -2025,15 +2086,23 @@ getDBConnection <- function(type, host = NULL, port = "", databaseName = "", use
20252086
on.exit({
20262087
if (previousNlsLang == "") Sys.unsetenv("NLS_LANG") else Sys.setenv(NLS_LANG = previousNlsLang)
20272088
}, add = TRUE)
2028-
conn <- ROracle::dbConnect(
2029-
ROracle::Oracle(unicode_as_utf8 = TRUE),
2030-
username = username,
2031-
password = password,
2032-
dbname = dbname,
2033-
# Rows fetched per OCI round trip. This is the main reason this data source is faster
2034-
# than the ODBC based one, so it is exposed on the connection as a user setting.
2035-
bulk_read = bulkReadRows
2036-
)
2089+
# Additional parameters are validated and type converted before they reach the driver.
2090+
# bulk_read has its own field on the connection, so that field wins and an additional
2091+
# parameter of the same name is ignored rather than silently overriding the visible value.
2092+
extraArgs <- parseOracleOCIAdditionalParams(additionalParams)
2093+
extraArgs$bulk_read <- NULL
2094+
conn <- do.call(ROracle::dbConnect, c(
2095+
list(
2096+
ROracle::Oracle(unicode_as_utf8 = TRUE),
2097+
username = username,
2098+
password = password,
2099+
dbname = dbname,
2100+
# Rows fetched per OCI round trip. This is the main reason this data source is faster
2101+
# than the ODBC based one, so it is exposed on the connection as a user setting.
2102+
bulk_read = bulkReadRows
2103+
),
2104+
extraArgs
2105+
))
20372106
if (user_env$pool_connection) {
20382107
connection_pool[[key]] <- conn
20392108
}
@@ -2140,7 +2209,8 @@ clearDBConnection <- function(type, host = NULL, port = NULL, databaseName, user
21402209
key <- paste("oracle", host, port, databaseName, username, timezone, sep = ":")
21412210
} else if (type %in% c("oracleoci")) {
21422211
# Built through the same helper getDBConnection uses, so the keys cannot drift apart.
2143-
key <- oracleOCIPoolKey(host, port, databaseName, username, timezone, bulkRead, connectionString)
2212+
key <- oracleOCIPoolKey(host, port, databaseName, username, timezone, bulkRead, connectionString,
2213+
additionalParams)
21442214
}
21452215
rm(list = key, envir = connection_pool) # Even if there is no matching key, this is harmless.
21462216
}
@@ -2309,13 +2379,15 @@ queryMySQL <- function(host, port, databaseName, username, password, numOfRows =
23092379
#' @param connectionString (optional) TNS alias, full connect descriptor, or EZConnect string.
23102380
#' When provided it is used as is and host, port and databaseName are ignored.
23112381
queryOracleOCI <- function(host = "", port = 1521, databaseName = "", username = "", password = "",
2312-
numOfRows = -1, query, timezone = "", bulkRead = 200000, connectionString = "", ...) {
2382+
numOfRows = -1, query, timezone = "", bulkRead = 200000, connectionString = "",
2383+
additionalParams = "", ...) {
23132384
if (!requireNamespace("ROracle")) { stop("package ROracle must be installed.") }
23142385
if (!requireNamespace("DBI")) { stop("package DBI must be installed.") }
23152386

23162387
conn <- getDBConnection(type = "oracleoci", host = host, port = port, databaseName = databaseName,
23172388
username = username, password = password, timezone = timezone,
2318-
bulkRead = bulkRead, connectionString = connectionString)
2389+
bulkRead = bulkRead, connectionString = connectionString,
2390+
additionalParams = additionalParams)
23192391
tryCatch({
23202392
query <- convertUserInputToUtf8(query)
23212393
# set envir = parent.frame() to get variables from users environment, not package environment
@@ -2328,7 +2400,8 @@ queryOracleOCI <- function(host = "", port = 1521, databaseName = "", username =
23282400
}, error = function(err) {
23292401
# clear connection in pool so that new connection will be used for the next try
23302402
clearDBConnection("oracleoci", host, port, databaseName, username, timezone = timezone,
2331-
bulkRead = bulkRead, connectionString = connectionString)
2403+
bulkRead = bulkRead, connectionString = connectionString,
2404+
additionalParams = additionalParams)
23322405
stop(err)
23332406
})
23342407
ROracle::dbClearResult(resultSet)

tests/testthat/test_system.R

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1233,13 +1233,15 @@ test_that("oracleOCIPoolKey builds identical keys from getDBConnection and clear
12331233
fromGet <- exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "", 10000, NULL)
12341234
fromClear <- exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "UTC", 10000, "")
12351235
expect_equal(fromGet, fromClear)
1236-
expect_equal(fromGet, "oracleoci:host:1521:svc:user:UTC:10000:")
1236+
# The trailing empty component is the additional parameters, which default to none.
1237+
expect_equal(fromGet, "oracleoci:host:1521:svc:user:UTC:10000::")
12371238

12381239
# Every component that changes the connection must change the key.
12391240
expect_false(fromGet == exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "Asia/Tokyo", 10000, ""))
12401241
expect_false(fromGet == exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "", 5000, ""))
12411242
expect_false(fromGet == exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "", 10000, "tns_alias"))
12421243
expect_false(fromGet == exploratory:::oracleOCIPoolKey("other", 1521, "svc", "user", "", 10000, ""))
1244+
expect_false(fromGet == exploratory:::oracleOCIPoolKey("host", 1521, "svc", "user", "", 10000, "", "prefetch=TRUE"))
12431245
})
12441246

12451247
test_that("findOracleClientLibDir handles both Oracle client layouts", {
@@ -1321,3 +1323,42 @@ test_that("oracleOCINlsLang always forces the AL32UTF8 character set", {
13211323
# A territory containing a dot must not confuse the character set split.
13221324
expect_equal(exploratory:::oracleOCINlsLang("A_B.C.JA16SJIS"), "A_B.C.AL32UTF8")
13231325
})
1326+
1327+
test_that("parseOracleOCIAdditionalParams accepts an empty setting", {
1328+
expect_equal(exploratory:::parseOracleOCIAdditionalParams(""), list())
1329+
expect_equal(exploratory:::parseOracleOCIAdditionalParams(NULL), list())
1330+
expect_equal(exploratory:::parseOracleOCIAdditionalParams(NA), list())
1331+
expect_equal(exploratory:::parseOracleOCIAdditionalParams(" "), list())
1332+
})
1333+
1334+
test_that("parseOracleOCIAdditionalParams converts values to the types ROracle expects", {
1335+
# dbConnect wants a logical for prefetch and an integer for stmt_cache. Passing the raw
1336+
# strings through would silently change behaviour, so they are converted here.
1337+
res <- exploratory:::parseOracleOCIAdditionalParams("prefetch=TRUE;stmt_cache=20")
1338+
expect_identical(res$prefetch, TRUE)
1339+
expect_identical(res$stmt_cache, 20L)
1340+
expect_identical(exploratory:::parseOracleOCIAdditionalParams("sysdba=false")$sysdba, FALSE)
1341+
# Written by hand, so tolerate case and spacing.
1342+
expect_identical(exploratory:::parseOracleOCIAdditionalParams(" prefetch = true ")$prefetch, TRUE)
1343+
})
1344+
1345+
test_that("parseOracleOCIAdditionalParams rejects anything dbConnect would silently drop", {
1346+
# ROracle::dbConnect ignores arguments it does not declare, so an unknown key would look
1347+
# like it took effect. Reject it here instead.
1348+
expect_error(exploratory:::parseOracleOCIAdditionalParams("FWC=T"), "Unknown Oracle \\(OCI\\) additional parameter")
1349+
expect_error(exploratory:::parseOracleOCIAdditionalParams("prefetch"), "Use the form key=value")
1350+
expect_error(exploratory:::parseOracleOCIAdditionalParams("prefetch=YES"), "must be TRUE or FALSE")
1351+
expect_error(exploratory:::parseOracleOCIAdditionalParams("prefetch=TRUE;stmt_cache=abc"), "must be a whole number")
1352+
})
1353+
1354+
test_that("parseOracleOCIAdditionalParams requires prefetch for a statement cache", {
1355+
# ROracle only honours stmt_cache in prefetch mode.
1356+
expect_error(exploratory:::parseOracleOCIAdditionalParams("stmt_cache=20"), "requires 'prefetch=TRUE'")
1357+
expect_silent(exploratory:::parseOracleOCIAdditionalParams("prefetch=TRUE;stmt_cache=20"))
1358+
})
1359+
1360+
test_that("oracleOCIPoolKey separates connections that differ only by additional parameters", {
1361+
base <- exploratory:::oracleOCIPoolKey("h", 1521, "SVC", "u", "UTC", 200000, "", "")
1362+
withPrefetch <- exploratory:::oracleOCIPoolKey("h", 1521, "SVC", "u", "UTC", 200000, "", "prefetch=TRUE")
1363+
expect_false(identical(base, withPrefetch))
1364+
})

0 commit comments

Comments
 (0)