GH-49081: [C++][Parquet] Correct variant's extension name#49082
GH-49081: [C++][Parquet] Correct variant's extension name#49082HuaHuaY wants to merge 3 commits intoapache:mainfrom
Conversation
|
|
github-actions
bot
added
Component: Parquet
Component: C++
awaiting review
| @@ -50,7 +50,7 @@ class PARQUET_EXPORT VariantExtensionType : public ::arrow::ExtensionType { | |||
| public: | |||
| explicit VariantExtensionType(const std::shared_ptr<::arrow::DataType>& storage_type); | |||
|
|
|||
| std::string extension_name() const override { return "parquet.variant"; } | |||
| std::string extension_name() const override { return "arrow.parquet.variant"; } | |||
There was a problem hiding this comment.
This change looks good.
Do you think it is worth adding a kVariantExtensionName in this file and move this file to cpp/src/arrow/extension/parquet_variant.h?
WDYT? @pitrou
There was a problem hiding this comment.
JsonExtensionType also uses hardcoded extension name. If we want to define string literals, I suggest to move other's extension types' names into a same file.
There was a problem hiding this comment.
Do we need to move variant_internal.h to cpp/src/arrow/extension/ like other extension types?
There was a problem hiding this comment.
Yes, that's exactly what I proposed above.
There was a problem hiding this comment.
I have moved variant_internal.cc to cpp/src/arrow/extension/parquet_variant.cc.
github-actions
bot
added
awaiting committer review
HuaHuaY
force-pushed
the
fix_issue_49081
branch
from
7b7dc64 to
1794f6f
Compare
2 participants
Rationale for this change
Correct variant extension according to arrow's specification.
What changes are included in this PR?
Modified variant's hardcoded extension name.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.