Skip to content

feat: Add temporary argument to duckdb_read_csv()#223

Merged
krlmlr merged 5 commits into
duckdb:mainfrom
ThomasSoeiro:main
Oct 14, 2024
Merged

feat: Add temporary argument to duckdb_read_csv()#223
krlmlr merged 5 commits into
duckdb:mainfrom
ThomasSoeiro:main

Conversation

@ThomasSoeiro
Copy link
Copy Markdown
Contributor

Here is a try for #142

@krlmlr: You suggested to modify the SQL and to use tbl_function().

I do not understand why it is needed. Cannot we just pass temporary to dbCreateTable() like I did? Can you please clarify?

Warning: this example will overwrite test.db and iris.csv files on your disk if present.

library(duckdb)
con <- dbConnect(duckdb(), dbdir = "test.db")

write.csv(iris, "iris.csv", row.names = FALSE)

duckdb_read_csv(con, "iris_temp_false", "iris.csv")
duckdb_read_csv(con, "iris_temp_true", "iris.csv", temporary = TRUE)

dbListTables(con)
# [1] "iris_temp_false" "iris_temp_true" 

dbReadTable(con, "iris_temp_false") |> head()
#   Sepal.Length Sepal.Width Petal.Length Petal.Width Species
# 1          5.1         3.5          1.4         0.2  setosa
# 2          4.9         3.0          1.4         0.2  setosa
# 3          4.7         3.2          1.3         0.2  setosa
# 4          4.6         3.1          1.5         0.2  setosa
# 5          5.0         3.6          1.4         0.2  setosa
# 6          5.4         3.9          1.7         0.4  setosa

dbReadTable(con, "iris_temp_true") |> head()
#   Sepal.Length Sepal.Width Petal.Length Petal.Width Species
# 1          5.1         3.5          1.4         0.2  setosa
# 2          4.9         3.0          1.4         0.2  setosa
# 3          4.7         3.2          1.3         0.2  setosa
# 4          4.6         3.1          1.5         0.2  setosa
# 5          5.0         3.6          1.4         0.2  setosa
# 6          5.4         3.9          1.7         0.4  setosa

dbDisconnect(con)

Check that the temporary table is gone:

con <- dbConnect(duckdb::duckdb(), dbdir = "test.db")

dbListTables(con)
# [1] "iris_temp_false"

dbDisconnect(con)

Copy link
Copy Markdown
Collaborator

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks, I was confused and thought there's a COPY TEMPORARY ... FROM syntax for disambiguation. There doesn't seem to be one: https://duckdb.org/docs/sql/statements/copy.html#copy--from .

Can you please also add a test?

Comment thread R/csv.R Outdated
#' dbDisconnect(con)
duckdb_read_csv <- function(conn, name, files, header = TRUE, na.strings = "", nrow.check = 500,
delim = ",", quote = "\"", col.names = NULL, lower.case.names = FALSE, sep = delim, transaction = TRUE, ...) {
duckdb_read_csv <- function(conn, name, files, ..., header = TRUE, na.strings = "", nrow.check = 500,
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.

We'd want to check that the ellipsis is actually empty. This is touching the same file, but the move of the ellipsis and the check could also be a separate PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have added a warning. Is it what you wanted?

@ThomasSoeiro

This comment was marked as off-topic.

@krlmlr

This comment was marked as off-topic.

@krlmlr
Copy link
Copy Markdown
Collaborator

krlmlr commented Oct 4, 2024

Thanks!

@krlmlr krlmlr changed the title add temporary parameter to duckdb_read_csv() feat: Add temporary parameter to duckdb_read_csv() Oct 4, 2024
@ThomasSoeiro
Copy link
Copy Markdown
Contributor Author

Hi @krlmlr,
Do we need anything else so that this PR can be merged?
Thanks!

@krlmlr krlmlr changed the title feat: Add temporary parameter to duckdb_read_csv() feat: Add temporary argument to duckdb_read_csv() Oct 14, 2024
@krlmlr krlmlr merged commit a60fe7a into duckdb:main Oct 14, 2024
@krlmlr
Copy link
Copy Markdown
Collaborator

krlmlr commented Oct 14, 2024

Thanks for the nudge. I thought I had turned on auto-merge for this PR.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants