databricks / databricks-sql-go

Golang database/sql driver for Databricks SQL.
Apache License 2.0
34 stars 37 forks source link

feat: Update Apache Arrow to `v16` #216

Closed candiduslynx closed 2 months ago

candiduslynx commented 2 months ago

Instead of https://github.com/databricks/databricks-sql-go/pull/209

candiduslynx commented 2 months ago

@kravets-levko would you mind also reviewing this PR & releasing afterwards? I've been using a fork at https://github.com/cloudquery/databricks-sql-go (cqmain branch) with #216 & #217 merged into to achieve everything required for the code I'm developing.

The major bump to v15 just updates the library code, the underlying arrow data (wire protocol) stays the same.

kravets-levko commented 2 months ago

@candiduslynx I totally understand your need in this upgrade, however I'm a bit new to this project in particular and Go language in general. So I need @andrefurlan-db or @rcypher-databricks approval, and then I'll merge and ship this change. Sorry, and thank you for understanding

yunbodeng-db commented 2 months ago

@andrefurlan-db can you help out on this one?

candiduslynx commented 2 months ago

@yunbodeng-db @andrefurlan-db @kravets-levko I've updated the PR as v16 was released. I'd like to see this merged sooner than later, though.

candiduslynx commented 2 months ago

@kravets-levko thanks for the review! Is there a way to expedite the merge & release of this code along with #207?

kravets-levko commented 2 months ago

@candiduslynx databricks/databricks-sql-go#207 should be already available in v1.5.4. For this PR - I think I'll add it to the next release, hopefully will be available on Monday-Tuesday

candiduslynx commented 2 months ago

@kravets-levko it's great to see this merged, thanks! I'm eagerly waiting for the new release 🙂

kravets-levko commented 2 months ago

@candiduslynx v1.5.5 is out - please give it a try and let me know if everything is okay. Thank you for patience

kravets-levko commented 1 month ago

@candiduslynx sorry for a bad news, but turns out that Arrow upgrade caused issues for library users. Library offers a feature that allows to access raw Arrow batches, and with this upgrade behavior of that feature changed. After discussion with other team members, we decided to revert this upgrade for now. v1.5.5 will remain unchanged, though, so you can keep using it

candiduslynx commented 1 month ago

@kravets-levko thanks for notifying! I think you need to add some tests to outline the expected behaviour then. However, I wonder what are the details of the changed behaviour that isn't deemed correct by clients? Is it just a requirement to update the library?