Infer Array(Dynamic) instead of unnamed Tuple for arrays of values with different types in JSON#80859
Conversation
clickhouse-gh
bot
added
the
pr-backward-incompatible
There was a problem hiding this comment.
The doc is crystal clear, the breaking change after a setting to allow previous behavior, and the comments in the code are perfect. Very nice work 🚀
I left two minor nitpick comments. I don't think the performance comparison have anything to do with your changes. They're known to be unstable for AMD #78555
I'd change the changelog entry to say how to disable the new behavior more explicitly, though. Something along these lines:
Infer Array(Dynamic) instead of unnamed Tuple for arrays of values with different types in JSON. To use previous behaviour, set input_format_json_infer_array_of_dynamic_from_array_of_different_types=0
src/Formats/SchemaInferenceUtils.cpp
Outdated
| /// Only unnamed Tuple types can be inferred from arrays. | ||
| for (const auto & type : data_types) | ||
| { | ||
| if (const auto * tuple_type = typeid_cast<const DataTypeTuple *>(type.get()); tuple_type && tuple_type->haveExplicitNames()) |
There was a problem hiding this comment.
I know this is not related to your changes and it's nitpicking, but I think we should rename haveExplicitNames() and the underlying variable have_explicit_names to hasExplicitNames() and has_explicit_names, for consistency.
There was a problem hiding this comment.
Agree, but I don't want to change it in this PR. Feel free to create a separate PR for it
There was a problem hiding this comment.
Alright, done in #82194
I added you as a reviewer directly, since you already have the context for this
Co-authored-by: Pablo Marcos <[email protected]>
|
@Avogar everything is green and ready to be merged 🚀 |
5 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Infer
Array(Dynamic)instead of unnamedTuplefor arrays of values with different types in JSON. To use previous behaviour, disable settinginput_format_json_infer_array_of_dynamic_from_array_of_different_types.Closes #74937
Documentation entry for user-facing changes