JinkunLiu commented on code in PR #27017:
URL: https://github.com/apache/flink/pull/27017#discussion_r2487148629
##########
flink-core/src/test/java/org/apache/flink/types/variant/BinaryVariantInternalBuilderTest.java:
##########
@@ -116,4 +119,12 @@ void testParseJsonObject() throws IOException {
assertThat(variant.getField("k1").getByte()).isEqualTo((byte) 2);
assertThat(variant.getField("k2").getDecimal()).isEqualTo(BigDecimal.valueOf(1.5));
}
+
+ @Test
+ void testAppendFloat() {
+ BinaryVariantInternalBuilder builder = new
BinaryVariantInternalBuilder(false);
+ ArrayList<Float> floatList = new ArrayList<>(Collections.nCopies(25,
4.2f));
+
+ assertThatCode(() ->
floatList.forEach(builder::appendFloat)).doesNotThrowAnyException();
Review Comment:
Thank you for the valuable suggestion — I also agree that having symmetric
tests is important.
But This bug is quite special and would never occur in real-world scenarios.
It only triggers when the buffer size happens to exceed the default char[128]
during appendFloat. However, appendFloat is only referenced in
org.apache.flink.types.variant.BinaryVariantBuilder#of(float), and each time
it’s called, a new internalBuilder instance is created and only adds one float.
Therefore, this bug would never actually occur in practice.
Since the internalBuilder was only designed to store a single float when
handling Float values, there’s no existing function to read cases involving
more than one float. Therefore, it’s hard to construct a symmetric read test
that writes and reads more than 25 floats.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
