implement casting between bytes and json #2482
Conversation
| r""" SELECT <json>b'foo'; """, | ||
| ['Zm9v'], | ||
| ['"Zm9v"'], | ||
| ) |
There was a problem hiding this comment.
Please also add tests when there's a bytes type is in a shape, e.g.
SELECT <json>User {
name,
some_bytes := b'aaa'
}
and the same query without <json> but when compiled in JSON output mode.
There was a problem hiding this comment.
Casting shapes with <json> and query_json() don't work with the current changes. I've been tinkering with the compiler for a while trying to find the relative parts. If anyone has any pointers let me know.
There was a problem hiding this comment.
It's an interesting issue, actually. See compile_TypeCast in edgeql/compiler/expr.py. Specifically the trailing else: branch sets up a context to compile the casted expression. What we are missing here is another context flag that would tell the shape processor in viewgen.py that it is inside a JSON cast, and that every shape element value should be cast to <json>. We do this type of recursion for collections, but not shapes, evidently. This isn't an issue for most types, since PostgreSQL's cast is the same as ours, but bytes exposed the bug here.
|
This query I haven't started debugging this yet, but it seems like there are two inconsistencies with this error
|
|
It's not about castability, you cannot change the type of an existing link or property in a shape. This is problem with this approach. I think the solution would be to not to redefine the original properties, but inject magic computables instead, e.g |
I have no idea where to start with this 😕 |
Why does it only complain about the |
Because we do implicit rewrites on introspection schema objects to support field defaults, and evidently that rewrite interacts badly with this. See |
|
I think it is postponed. Maybe it should just be closed for now? @elprans |
|
I marked it as draft. We'll come back to this later. |
|
@msullivan Sully, please take a look at this, would be nice to fix this in 1.0. Ask Elvis about the current shortcomings of this pr. |
fixes #2459 Casting between bytes and json is enabled by encoding bytes to base64 encoded json strings.
… casts This requires some various plumbing changes to make this work in nested cases. Some logic in the IR side cast generation needed to be updated to not include an inaccurate intermediate type when casting a bytes array literal to json. I think that much of the logic on the IR side for compiling json casts from arrays and tuples can be excised (though probably with the addition of a call to eta_expand), but I'm leaving that to a follow-up.
|
I took a different approach than the proposed one. In addition to making bytes render as base64 when cast to json, we need to have them render that way in json output mode too. Once that is fixed, the whole object casting problem goes away, since casting objects to json simply relies on first serializing the object in a json output mode. |
| irtyputils.is_bytes(styperef) | ||
| and not expr.ser_safe | ||
| ): | ||
| cast_name = s_casts.get_cast_fullname_from_names( |
There was a problem hiding this comment.
I'd add a TODO: note here to remind us to find a way to generalize this, since bytes wouldn't be the last scalar with a custom JSON cast.
edb/ir/typeutils.py
Outdated
| @@ -104,6 +105,16 @@ def is_abstract(typeref: irast.TypeRef) -> bool: | |||
| return typeref.is_abstract | |||
|
|
|||
|
|
|||
| def is_json(typeref: irast.TypeRef) -> bool: | |||
| """Return True if *typeref* describes the json type.""" | |||
| return typeref.id == s_obj.get_known_type_id('std::json') | |||
fixes #2459
Casting between bytes and json is enabled by encoding bytes to base64 encoded json strings.