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.

Sign up for GitHub

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

Jump to bottom

Fix fallback to NullType when dialect type not found #215

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix fallback to NullType when dialect type not found #215

wants to merge 1 commit into from

Conversation

Rustem
Copy link

@Rustem Rustem commented Feb 15, 2021 •
edited
Loading

GEOGRAPHY custom type is not supported yet. Decided to fix issue similarly how sqlalchemy/postgresql dialect fixes it using sqlalchemy generic type called NULLTYPE. For more information see here

Root cause: sqltypes.NULLTYPE is already an instance of sqltypes.NullType, however when type is not supported logic does not account for that and tries to initalize already initialized instance.

Related Issue: #194

@Rustem
Copy link
Author

Rustem commented Feb 15, 2021 •
edited
Loading

Ok tested this workaround on few tables that contain geography and without it, now it works!

@Rustem
Copy link
Author

Rustem commented Feb 17, 2021

Could you help to merge this bug fix asap please? As we temporarily patched the dialect on our end.

@Rustem Rustem changed the title fallback to NullType when dialect type not found Fix fallback to NullType when dialect type not found Feb 17, 2021
@Mafioso
Copy link

Mafioso commented Feb 18, 2021

We have same problem with dialect. Can you please merge this pull request. @sfc-gh-kwagner @sfc-gh-mkeller

emreakg
emreakg reviewed Jul 30, 2021
View reviewed changes
snowdialect.py
@@ -380,8 +380,7 @@ def _get_schema_columns(self, connection, schema, **kw):
(sqltypes.String, sqltypes.BINARY)):
col_type_kw['length'] = character_maximum_length

type_instance = col_type(**col_type_kw)

type_instance = col_type if isinstance(col_type, sqltypes.NullType) else col_type(**col_type_kw)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need the same change at line 453

@emreakg
Copy link

emreakg commented Jul 30, 2021

Any plans to merge this? This looks like a good hotfix to integrate. engine.dialect.get_columns() is unusable due to this if any table in schema contains a GEOGRAPHY column.@sfc-gh-kwagner @sfc-gh-mkeller

@nikhiltjoy-dd
Copy link

+1 GEOGRAPHY columns are causing issues

@hadia206
Copy link

+1 for merge request.
I am getting this error as well when trying to do df.to_sql if any table in schema contains a GEOGRAPHY column.

@njriasan
Copy link

+1 for merge request.
I am getting this error as well when trying to do df.to_sql if any table in schema contains a GEOGRAPHY column.

+1 on the merge request. I have verified that applying this change resolves my issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emreakg emreakg emreakg left review comments

@sfc-gh-kwagner sfc-gh-kwagner Awaiting requested review from sfc-gh-kwagner

@sfc-gh-mkeller sfc-gh-mkeller Awaiting requested review from sfc-gh-mkeller

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Rustem @Mafioso @emreakg @nikhiltjoy-dd @hadia206 @njriasan