-
Notifications
You must be signed in to change notification settings - Fork 31
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
SQLAlchemy: Use JSON type adapter for implementing CrateDB's OBJECT
#561
Conversation
# Designated name to refer to. `Object` is too ambiguous. | ||
ObjectType = MutableDict.as_mutable(ObjectTypeImpl) |
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.
SQLAlchemy also offers a MutableDict
implementation. It looks like using that would resolve some other specializations of the CrateDB dialect, which are a bit anomalous, as we discovered on behalf of the discussion at sqlalchemy/sqlalchemy#5915.
However, it did not work out of the box. SQLAlchemy's MutableDict
, or associated code paths, did not seem to support nested object access well. Maybe indeed this is one of the essential specializations the CrateDB dialect has to offer.
This topic will need to be re-visited. /cc GH-425
class ObjectTypeImpl(sqltypes.UserDefinedType, sqltypes.JSON): | ||
|
||
__visit_name__ = "OBJECT" | ||
|
||
cache_ok = False |
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.
We still need to use cache_ok = False
, otherwise things go south on indexed dictionary access. So, it does not improve the situation wrt. GH-559. I will ask for advise on the upstream issue tracker about this detail.
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.
Thank you! Looks good, but please wait for the rest of the reviewers.
CHANGES.txt
Outdated
@@ -15,6 +15,8 @@ Unreleased | |||
|
|||
- SQLAlchemy: Fix SQL statement caching for CrateDB's ``OBJECT`` type. | |||
|
|||
- SQLAlchemy: Refactor ``OBJECT`` type to use SQLAlchemy's JSON type machinery. |
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'd personally prefer infrastructure
or functionality
over machinery
.
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.
Thanks. Updated the changelog item.
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.
👍
cbb1053
to
bb192b5
Compare
SQLAlchemy's `sqltypes.JSON` provides a facade for vendor-specific JSON types. Since it supports JSON SQL operations, it only works on backends that have an actual JSON type, which are currently PostgreSQL, MySQL, SQLite, and Microsoft SQL Server. This patch starts leveraging the same infrastructure, thus bringing corresponding interfaces to the CrateDB dialect. The major difference is that it will not actually do any JSON marshalling, but propagate corresponding data structures 1:1, because within CrateDB's SQL, `OBJECT`s do not need to be serialized into JSON strings before transfer.
def get_col_spec(self): | ||
return 'OBJECT' | ||
# Backward-compatibility aliases. | ||
_deprecated_Craty = ObjectType |
Check notice
Code scanning / CodeQL
Unused global variable
return 'OBJECT' | ||
# Backward-compatibility aliases. | ||
_deprecated_Craty = ObjectType | ||
_deprecated_Object = ObjectType |
Check notice
Code scanning / CodeQL
Unused global variable
About
Following up on crate/sqlalchemy-cratedb#93, by using the generalized parts of SQLAlchemy's infrastructure for implementing JSON types for a few databases, the CrateDB dialect will be able to leverage the same interfaces, like data casters, thus improving compatibility.
Example
Details
SQLAlchemy's
sqltypes.JSON
provides a facade for vendor-specific JSON types. Since it supports JSON SQL operations, it only works on backends that have an actual JSON type, which are currently PostgreSQL, MySQL, SQLite, and Microsoft SQL Server.This patch starts leveraging the same infrastructure, thus bringing corresponding interfaces to the CrateDB dialect. The major difference is that it will not actually do any JSON marshalling, but propagate corresponding data structures 1:1, because within CrateDB's SQL,
OBJECT
s do not need to be serialized into JSON strings before transfer.Resources