raminqaf commented on code in PR #29370:
URL: https://github.com/apache/flink/pull/29370#discussion_r4183139942


##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/functions/VariantCastUtilsTest.java:
##########
@@ -65,6 +83,74 @@ void testCastToVariantRejectsValuesOverTheSizeLimit() {
                 .hasMessageStartingWith("Cannot cast a string value of 
16777212 bytes to VARIANT.");
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"", "hello", "Grüße, 世界 🚀"})
+    void testCastStringToVariantStoresItsUtf8Bytes(final String str) {
+        
assertThat(fromString(binaryString(str.getBytes(UTF_8)))).isEqualTo(BUILDER.of(str));
+    }
+
+    @ParameterizedTest
+    @MethodSource("invalidUtf8")
+    void testCastStringToVariantReplacesInvalidUtf8(final byte[] invalid) {
+        final Variant variant = fromString(binaryString(invalid));
+
+        assertThat(variant.getString()).contains(REPLACEMENT_CHARACTER);
+        assertThat(variant).isEqualTo(BUILDER.of(new String(invalid, UTF_8)));

Review Comment:
   Done in both places. They compare against `BUILDER.of(value.toString())` now.



##########
flink-core/src/test/java/org/apache/flink/types/variant/BinaryVariantInternalBuilderTest.java:
##########
@@ -201,4 +208,58 @@ void testAppendFloat() {
 
         assertThatCode(() -> 
floatList.forEach(builder::appendFloat)).doesNotThrowAnyException();
     }
+
+    @ParameterizedTest
+    @ValueSource(
+            strings = {
+                "",
+                "Grüße, 世界 🚀",
+                "A string longer than 63 bytes is stored with a 4-byte length 
header."

Review Comment:
   Done. The test runs 0, 63 and 64 bytes through `appendString(byte[], int, 
int)` at offset 1 and compares the result with `appendString(byte[])`. I 
checked that `>=` fails the 63-byte case.



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

Reply via email to