Skip to content

Set auto commit to true for batched queries #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
csafreen merged 14 commits into from
Nov 11, 2025
Merged

Set auto commit to true for batched queries #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3da792f
Set auto commit to true
csafreen Oct 13, 2025
331476a
Update jar file
csafreen Oct 13, 2025
452b7e2
Update jar file
csafreen Oct 13, 2025
cd94785
Test
csafreen Oct 13, 2025
4b8705f
test
csafreen Oct 13, 2025
a612c0c
Test
csafreen Oct 14, 2025
0ca4e4f
Test
csafreen Oct 14, 2025
4a95987
Test
csafreen Oct 14, 2025
1259754
parse json
csafreen Oct 14, 2025
7866c85
Test print
csafreen Oct 14, 2025
517c080
Update parse
csafreen Oct 14, 2025
d54a4b2
Test
csafreen Oct 15, 2025
e381b0f
Removing test logs
csafreen Oct 15, 2025
2bca9bd
Remove hard coded value from java file
csafreen Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions R/Andromeda.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ lowLevelQuerySqlToAndromeda.default <- function(connection,
connection@jConnection,
query,
dbms(connection),
FALSE
TRUE
)

on.exit(rJava::.jcall(batchedQuery, "V", "clear"))
Expand Down Expand Up @@ -241,7 +241,6 @@ querySqlToAndromeda <- function(connection,
if (packageVersion("Andromeda") < "0.6.0") {
stop(sprintf("Andromeda version 0.6.0 or higher required, but version %s found", packageVersion("Andromeda")))
}

# Calling splitSql, because this will also strip trailing semicolons (which cause Oracle to crash).
sqlStatements <- SqlRender::splitSql(sql)
if (length(sqlStatements) > 1) {
Expand Down
2 changes: 1 addition & 1 deletion R/DBI.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ setMethod(
conn@jConnection,
statement,
dbms,
FALSE
TRUE
),
error = function(error) {
# Rethrowing error to avoid 'no field, method or inner class called 'use_cli_format''
Expand Down
12 changes: 10 additions & 2 deletions R/InsertTable.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ insertTable.default <- function(connection,
useMppBulkLoad = Sys.getenv("USE_MPP_BULK_LOAD"),
progressBar = FALSE,
camelCaseToSnakeCase = FALSE) {

if (is(connection, "Pool")) {
connection <- pool::poolCheckout(connection)
on.exit(pool::poolReturn(connection))
Expand Down Expand Up @@ -334,7 +335,6 @@ insertTable.default <- function(connection,
tempEmulationSchema = tempEmulationSchema
)
batchSize <- 10000

if (nrow(data) > 0) {
if (progressBar) {
pb <- txtProgressBar(style = 3)
Expand Down Expand Up @@ -368,7 +368,8 @@ insertTable.default <- function(connection,
} else if (is(column, "Date")) {
rJava::.jcall(batchedInsert, "V", "setDate", i, as.character(column))
} else {
rJava::.jcall(batchedInsert, "V", "setString", i, as.character(column))
column <- escapeJson(column)
Copy link
Contributor

@alicia-koh alicia-koh Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can check why this change is needed? Because it looks like it will replace all the single quotes and semicolons if the data is string?

Copy link
Contributor Author

@csafreen csafreen Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cohort generator module is failing to upload the results specifically for cg_cohort_definition table.
The table has a column called "sql_command" which has the queries run during the module execution.
The queries has ";" which is causing the import to fail. Thats why removed it.

Copy link
Contributor Author

@csafreen csafreen Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quotes also needed to be escaped before inserting into db as the column value is by default enclosed in a single quote.

Copy link
Contributor

@alicia-koh alicia-koh Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But besides cohort gen module other ohdsi packages (not sure which) could be using this function? Wouldn't this change the data if it has single quotes and a ;?

Copy link
Contributor Author

@csafreen csafreen Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since its an insert, escaping single quote should be fine.
semicolon will most likely be in texts.
Will have to add custom check there with table name for others to not be effected.
But if the text contains semicolon then duckdb will have an issue while insert for all tables.

Copy link
Contributor

@alicia-koh alicia-koh Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can control this by setting an env variable from strategus plugin if Cohort generator module is called

Copy link
Contributor Author

@csafreen csafreen Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But any insert to a table for a text column with semicolon will fail with trex connection.

rJava::.jcall(batchedInsert, "V", "setString", i, column)
}
return(NULL)
}
Expand Down Expand Up @@ -401,6 +402,7 @@ insertTable.DatabaseConnectorDbiConnection <- function(connection,
useMppBulkLoad = Sys.getenv("USE_MPP_BULK_LOAD"),
progressBar = FALSE,
camelCaseToSnakeCase = FALSE) {

if (!is.null(oracleTempSchema) && oracleTempSchema != "") {
warn("The 'oracleTempSchema' argument is deprecated. Use 'tempEmulationSchema' instead.",
.frequency = "regularly",
Expand Down Expand Up @@ -484,3 +486,9 @@ convertLogicalFields <- function(data) {
}
return(data)
}

escapeJson <- function(json) {
json <- gsub("'", "''", json, fixed = TRUE) # Escape single quotes
json <- gsub(";", "", json, fixed = TRUE) # Remove semicolons
return(json)
}
Binary file modified inst/java/DatabaseConnector.jar
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletion java/org/ohdsi/databaseConnector/BatchedInsert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public boolean executeBatch() throws SQLException, ParseException {
private boolean executeBigQueryBatch() throws SQLException, ParseException {
checkColumns();
try {
trySettingAutoCommit(false);
trySettingAutoCommit(true);

int offset = 0;
while (offset < rowCount) {
Expand Down
Loading