-
Notifications
You must be signed in to change notification settings - Fork 188
Support VARBINARY
query parameter
#299
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
Conversation
02cd3f5
to
19c51a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good % one comment
.add_field(sql="X'65683F'", python='ZWg/') \ | ||
.add_field(sql="X''", python='') \ | ||
.add_field(sql="CAST('' AS VARBINARY)", python='') \ | ||
.add_field(sql="from_utf8(CAST('😂😂😂😂😂😂' AS VARBINARY))", python='😂😂😂😂😂😂') \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you removed it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because from_utf8
converts into VARCHAR
so it is not relevant here.
@hashhar: PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % additional test
def test_binary_query_param(trino_connection): | ||
cur = trino_connection.cursor(experimental_python_types=True) | ||
|
||
cur.execute("SELECT ?", params=(bytearray("a", "utf-8"),)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add a case with non-utf-8 bytes (to make sure implementation isn't doing the stupid thing of converting to str
and then decoding/encoding to byte-array).
19c51a1
to
f1904f7
Compare
Description
Non-technical explanation
Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: