raminqaf commented on code in PR #29370:
URL: https://github.com/apache/flink/pull/29370#discussion_r4183139340
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/VariantCastUtils.java:
##########
@@ -593,14 +593,31 @@ public static Variant fromDouble(double value) {
}
public static Variant fromDecimal(DecimalData value) {
- return BUILDER.of(value.toBigDecimal());
+ if (!value.isCompact()) {
+ return BUILDER.of(value.toBigDecimal());
+ }
+ final BinaryVariantInternalBuilder builder = new
BinaryVariantInternalBuilder(false);
+ builder.appendDecimal(value.toUnscaledLong(), value.scale());
+ return builder.build();
}
+ /**
+ * Stores the UTF-8 bytes of the string as they are. A {@link StringData}
may hold invalid
+ * UTF-8, which the variant spec does not allow, so such a value is
decoded first and every
+ * malformed sequence is stored as the U+FFFD replacement character.
+ */
public static Variant fromString(StringData value) {
+ final byte[] utf8 = value.toBytes();
Review Comment:
Yes. `BinaryStringData` is the only `StringData` implementation, and the
runtime already casts to it in many places. `fromString` now reads a string
that lies in the first heap segment in place, through a new
`appendString(byte[], int, int)`, so it no longer copies the bytes out with
`toBytes()`. Otherwise it still takes the copy, and a string that only exists
as a Java object is encoded straight from it, as on master.
`VariantCastUtilsTest` covers each layout.
Against the previous revision, JMH shows the cast 6 to 17% faster, and
allocation drops by a quarter from 1 KiB up. A 64 KiB string now allocates 197
KB per call instead of 262 KB.
--
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]
