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

expose connectorx 'protocol' param to "read_sql" so it can work with redshift #2003

Merged
merged 6 commits into from
Dec 6, 2021
Merged

expose connectorx 'protocol' param to "read_sql" so it can work with redshift #2003

merged 6 commits into from
Dec 6, 2021

Conversation

alexander-beedie
Copy link
Collaborator

Related to (but not dependent on) sfu-db/connector-x/#187. This is a small patch that adds the 'protocol' param to polars' read_sql so that it can be passed-through to connectorx; this is required for redshift backends as the param has to be set to a non-default value ('cursor' instead of 'binary'), otherwise loading fails. No change to default behaviour or other parameters.

BEFORE

>>> # fails to execute against redshift dbs
>>> df = pl.read_sql( connection_uri='postgresql://...redshiftdb...', sql='SELECT * FROM table' )
RuntimeError: db error: ERROR: syntax error at or near "("

AFTER

>>> # successfully loads redshift data into polars frame
>>> df = pl.read_sql( connection_uri='postgresql://...redshiftdb...', sql='SELECT * FROM table', protocol='cursor' )

Also added some additional test coverage for read_sql, and made some trivial docstring updates (in line with the connectorx patch referenced above). Thanks!

@alexander-beedie
Copy link
Collaborator Author

(apologies for the slightly noisy commit; happy to squash it or rebase/recommit if you prefer ;)

@ritchie46
Copy link
Member

(apologies for the slightly noisy commit; happy to squash it or rebase/recommit if you prefer ;)

No worries, I'll squash! Thanks for the PR. :)

@ritchie46 ritchie46 merged commit e4bd365 into pola-rs:master Dec 6, 2021
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