You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "philippeVerney (via GitHub)" <gi...@apache.org> on 2025/11/13 08:20:06 UTC
[PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
philippeVerney opened a new pull request, #3545:
URL: https://github.com/apache/avro/pull/3545
## What is the purpose of the change
This pull request allows (C++ 17) std::optional structure to be encoded and decoded.
## Verifying this change
This change added tests and can be verified as follows:
- Added test in test/SpecificTests.cc
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? Same documentation than for other specific specific encoder/decoder
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "philippeVerney (via GitHub)" <gi...@apache.org>.
philippeVerney commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522362929
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
+ if (b) {
+ e.encodeUnionIndex(1);
+ avro::encode(e, b.value());
+ } else {
+ e.encodeUnionIndex(0);
+ e.encodeNull();
+ }
+ }
+
+ /**
+ * Decodes into a given value.
+ */
+ static void decode(Decoder &d, std::optional<T> &s) {
+ size_t n = d.decodeUnionIndex();
+ if (n >= 2) { throw avro::Exception("Union index too big"); }
+ switch (n) {
+ case 0: {
+ d.decodeNull();
+ s = std::nullopt;
+ } break;
+ case 1: {
+ T t;
+ avro::decode(d, t);
+ s.emplace(t);
Review Comment:
100% agree. I change that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "philippeVerney (via GitHub)" <gi...@apache.org>.
philippeVerney commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522700102
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
Review Comment:
Got it now, thanks for the explanation.
I don't see anything else to improve about your remark now that I added the comment in the documentation of the code_traits.
However, I now perfectly share your concern.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "philippeVerney (via GitHub)" <gi...@apache.org>.
philippeVerney commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522496685
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
Review Comment:
I add this remark in a comment not to lost it. Thanks.
Could you give me another schema example of an AVRO optional please.
I have some "optional" as `["null", U, V, T, ...]` but I map them to `std::variant` for now (other PR to come maybe)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522606982
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
Review Comment:
> AVRO optional
There is no such thing as `Avro optional`. The type is `Union`. It can have from 2 to N variants.
"null" is not a mandatory variant. It could be at any index in the variants, but usually it is the first one. Without a Schema it is a wild guess.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "thiru-mg (via GitHub)" <gi...@apache.org>.
thiru-mg commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2591450372
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
Review Comment:
I like the idea of standard encoding for `optional<T>`. Users are expected to use it only if the schema is `[null, T]`. We can even make the code generator use `optional<T>` whenever the schema is `[null, T]`. This will break compatibility, of course. So there should be a switch for the generator to make this optional (pun intended).
Since this PR does not break compatibility and makes its intent clear, I think, we should merge it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "philippeVerney (via GitHub)" <gi...@apache.org>.
philippeVerney commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522513150
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
+ if (b) {
+ e.encodeUnionIndex(1);
+ avro::encode(e, b.value());
+ } else {
+ e.encodeUnionIndex(0);
+ e.encodeNull();
+ }
+ }
+
+ /**
+ * Decodes into a given value.
+ */
+ static void decode(Decoder &d, std::optional<T> &s) {
+ size_t n = d.decodeUnionIndex();
+ if (n >= 2) { throw avro::Exception("Union index too big"); }
+ switch (n) {
+ case 0: {
+ d.decodeNull();
+ s = std::nullopt;
+ } break;
+ case 1: {
+ T t;
+ avro::decode(d, t);
+ s.emplace(t);
Review Comment:
I actually even preferred in place construction. Hoping it is fine with you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522416884
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
Review Comment:
This makes an assumption that the schema is `["null", T]`.
This is true most of the time but it is not mandatory. Without having the schema in scope I see no way how to improve it.
##########
lang/c++/include/avro/Specific.hh:
##########
Review Comment:
```suggestion
#include "Encoder.hh"
#include "Exception.hh"
```
for the new usage at https://github.com/apache/avro/pull/3545/files#diff-6a9c42629e99021e3a2ea7e61e2c364940c9e04228a8b382ea47032f04d44c74R192
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
+ if (b) {
+ e.encodeUnionIndex(1);
+ avro::encode(e, b.value());
+ } else {
+ e.encodeUnionIndex(0);
+ e.encodeNull();
+ }
+ }
+
+ /**
+ * Decodes into a given value.
+ */
+ static void decode(Decoder &d, std::optional<T> &s) {
+ size_t n = d.decodeUnionIndex();
+ if (n >= 2) { throw avro::Exception("Union index too big"); }
Review Comment:
```suggestion
if (n >= 2) { throw avro::Exception("Union index too big for optional (expected 0 or 1, got " + std::to_string(n) + ")"); }
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4200: [C++] Encode and decode std::optional [avro]
Posted by "KalleOlaviNiemitalo (via GitHub)" <gi...@apache.org>.
KalleOlaviNiemitalo commented on code in PR #3545:
URL: https://github.com/apache/avro/pull/3545#discussion_r2522356433
##########
lang/c++/include/avro/Specific.hh:
##########
@@ -165,6 +166,44 @@ struct codec_traits<double> {
}
};
+/**
+* codec_traits for Avro optional.
+*/
+template<typename T>
+struct codec_traits<std::optional<T>> {
+ /**
+ * Encodes a given value.
+ */
+ static void encode(Encoder &e, const std::optional<T> &b) {
+ if (b) {
+ e.encodeUnionIndex(1);
+ avro::encode(e, b.value());
+ } else {
+ e.encodeUnionIndex(0);
+ e.encodeNull();
+ }
+ }
+
+ /**
+ * Decodes into a given value.
+ */
+ static void decode(Decoder &d, std::optional<T> &s) {
+ size_t n = d.decodeUnionIndex();
+ if (n >= 2) { throw avro::Exception("Union index too big"); }
+ switch (n) {
+ case 0: {
+ d.decodeNull();
+ s = std::nullopt;
+ } break;
+ case 1: {
+ T t;
+ avro::decode(d, t);
+ s.emplace(t);
Review Comment:
`std::move(t)` here could avoid a copy. [Compiler Explorer](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:1,endLineNumber:13,positionColumn:1,positionLineNumber:13,selectionStartColumn:1,selectionStartLineNumber:13,startColumn:1,startLineNumber:13),source:'%23include+%3Coptional%3E%0A%23include+%3Cstring%3E%0A%0Avoid+setopt1(std::optional%3Cstd::string%3E+%26s)+%7B%0A++++std::string+t+%3D+%22text+1%22%3B%0A++++s.emplace(t)%3B%0A%7D%0A%0Avoid+setopt2(std::optional%3Cstd::string%3E+%26s)+%7B%0A++++std::string+t+%3D+%22text+2%22%3B%0A++++s.emplace(std::move(t))%3B%0A%7D%0A'),l:'5',n:'0',o:'C%2B%2B+source+%231',t:'0')),k:32.74777136864185,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:g152,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fon
tUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'-O2',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+gcc+15.2+(Editor+%231)',t:'0')),k:67.25222863135816,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org