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

Implement Table.Reader for Adbc #132

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josevalim
Copy link
Member

This would allow it to be used conveniently in Livebook for querying without Explorer.

@josevalim
Copy link
Member Author

@cocoa-xu I think we could improve our APIs a bit.

  • I would expect Result.to_list to return a list but that doesn't make sense, so I believe the function should be removed?
  • I would expect Column.to_list to return a list and do the whole view handling that is currently done inside Result.list_to_map

However, if we change the behaviour of Column.to_list, is there a use case for its current implementation, where it handles some lists but not all (i.e. not views)?

@cocoa-xu
Copy link
Member

@cocoa-xu I think we could improve our APIs a bit.

  • I would expect Result.to_list to return a list but that doesn't make sense, so I believe the function should be removed?
  • I would expect Column.to_list to return a list and do the whole view handling that is currently done inside Result.list_to_map

However, if we change the behaviour of Column.to_list, is there a use case for its current implementation, where it handles some lists but not all (i.e. not views)?

Yea I think we can make this happen, let me check the failed test cases on CI

@cocoa-xu
Copy link
Member

cocoa-xu commented Mar 19, 2025

However, if we change the behaviour of Column.to_list, is there a use case for its current implementation, where it handles some lists but not all (i.e. not views)?

Okay I think it's fine to make this change, and it's also probably okay to just move Adbc.Result.list_to_map/1 to Adbc.Column.to_list/1

Essentially, we can just eliminate Adbc.Result.list_to_map/1 entirely, and then implement Adbc.Column.to_list/1 for the remaining types (like :list, :struct and probably all the primitive data types)

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.

2 participants