-
Notifications
You must be signed in to change notification settings - Fork 242
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
Re-enable from_json
/ JsonToStructs
#9423
Conversation
Signed-off-by: Andy Grove <[email protected]>
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.
For me personally I would disable JSON reading and CSV reading for that matter by default, instead of compounding the problem.
from_json
/ JsonToStructs
from_json
/ JsonToStructs
I am seeing two errors with the current code:
|
@@ -3556,17 +3556,19 @@ object GpuOverrides extends Logging { | |||
expr[JsonToStructs]( | |||
"Returns a struct value with the given `jsonStr` and `schema`", | |||
ExprChecks.projectOnly( | |||
TypeSig.MAP.nested(TypeSig.STRING).withPsNote(TypeEnum.MAP, | |||
"MAP only supports keys and values that are of STRING type"), | |||
TypeSig.STRUCT.nested(TypeSig.all) + |
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 don't think we really support all under a Struct. We don't support maps under a struct and we don't support UDTs under a struct. Could we be more explicit about what we do and do not support here?
from_json
/ JsonToStructs
from_json
/ JsonToStructs
These issues are now resolved by recent bug fixes in cuDF |
build |
Depends on rapidsai/cudf#14309
This PR re-enables
from_json
/JsonToStructs
. It was previously disabled (see #8558) because it can cause incorrect results in some cases, but we have similar issues when reading directly from JSON files (see xfailed cases in #9304).Given that this expression does work correctly in many cases where the inputs are valid JSON, and that it is disabled by default, I would like to discuss adding this back in.