Skip to content

Conversation

@josevalim
Copy link
Contributor

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

@josevalim
Copy link
Contributor 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
Contributor

@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
Contributor

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)

@josevalim josevalim force-pushed the jv-implement-table-reader branch from 80b033a to 754c916 Compare May 31, 2025 14:00
@josevalim josevalim merged commit 1174177 into main May 31, 2025
3 checks passed
@josevalim
Copy link
Contributor Author

💚 💙 💜 💛 ❤️

@josevalim josevalim deleted the jv-implement-table-reader branch May 31, 2025 14:32
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.

3 participants