-
Notifications
You must be signed in to change notification settings - Fork 2
Types: Add support for BINARY columns #12
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import base64 | ||
|
|
||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| class LargeBinary(sa.String): | ||
| """A type for large binary byte data. | ||
|
|
||
| The :class:`.LargeBinary` type corresponds to a large and/or unlengthed | ||
| binary type for the target platform, such as BLOB on MySQL and BYTEA for | ||
| PostgreSQL. It also handles the necessary conversions for the DBAPI. | ||
|
|
||
| """ | ||
|
|
||
| __visit_name__ = "large_binary" | ||
|
|
||
| def bind_processor(self, dialect): | ||
| if dialect.dbapi is None: | ||
| return None | ||
|
|
||
| # TODO: DBAPIBinary = dialect.dbapi.Binary | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can it be removed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... or activated, to be more standards-compliant. Let's investigate within another iteration before getting things wrong. Thanks! |
||
|
|
||
| def process(value): | ||
| if value is not None: | ||
| # TODO: return DBAPIBinary(value) | ||
| return base64.b64encode(value).decode() | ||
| else: | ||
| return None | ||
|
|
||
| return process | ||
|
Comment on lines
+17
to
+30
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self, or others who want to pick this up: Review that detail about returning a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amotl Did you review it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. I just staged the patch directly from its origin, where the code is in use and works well. However, it can still be wrong from a generic perspective. Let's toggle this patch back into draft mode: Better safe than sorry, thanks! The other commit 465b55b has been removed here and diverged to a separate patch. |
||
|
|
||
| # Python 3 has native bytes() type | ||
| # both sqlite3 and pg8000 seem to return it, | ||
| # psycopg2 as of 2.5 returns 'memoryview' | ||
| def result_processor(self, dialect, coltype): | ||
| if dialect.returns_native_bytes: | ||
| return None | ||
|
|
||
| def process(value): | ||
| if value is not None: | ||
| return base64.b64decode(value) | ||
| return value | ||
|
|
||
| return process | ||
|
Comment on lines
+32
to
+44
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self, or others who want to pick this up: Review that detail about
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amotl Did you review 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.
I suggest that we also mention CrateDB'
BLOBtype hereThere 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.
CrateDB does not provide a BLOB type, does it?