-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Add fix for pyodbc+mssql #6621
Add fix for pyodbc+mssql #6621
Conversation
chinhngt
commented
Jan 8, 2019
- Handle Row object returned by pyodbc/msodbc
- Add tests
@chinhngt you missed to write what is the problem you are fixing. I suppose one between the tuple or the iterator breaks something else later in the chain. Or maybe both? I don't know the code but maybe it's the caller that must be fixed as an iterator may be a good idea to save memory. |
@xrmx Currently when using Superset with pyodbc, the data pyodbc returns is in the form of [pyodbc.Row, pyodbc.Row, ...] instead of [(tuple), (tuple), ...] so SupersetDataFrame throws exception on wrong shape (it got 1 column instead of N columns). This change converts pyodbc.Row object to regular list before passing to SupersetDataFrame. I think BigQuery (BQEngineSpec in db_engine_specs.py) is doing similar thing :) |
@chinhngt thanks for the explanation. So you have to prepare data that pandas can digest for a DataFrame. |
@xrmx I don't see sqla uri is available within db_engine_specs. Any hint? |
@chinhngt no clue, anyway it if works for you it's fine for me. |
@xrmx Yes, it works for both Azure SQL and Azure Datawarehouse for me. |
@mistercrunch as this PR was not rebased against master the license check did not kick in on the PR. It does have a new file and therefore master now fails building. To solve this have a PR with a license header on an existing file ;-) |