Skip to content

sqlalchemy: support TINYINT type - #126

Closed
susodapop wants to merge 6 commits into
mainfrom
issue-125
Closed

sqlalchemy: support TINYINT type#126
susodapop wants to merge 6 commits into
mainfrom
issue-125

Conversation

@susodapop

@susodapopsusodapop commented May 16, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Adds support for the Databricks TINYINT type.

To use the TINYINT users can:

fromdatabricks.sqlalchemy.dialectimportTINYINTSampleTable=Table(
"some_table_name",
metadata_obj,
Column("small_number",TINYINT),
)

Related Tickets & Documents

Closes#123

Related to #108 since we'll need to implement BINARY in a similar fashion.

Jesse Whitehouse added 4 commits May 15, 2023 14:55
Jesse Whitehouse
TINYINT is specific to Databricks so must be implemented as a vendor-…
918a7de
…specific type per this link: https://docs.sqlalchemy.org/en/13/core/type_basics.html#vendor-specific-types
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Jesse Whitehouse
Define the TINYINT type
96774b1
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Jesse Whitehouse
First implementation that passes the integration test.
b35f0c9
There seems to be two mechanisms where I can hook into the type compiler
In this one, I explicitly set a dialect-specific compilation overwrite.
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Jesse Whitehouse
This is the second way to make the integration test pass using the visit
71321aa
mechanism. This is simpler than the approach in the commit I'm reverting.
This partially reverts commit b35f0c9.
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Jesse Whitehouse
Black the source code
7591028
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop

susodapop commented May 16, 2023

Copy link
Copy Markdown
ContributorAuthor

For future reference, I identified a couple ways to register this type with SQLAlchemy's type compiler. I committed both on this branch but reverted the more complicated solution and kept the simplest one (using the visit_ mechanism).

Jesse Whitehouse
Add tiny_int to the INSERT test.
2896665
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop

susodapop commented May 24, 2023

Copy link
Copy Markdown
ContributorAuthor

In this PR, I also want to update examples/sqlalchemy.py tos how how to use the TINYINT type. When I tried this I'm seeing this exception ModuleNotFoundError: No module named 'sqlalchemy.orm'; 'sqlalchemy' is not a package which seems like an environment problem. Going to revisit this after I open a PR for reusing TCP connections.

@susodapop
susodapop marked this pull request as draft May 24, 2023 19:06
@gdippolito

gdippolito commented Jul 21, 2023

Copy link
Copy Markdown

Hello! I think we are facing an issue that would be fixed by this PR. Would it be possible to merge it?

@gdippolito

gdippolito commented Sep 20, 2023

Copy link
Copy Markdown

Ping @susodapop in case you didn't see my previous comment 😄

Do you have any plans to add support for TINYINT?

@susodapop

susodapop commented Oct 28, 2023

Copy link
Copy Markdown
ContributorAuthor

Hey @gdippolito I'm dropping this PR in favour of #265 which will merge and be released next week. Thanks for your patience in this regard. We've completely rewritten our dialect and incorporated this addition into that effort.

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

Reviewers

@arikfrarikfrAwaiting requested review from arikfr
@yunbodeng-dbyunbodeng-dbAwaiting requested review from yunbodeng-db
@andrefurlan-dbandrefurlan-dbAwaiting requested review from andrefurlan-db

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Delta has TINYINT now which needs to be added in?

2 participants

@susodapop@gdippolito