feat: add json serde for table metadata #75
Conversation
wgtmac
force-pushed
the
table-meta-serde
branch
2 times, most recently
from
6acbdc6 to
2d55ea2
Compare
| } | ||
| }; | ||
|
|
||
| /// \brief Returns a string representation of a BlobMetadata |
There was a problem hiding this comment.
The method name may need to use BlobMetadataToString to avoid conflicts with the ToString method in member methods ？
There was a problem hiding this comment.
I have removed ToString member function because inheritance prohibits aggregate initialization.
There was a problem hiding this comment.
SGTM. We could extend the std::format specialization to look for an overload of iceberg::ToString or something (or try argument-dependent lookup? 🤔)
There was a problem hiding this comment.
Yes, I think we can specialize std::formatter in the iceberg/util/formatter.h for classes that have iceberg::ToString(const T&) at hands.
| } | ||
| }; | ||
|
|
||
| /// \brief Returns a string representation of a BlobMetadata |
There was a problem hiding this comment.
SGTM. We could extend the std::format specialization to look for an overload of iceberg::ToString or something (or try argument-dependent lookup? 🤔)
wgtmac
force-pushed
the
table-meta-serde
branch
from
2d55ea2 to
1f4447a
Compare
wgtmac
force-pushed
the
table-meta-serde
branch
from
1f4447a to
45514cf
Compare
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
@wgtmac Thanks for raising this PR, this looks great already! How do you feel about adding some tests?
We have some example JSONs here: https://github.com/apache/iceberg/tree/main/core/src/test/resources
6 participants
No description provided.