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: n_distinct() supports na.rm = TRUE with a single vector argument again #216

Merged
merged 1 commit into from
Sep 14, 2024

Conversation

lschneiderbauer
Copy link
Contributor

closes #204 .

Copy link
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.

Nice, thanks!


# https://duckdb.org/docs/sql/data_types/struct.html#creating-structs-with-the-row-function
str_struct <- paste0("row(", paste0(list(...), collapse = ", "), ")")
return(sql(paste0("COUNT(DISTINCT ", cols[[1]], ")")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems totally fine to omit the return() here and below. Not a breaker.

# Why not use ROW() as well? Because duckdb's FILTER clause does not support
# a windowing context as of now: https://duckdb.org/docs/sql/query_syntax/filter.html
if (length(cols) > 1) {
stop("n_distinct(): Only one vector argument is currently supported when `na.rm = TRUE`.", call. = FALSE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't understand "vector argument" when seeing this error, but it makes sense in the context of this PR and it's also easy to find.

@krlmlr krlmlr changed the title n_distinct(): restore na.rm = TRUE capability with single vector argument feat: n_distinct() supports na.rm = TRUE with a single vector argument again Sep 14, 2024
@krlmlr krlmlr merged commit 698fc54 into duckdb:main Sep 14, 2024
17 checks passed
@krlmlr
Copy link
Collaborator

krlmlr commented Sep 14, 2024

Thanks!

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.

Duckdb-R n_distinct translation doesn't allow management of NA values
2 participants