-
Notifications
You must be signed in to change notification settings - Fork 230
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
Remove the mixedType config for JSON as it has no downsides any longer #10716
Conversation
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
build |
docs/compatibility.md
Outdated
Mixed types can have some problem. If an item being read could have some lines that are arrays | ||
and others that are structs/dictionaries it is possible for an error will be thrown. |
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.
Mixed types can have some problem. If an item being read could have some lines that are arrays | |
and others that are structs/dictionaries it is possible for an error will be thrown. | |
Mixed types can have some problems. If an item being read could have some lines that are arrays | |
and others that are structs/dictionaries it is possible an error will be thrown. |
@@ -268,6 +268,7 @@ object GpuJsonReadCommon { | |||
|
|||
private def throwMismatchException(cv: ColumnView, | |||
dt: DataType): (Option[ColumnView], Seq[AutoCloseable]) = { | |||
ai.rapids.cudf.TableDebug.get.debug(s"JSON MISMATCH $dt", cv) |
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.
Intentional or debug code left in?
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.
oops
build |
mixedTypesAsStrings was added in when we relied on CUDF to do schema discovery when parsing JSON. That is no longer the case and there are no real downsides to it any longer. So remove the config because it is not really needed.
This patch sets it to always be on, as that is the direction we want to go in.