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

get_prepared_statement_values fails if HEADER_ADDED_PREPARE is present in header but empty #262

Closed
1 task done
derek-pyne opened this issue Oct 12, 2022 · 3 comments
Closed
1 task done

Comments

@derek-pyne
Copy link
Contributor

Expected behavior

Moving from 0.316 to 0.317 causes Trino queries to (run with pandas read_sql) to fail inside get_prepared_statement_values (

def get_prepared_statement_values(headers, header):
) with a Value Error (ValueError: not enough values to unpack (expected 2, got 1))

This seems to be caused by this commit which checks for the HEADER_ADDED_PREPARE and parses the data in it.

However, in our case, our Trino setup is always returning this header ('X-Trino-Added-Prepare'), just with an empty string if there is no data in it. get_prepared_statement_values then fails trying to parse the data in an empty string.

One fix, would just to not only check that this header is present (which is what is currently done), but also check that there is a non-empty value in it.

Note: I have very little understanding of any of the guts here and only a surface level understanding of a Trino setup. If the feedback is 'Your Trino setup should not be returning headers with empty values' then I can get some more information about why thats the case on my side :)

Actual behavior

Value Error if X-Trino-Added-Prepare is present but empty

Steps To Reproduce

Add X-Trino-Added-Prepare to response with empty string. Unsure if this is expected behaviour (can get more information from my side if this is weird)

Log output

No response

Operating System

macOS 12.6

Trino Python client version

0.317

Trino Server version

388.007

Python version

3.9.9

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@hashhar
Copy link
Member

hashhar commented Oct 13, 2022

Trino Server version

388.007

Doesn't look like a real Trino version. Are you running a fork with some modifications?
Asking because the header being passed with empty values is not expected AFAIK.

@hashhar
Copy link
Member

hashhar commented Oct 13, 2022

BTW even if it's not expected we should still make the code more defensive either way (for example if there's some proxy sitting between Trino and the client which mangles the headers) - so please send a PR.

I do plan to improve the general mechanism by which we handle request and response headers so it'll likely get fixed then as well but I can't commit to "when" I'll be done with it.

@hashhar
Copy link
Member

hashhar commented Oct 31, 2022

Fixed in #263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants