fvsOL: fix RSQLite drift in temp-table writes#29
Open
RoopsyDaisy wants to merge 2 commits into
Open
Conversation
On current RSQLite, writing a temp table by passing a schema-qualified
name as `DBI::SQL("temp.X")` fails:
Error: Named parameters not used in query: name
Reproduced on R 4.6.0, RSQLite 3.53.1, DBI 1.3.0 (P3M snapshot
2026-05-27). The DBI-idiomatic form
dbWriteTable(con, "X", df, temporary = TRUE, overwrite = TRUE)
works and keeps the table queryable as `temp.X`, so all downstream
`select ... from temp.X` reads continue to resolve correctly.
This converts all 11 affected call sites:
server.R (6)
fvsRunUtilities.R (2)
externalCallable.R (2)
writeKeyFile.R (1)
Each converted write keeps the base name its downstream queries
reference (e.g. write "RunStds" -> read temp.RunStds). No behaviour
change to the temp-table contents; only the call form.
366d08d to
bfe7edb
Compare
The "Output .xlsx for current run" download handler (output$dlFVSOutxlsx) built its CaseID filter table with dbWriteTable(name=DBI::SQL(casesToGet), ...). Passing a DBI::SQL() object as the table name throws "Named parameters not used in query: name" on RSQLite >= 3 -- the same error already fixed at the 11 temp.* sites -- so the export crashed the moment a user clicked the button. It also carried an overwirte= typo (overwrite never applied). The handler attached a per-pid/uuid :memory: database solely to hold that one table, then detached it. Replace that with a session-private temporary table (the same temporary=TRUE pattern as the other sites; temp tables are per- connection, so no cross-session collision), drop the now-unused attach/detach, and fix the typo. https://claude.ai/code/session_011C6wxKosiwHnEYvAPJfmcW
wagnerds
reviewed
Jun 4, 2026
| dbExecute(dbGlb$dbOcon,paste0("attach database ':memory:' as ",tmp)) | ||
| casesToGet = paste0(tmp,".casesToGet") | ||
| dbWriteTable(dbGlb$dbOcon,name=DBI::SQL(casesToGet),value=cases,overwirte=TRUE) | ||
| casesToGet = "temp.casesToGet" |
Collaborator
There was a problem hiding this comment.
Thank you for your contribution and making these important updates. I am working on determining the reasoning for originally naming the temporary database with the run id and pid at this location and whether the change to a much simpler identifier will have any unanticipated effects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On current RSQLite, writing a temp table by passing a schema-qualified
name as
DBI::SQL("temp.X")fails:Versions (Posit Package Manager snapshot 2026-05-27): R 4.6.0,
RSQLite 3.53.1, DBI 1.3.0, bundled SQLite 3.53.1.
The DBI-idiomatic form works and the table is still queryable as
temp.X:This breaks fvsOL at runtime (stand selection, keyword-file generation,
and the external-callable API all build
temp.*helper tables). ThisPR converts all 11 affected sites:
server.R(6)writeKeyFile.R(1)externalCallable.R(2)fvsRunUtilities.R(2)Each converted write keeps the base name its downstream queries
reference (e.g. write
"RunStds"→ readtemp.RunStds).Update — also covers the Excel-export path (2nd commit). RSQLite ≥ 3 rejects any DBI::SQL() object passed as the dbWriteTable name argument (Error: Named parameters not used in query: name). Beyond the 11 temp.* sites in the first commit, the "Output .xlsx for current run" handler (output$dlFVSOutxlsx in server.R) hit the same error via a dynamic DBI::SQL(casesToGet) name — and carried an overwirte= typo, so overwrite never applied. The second commit converts it to a temporary table (the same temporary=TRUE form as the other sites), drops the now-redundant attached-:memory:-db it used for that one table, and fixes the typo. All DBI::SQL()-as-name dbWriteTable sites in fvsOL (12 total) are now addressed. Verified end-to-end: the .xlsx export works in the WebGUI.