You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "wgtmac (via GitHub)" <gi...@apache.org> on 2025/04/25 15:09:06 UTC

[PR] AVRO-4132: [C++] Add ZSTD codec [avro]

wgtmac opened a new pull request, #3364:
URL: https://github.com/apache/avro/pull/3364

   ## What is the purpose of the change
   
   Add ZSTD codec to the C++ implementation
   
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
   - *Added test that validates the codec works perfectly*
   
   
   ## Documentation
   
   - Does this pull request introduce a new feature? no
   - If yes, how is the feature documented? not applicable
   


-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #3364:
URL: https://github.com/apache/avro/pull/3364#issuecomment-2830823469

   @thiru-mg @martin-g Could you help review this?
   
   cc @Fokko I think iceberg-cpp requires this for manifest file


-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #3364:
URL: https://github.com/apache/avro/pull/3364#issuecomment-2833270849

   Thank you all!


-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #3364:
URL: https://github.com/apache/avro/pull/3364#issuecomment-2833242493

   Thank you, @wgtmac !


-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #3364:
URL: https://github.com/apache/avro/pull/3364#discussion_r2061907741


##########
lang/c++/include/avro/DataFile.hh:
##########
@@ -39,7 +39,11 @@ enum Codec {
     DEFLATE_CODEC,
 
 #ifdef SNAPPY_CODEC_AVAILABLE
-    SNAPPY_CODEC
+    SNAPPY_CODEC,
+#endif
+
+#ifdef ZSTD_CODEC_AVAILABLE
+    ZSTD_CODEC

Review Comment:
   Agreed. Added explicit values.



-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #3364:
URL: https://github.com/apache/avro/pull/3364


-- 
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-4132: [C++] Add ZSTD codec [avro]

Posted by "Gerrit0 (via GitHub)" <gi...@apache.org>.
Gerrit0 commented on code in PR #3364:
URL: https://github.com/apache/avro/pull/3364#discussion_r2061668392


##########
lang/c++/include/avro/DataFile.hh:
##########
@@ -39,7 +39,11 @@ enum Codec {
     DEFLATE_CODEC,
 
 #ifdef SNAPPY_CODEC_AVAILABLE
-    SNAPPY_CODEC
+    SNAPPY_CODEC,
+#endif
+
+#ifdef ZSTD_CODEC_AVAILABLE
+    ZSTD_CODEC

Review Comment:
   It seems to me that these enum values should really have an explicit value to avoid potential confusion where "2" could mean either depending on what preprocessor definitions are set.



-- 
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