AHeise commented on code in PR #29370:
URL: https://github.com/apache/flink/pull/29370#discussion_r4183489872
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/VariantCastUtils.java:
##########
@@ -593,17 +594,64 @@ public static Variant fromDouble(double value) {
}
public static Variant fromDecimal(DecimalData value) {
- return BUILDER.of(value.toBigDecimal());
+ final BinaryVariantInternalBuilder builder = new
BinaryVariantInternalBuilder(false);
+ appendDecimal(builder, value);
+ return builder.build();
+ }
+
+ /** Like {@link #fromDecimal(DecimalData)}, but writes into a shared
builder. */
+ static void appendDecimal(BinaryVariantInternalBuilder builder,
DecimalData value) {
+ if (value.isCompact()) {
+ builder.appendDecimal(value.toUnscaledLong(), value.scale());
+ } else {
+ builder.appendDecimal(value.toBigDecimal());
+ }
}
+ /**
+ * 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 BinaryVariantInternalBuilder builder = new
BinaryVariantInternalBuilder(false);
try {
- return BUILDER.of(value.toString());
+ appendString(builder, value);
+ return builder.build();
} catch (VariantTypeException e) {
throw sizeLimitExceeded(e, "string", value.toBytes().length);
Review Comment:
nit: `toBytes()` copies up to 16 MiB just to report the length.
`((BinaryStringData) value).getSizeInBytes()` gives the same number without the
copy for a binary string. Only the failure path, so fine either way.
--
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]
