voonhous commented on code in PR #19495:
URL: https://github.com/apache/hudi/pull/19495#discussion_r3709386372
##########
hudi-trino/src/test/java/io/trino/plugin/hudi/util/TestHudiAvroSerializer.java:
##########
@@ -41,32 +45,39 @@
class TestHudiAvroSerializer
{
- @Test
- public void testDecimalConverter()
- {
- HudiAvroSerializer.AvroDecimalConverter converter = new
HudiAvroSerializer.AvroDecimalConverter();
-
- assertThat(converter.convert(10, 2,
unscaledBytes("123.45"))).isEqualTo(new BigDecimal("123.45"));
- // Same (precision, scale) again: served from the cached schema
- assertThat(converter.convert(10, 2,
unscaledBytes("-0.07"))).isEqualTo(new BigDecimal("-0.07"));
- // Same precision, different scale, and vice versa: must not collide
in the cache
- assertThat(converter.convert(10, 4,
unscaledBytes("123.4567"))).isEqualTo(new BigDecimal("123.4567"));
- assertThat(converter.convert(18, 2,
unscaledBytes("9999999999999999.99"))).isEqualTo(new
BigDecimal("9999999999999999.99"));
- assertThat(converter.convert(5, 0, unscaledBytes("42"))).isEqualTo(new
BigDecimal("42"));
- }
-
- @Test
- public void testAppendShortDecimalFromAvroFixed()
+ /**
+ * A short decimal is stored in Trino as the unscaled value, which is
exactly what Avro writes into the
+ * fixed bytes, so the read is a plain big-endian two's complement decode.
The cases below pin the parts
+ * that decode gets wrong if it is ever rewritten: sign extension for
negatives, scale 0, and the
+ * full-width value at the maximum short-decimal precision.
+ */
+ @ParameterizedTest
+ @MethodSource("shortDecimals")
+ public void testAppendShortDecimalFromAvroFixed(int precision, int scale,
String value, long expectedUnscaled)
{
- DecimalType type = DecimalType.createDecimalType(10, 2);
- byte[] bytes = unscaledBytes("123.45");
+ DecimalType type = DecimalType.createDecimalType(precision, scale);
+ byte[] bytes = unscaledBytes(value);
Review Comment:
Confirmed and fixed. `unscaledBytes` gave the minimal encoding, so `-0.07`
was a single `F9` byte and `-42` a single `D6` -- no case ever crossed a
padding byte, which is exactly the part the decode is most likely to get wrong.
The fixture now sizes the fixed from the precision (matching how
`TestHudiUtilColumnHandles` builds its decimal fixed schema) and runs the value
through Avro's own `DecimalConversion.toFixed`, so the bytes are what a writer
actually emits:
| case | before | after |
| --- | --- | --- |
| `dec(10,2) 123.45` | `30 39` | `00 00 00 30 39` |
| `dec(10,2) -0.07` | `F9` | `FF FF FF FF F9` |
| `dec(5,0) -42` | `D6` | `FF FF D6` |
Same cases, still green.
--
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]
