Skip to content

Support for Presto decimals #430

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 7, 2022
Merged

Conversation

serenajiang
Copy link
Contributor

@serenajiang serenajiang commented Mar 7, 2022

Currently, Presto decimals are returned as strings. This PR processes the data to return decimals as Decimals, similar to the hive cursor.

See issue: #290

This PR is similar to a number of previous PRs, but as those seem to have been dropped, I figured I'd try again 🙏

@serenajiang
Copy link
Contributor Author

@bkyryliuk @betodealmeida ?

Copy link
Contributor

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

This looks good to me.

pyhive/presto.py Outdated
for i, col in enumerate(self.description):
if col[1] == 'varbinary':
col_type = col[1].split("(")[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Are column types always returned in lowercase? Might be safer to add a .lower() here.

@bkyryliuk bkyryliuk merged commit 3547bd6 into dropbox:master Mar 7, 2022
hsheth2 pushed a commit to hsheth2/PyHive that referenced this pull request Mar 19, 2022
* Support for Presto decimals

* lower
betodealmeida added a commit to preset-io/PyHive that referenced this pull request Aug 8, 2024
* feat: add HTTP and HTTPS to hive (dropbox#385)

* feat: add https protocol

* support HTTP

* fix: make hive https py2 compat (dropbox#389)

* fix: make hive https py2 compat

* fix lint

* Update README.rst (dropbox#423)

* chore: rename Trino entry point (dropbox#428)

* Support for Presto decimals (dropbox#430)

* Support for Presto decimals

* lower

* Use str type for driver and name in HiveDialect (dropbox#450)

PyHive's HiveDialect usage of bytes for the name and driver fields is not the norm is causing issues upstream: apache/superset#22316
Even other dialects within PyHive use strings. SQLAlchemy does not strictly require a string, but all the stock dialects return a string, so I figure it is heavily implied.

I think the risk of breaking something upstream with this change is low (but it is there ofc). I figure in most cases we just make someone's `str(dialect.driver)` expression redundant.

Examples for some of the other stock sqlalchemy dialects (name and driver fields using str):
https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/dialects/sqlite/pysqlite.py#L501
https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/dialects/sqlite/base.py#L1891
https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/dialects/mysql/base.py#L2383
https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/dialects/mysql/mysqldb.py#L113
https://github.com/zzzeek/sqlalchemy/blob/main/lib/sqlalchemy/dialects/mysql/pymysql.py#L59

* Correcting Iterable import for python 3.10 (dropbox#451)

* changing drivers to support hive, presto and trino with sqlalchemy>=2.0 (dropbox#448)

* Revert "changing drivers to support hive, presto and trino with sqlalchemy>=2.0 (dropbox#448)" (dropbox#452)

This reverts commit b0206d3.

* Update __init__.py (dropbox#453)

dropbox@1c1da8b

dropbox@1f99552

* use pure-sasl with python 3.11 (dropbox#454)

* minimal changes for sqlalchemy 2.0 support (dropbox#457)

* update readme to reflect recent changes (dropbox#459)

* Update README.rst (dropbox#475)

* Update README.rst (dropbox#476)

* feat: JWT support

* Add CI to build package

---------

Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com>
Co-authored-by: Bogdan <b.kyryliuk@gmail.com>
Co-authored-by: serenajiang <serena.jiang@airbnb.com>
Co-authored-by: Usiel Riedl <usiel.riedl@gmail.com>
Co-authored-by: Multazim Deshmukh <57723564+mdeshmu@users.noreply.github.com>
Co-authored-by: nicholas-miles <nicholas.miles6@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants