Skip to content

fvsOL: fix RSQLite drift in temp-table writes#29

Open
RoopsyDaisy wants to merge 2 commits into
USDAForestService:mainfrom
RoopsyDaisy:upstream-pr/fvsol-rsqlite-temp-tables
Open

fvsOL: fix RSQLite drift in temp-table writes#29
RoopsyDaisy wants to merge 2 commits into
USDAForestService:mainfrom
RoopsyDaisy:upstream-pr/fvsol-rsqlite-temp-tables

Conversation

@RoopsyDaisy
Copy link
Copy Markdown

@RoopsyDaisy RoopsyDaisy commented Jun 1, 2026

On current RSQLite, writing a temp table by passing a schema-qualified
name as DBI::SQL("temp.X") fails:

library(DBI); library(RSQLite)
con <- dbConnect(SQLite(), ":memory:")
dbWriteTable(con, DBI::SQL("temp.Grps"), data.frame(Stand_ID = "", Grp = ""))
#> Error: Named parameters not used in query: name

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:

dbWriteTable(con, "Grps", df, temporary = TRUE, overwrite = TRUE)

This breaks fvsOL at runtime (stand selection, keyword-file generation,
and the external-callable API all build temp.* helper tables). This
PR 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" → read temp.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.

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.
@RoopsyDaisy RoopsyDaisy force-pushed the upstream-pr/fvsol-rsqlite-temp-tables branch from 366d08d to bfe7edb Compare June 2, 2026 18:03
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
Comment thread fvsOL/R/server.R
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"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants