-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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(python): streamline adbc
connectivity, adding snowflake support
#9600
feat(python): streamline adbc
connectivity, adding snowflake support
#9600
Conversation
@alexander-beedie let me know if you want help testing this on a Snowflake instance. |
Sounds good! If you're able to grab the PR and try it out in advance that would be much appreciated :) |
@jonashaag will you test this PR before we merge it? |
I can test today. |
For the purposes of this PR, I think we can say that it works. A few problems:
|
Thanks for testing! Looks good to me, given that we just pass-through to the ADBC driver. Does snowflake not allow you to specify the warehouse in the URI? Bit of a shame! If so, and given there isn’t an “executemany”, maybe we could sequentially execute ADBC statements given as a list… can return to that after a think 🤔 |
It does allow you to, that's how I ended up testing this. Took me a while to think of that though. |
Ahh, ok - I thought you had to issue a separate "USE WAREHOUSE ..." statement :) If you've got a sample URI with the warehouse option we could add an example of it into the docstring to help out any other snowflake users 👍 |
Sorry for late reply, here is what I ended up using
|
Thx :) The ADBC docs were lacking, but I eventually found the URI param/option referenced in snowflake's docs for sqlalchemy: https://docs.snowflake.com/en/developer-guide/python-connector/sqlalchemy#additional-connection-parameters |
Should close #9569, by better generalising
adbc
module inference/connection.Will need confirmation about snowflake, as I don't actually have access to an instance, but our existing support for postgres/sqlite is confirmed to work with the updated code (I spun-up a local docker postgres image to validate it).