JinkunLiu commented on code in PR #27017:
URL: https://github.com/apache/flink/pull/27017#discussion_r2487263177
##########
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.
However, 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 public function or public member
variable to read cases involving more than one float. Therefore, it’s hard to
construct a symmetric 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]
