search

trinodb / trino-python-client

Python client for Trino
Apache License 2.0
328 stars 163 forks source link

Add ability to specify catalog as a SQLALchemy table argument #186

Closed laserkaplan closed 2 years ago

laserkaplan commented 2 years ago

This change allows for specifying a trino_catalog table argument to SQLALchemy Table objects, which is then checked when compiling statements and prepended to the proper objects. This allows for writing queries that talk to multiple catalogs at the same time.

This change was largely implemented by @AlexandreOuellet in the PyHive repository, with some minor edits by @VinceDPM and myself. However, since that repository is no longer well-maintained, and since Trino seems to have better support currently than Presto on the whole, I have switched my own focus to using Trino, and thus wanted to get this functionality working here as well.

As this is the first time I've attempted to contribute here, please let me know if I am missing anything in this PR! It would be great if this could be merged in an efficient manner.

cla-bot[bot] commented 2 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot[bot] commented 2 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

laserkaplan commented 2 years ago

Could you submit CLA if you haven't yet sent it?

I sent my signed CLA form yesterday. Please let me know if it hasn't been received!

ebyhr commented 2 years ago

@cla-bot check

cla-bot[bot] commented 2 years ago

The cla-bot has been summoned, and re-checked this pull request!

laserkaplan commented 2 years ago

I have added an additional bit to this PR since it hadn't been approved/merged yet. The visitors worked when compiling SQL statements, but another method was needed to do the same functionality for DDL statements (like CREATE TABLE).

laserkaplan commented 2 years ago

Hi all, is there anything else blocking this PR from completion?

laserkaplan commented 2 years ago

Please squash the commits now (since it's one logical change).

Is "squash and merge" enabled on the repo for when this is ready to merge?

mdesmet commented 2 years ago

Is "squash and merge" enabled on the repo for when this is ready to merge?

AFAIK we don't do squash and merge, only rebase and merge.