You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2025/12/16 15:56:10 UTC
[I] Serde incompatible with C++ TDigest (datasketches-java)
tisonkun opened a new issue, #701:
URL: https://github.com/apache/datasketches-java/issues/701
In Java's TDigestDouble, this field is always ignored as unused.
https://github.com/apache/datasketches-java/blob/37edd8c9898f8f073aaafad7097cb0949eb84c85/src/main/java/org/apache/datasketches/tdigest/TDigestDouble.java#L429
However, C++'s impl can use it for denote the size of buffer vector.
https://github.com/apache/datasketches-cpp/blob/7bb979d3ef8929e235bcd22d67579e1f695f6ecd/tdigest/include/tdigest_impl.hpp#L321-L326
Also the Java impl always assumes there is no buffer serialized. This can lead to serde incompatible for any C++ TDigest has buffered data serialized with `with_buffer`, like:
https://github.com/apache/datasketches-cpp/blob/7bb979d3ef8929e235bcd22d67579e1f695f6ecd/tdigest/test/tdigest_test.cpp#L222-L235
--
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@datasketches.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3667447019
@AlexanderSaydakov,
As @tisonkun points out, this leads to an immediate incompatibility between C++ and Java serialized TDigest sketches.
How do you suggest we fix this?
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3667537557
Maybe keep it compatible with the deserializing part? (As I did in https://github.com/apache/datasketches-rust/pull/23).
> I don't remember why did we decide to provide this option to avoid compression and serializing the buffer.
Yes, IMO always compress before serializing in this case helps.
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3672886198
Can we agree that the path for Rust is to compress before serializing, which means follow the Java version for this behavior.
If so, can we close?
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "AlexanderSaydakov (via GitHub)" <gi...@apache.org>.
AlexanderSaydakov commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3667519715
In the default mode they must be compatible.
I suggest fixing this by implementing the same approach in Java.
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3663015916
Thank you for pointing this out. We are looking into 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: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho closed issue #701: Serde incompatible with C++ TDigest
URL: https://github.com/apache/datasketches-java/issues/701
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3673054830
Here is the patch: #704
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3672998937
> Can we agree that the path for Rust is to compress before serializing, which means follow the Java version for this behavior.
This is correct.
> If so, can we close?
The issue here is that C++ can serialize a TDigest sketch `with_buffer`. So Java's deserialization (heapify) should not assume no buffer in the bytes, which causes the incompatibility. I can submit a patch to demonstrate it and fix it.
There are even prepared test data where we test only those that compress before serializing, not the `tdigest_double_buf_n{scale}_cpp.sk` ones.
https://github.com/apache/datasketches-java/blob/37edd8c9898f8f073aaafad7097cb0949eb84c85/src/test/java/org/apache/datasketches/tdigest/TDigestCrossLanguageTest.java#L37-L79
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [I] Serde incompatible with C++ TDigest (datasketches-java)
Posted by "AlexanderSaydakov (via GitHub)" <gi...@apache.org>.
AlexanderSaydakov commented on issue #701:
URL: https://github.com/apache/datasketches-java/issues/701#issuecomment-3666282396
Apparently, at some point we implemented serializing the buffer in C++, but did not do that in Java.
The default is to compress first, which is an undesirable side effect of serialization, so that we don't have to serialize the buffer (which takes more space). I don't remember why did we decide to provide this option to avoid compression and serializing the buffer.
--
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@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org