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.

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

Add support for pyformat paramstyle #192

Open
quickcoffee opened this issue Jul 4, 2022 · 3 comments
Open

Add support for pyformat paramstyle #192

quickcoffee opened this issue Jul 4, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@quickcoffee
Copy link

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: dropbox/PyHive@d6e7140

@ebyhr ebyhr added the enhancement New feature or request label Jul 4, 2022
@mdesmet
Copy link
Contributor

mdesmet commented Jul 15, 2022

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:

if params:
assert isinstance(params, (list, tuple)), (
'params must be a list or tuple containing the query '
'parameter values'
)
statement_name = self._generate_unique_statement_name()
# Send prepare statement
added_prepare_header = self._prepare_statement(
operation, statement_name
)
try:
# Send execute statement and assign the return value to `results`
# as it will be returned by the function
self._query = self._get_added_prepare_statement_trino_query(
statement_name, params
)
result = self._query.execute(
additional_http_headers={
constants.HEADER_PREPARED_STATEMENT: added_prepare_header
}
)
finally:
# Send deallocate statement
# At this point the query can be deallocated since it has already
# been executed
self._deallocate_prepare_statement(added_prepare_header, statement_name)

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

@hashhar
Copy link
Member

hashhar commented Jul 18, 2022

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
Copy link
Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

4 participants