Skip to content
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.

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

feat: Map NA to SQLNULL #143

Merged
merged 2 commits into from
Apr 24, 2024
Merged

feat: Map NA to SQLNULL #143

merged 2 commits into from
Apr 24, 2024

Conversation

krlmlr
Copy link
Collaborator

@krlmlr krlmlr commented Apr 24, 2024

Is this a reasonable change? This helps with at least one package with duckplyr.

@krlmlr krlmlr requested a review from hannes April 24, 2024 07:36
@krlmlr
Copy link
Collaborator Author

krlmlr commented Apr 24, 2024

As it stands, this is a breaking change:

v0.10.1

library(duckdb)
#> Loading required package: DBI
con <- dbConnect(duckdb::duckdb())

out <- dbGetQuery(con, "SELECT ? AS a", params = list(NA))
dput(out$a)
#> NA

Created on 2024-04-24 with reprex v2.1.0

v0.10.2 RC

...
dput(out$a)
#> NA

Created on 2024-04-24 with reprex v2.1.0

This PR

...
dput(out$a)
#> NA_integer_

Any other failure modes you could think of, Hannes?

@krlmlr
Copy link
Collaborator Author

krlmlr commented Apr 24, 2024

With the latest change:

...
dput(out$a)
#> NA

Created on 2024-04-24 with reprex v2.1.0

Still wondering if SQLNULL is the way to go here. These are the semantics of dplyr, anyway: whenever we see a single NA, it is treated as an untyped missing value.

@krlmlr
Copy link
Collaborator Author

krlmlr commented Apr 24, 2024

Need to see about the R-devel checks. Merging now, review still appreciated.

@krlmlr krlmlr merged commit 8feebf8 into main Apr 24, 2024
21 of 23 checks passed
@krlmlr krlmlr deleted the f-untyped-null branch April 24, 2024 18:40
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.

1 participant