trinodb / trino-python-client

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

Add support for pyformat paramstyle #192

Closed quickcoffee closed 1 month ago

quickcoffee commented 2 years ago

Currently the trino python client does not support the pyformat parameter style.
Support for this paramstyle would allow to specify the parameters via a dict and use parameters in more complex queries where multiple parameters are defined, while maintaining a high code readability.
See the PyHive implementation: https://github.com/dropbox/PyHive/commit/d6e7140d366cc37d01af0d8308e7739d090d73ef

mdesmet commented 2 years ago

Currently the parameters are interpolated using the PREPARE and EXECUTE statements. The parameters are sent to the server using HTTP headers.

Using pyformat would mean doing the interpolation on the clientside and would result in just one in the HTTP body.

The relevant code is here:

https://github.com/trinodb/trino-python-client/blob/951ad82b7a7571c99dddf4e35f66fb403f052c11/trino/dbapi.py#L452-L480

@quickcoffee: Are you willing to do a PR to implement this?

hashhar commented 2 years ago

Client side interpolation doesn't seem a good idea because then the client will end up playing catch up with what the server supports. While the end goal is useful (more readable Python code with complex queries) the solution has a cost.

I'd argue that people should use something higher level like SQLAlchemy if they need more choice.

quickcoffee commented 2 years ago

@mdesmet I could create a PR, if you are willing to use client side interpolation, which we should figure out first.

Regarding the comment from @hashhar: I understand that there is a cost. Maybe the python client is then not the right place to support pyformat, but rather have a separate SQLAlchemy dialect package?
As I understand you need to provide SQLAlchemy with a dialect, which is currently registered by the trino python client. As the python client does not support pyformat, another backend needs to be used (eg. PyHive), but I would rather use an official trino backend than an unsupported package

hashhar commented 1 month ago

We don't intend to provide multiple options for paramstyle in the near future at-least. Closing for now.