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

Adding lazy relation -> data.frame conversion for R client #5181

Merged
merged 22 commits into from
Nov 23, 2022

Conversation

hannes
Copy link
Member

@hannes hannes commented Nov 3, 2022

We add functionality to convert a R DuckDB relation object to a data.frame without actually computing the query result using lazy ALTREP vectors. This works great except for list/struct/blob columns. There is hope R will add list ALTREP support soon-ish, which should remove the limitation.

@hannes hannes requested a review from krlmlr November 3, 2022 10:23
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.

🎉

tools/rpkg/src/reltoaltrep.cpp Outdated Show resolved Hide resolved
tools/rpkg/src/reltoaltrep.cpp Show resolved Hide resolved
tools/rpkg/src/reltoaltrep.cpp Outdated Show resolved Hide resolved
tools/rpkg/src/reltoaltrep.cpp Show resolved Hide resolved
@hannes
Copy link
Member Author

hannes commented Nov 4, 2022

@krlmlr do we need to remove the examples for the un-exported methods? R CMD check does not seem to like them.

@krlmlr
Copy link
Collaborator

krlmlr commented Nov 4, 2022 via email

relaltrepdf-2 # Please enter a commit message to explain why this merge is
necessary, # especially if it merges an updated upstream into a topic
branch. # # Lines starting with '#' will be ignored, and an empty message
aborts # the commit.
@hannes
Copy link
Member Author

hannes commented Nov 4, 2022

I'm also wondering why those things have to be private in the first place. Maybe its better to mark them experimental.

@hannes hannes changed the title Making relational API private and adding lazy data frames Adding lazy relation -> data.frame conversion for R client Nov 4, 2022
@krlmlr
Copy link
Collaborator

krlmlr commented Nov 5, 2022

We agreed that keeping the functions private will make it easier for us later to use the new relational package. What is the advantage of keeping them exported?

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.

I don't have a strong opinion regarding reexport, I see there is now a message that explicitly alerts users. Would you like a beautiful orange badge, like in https://dplyr.tidyverse.org/reference/rows.html?

@hannes
Copy link
Member Author

hannes commented Nov 18, 2022

Would you like a beautiful orange badge, like in https://dplyr.tidyverse.org/reference/rows.html?

Since this pulls in the lifecycle package I am adding @noRd as discussed

@hannes hannes merged commit e56ace4 into duckdb:master Nov 23, 2022
@hannes hannes deleted the relaltrepdf-2 branch March 1, 2023 10:05
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.

None yet

2 participants