Skip to content

Commit 882ef5b

Browse files
cowtowncodercowtowncoder
authored
Backport #745 (+ follow-ups) to 2.x: Avro union resolution and schema default precision (#754)
1 parent 9f81ea6 commit 882ef5b

7 files changed

Lines changed: 286 additions & 16 deletions

File tree

‎avro/src/main/java/com/fasterxml/jackson/dataformat/avro/deser/AvroFieldDefaulters.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ public static AvroFieldReader createDefaulter(String name,
3131
case VALUE_NUMBER_INT:
3232
switch (defaultAsNode.numberType()) {
3333
case INT:
34-
return new ScalarDefaults.FloatDefaults(name, defaultAsNode.asInt());
34+
return new ScalarDefaults.IntDefaults(name, defaultAsNode.asInt());
3535
case BIG_INTEGER: // TODO: maybe support separately?
3636
case LONG:
3737
default:
38-
return new ScalarDefaults.FloatDefaults(name, defaultAsNode.asLong());
38+
return new ScalarDefaults.LongDefaults(name, defaultAsNode.asLong());
3939
}
4040
case VALUE_STRING:
4141
return new ScalarDefaults.StringDefaults(name, defaultAsNode.asText());

‎avro/src/main/java/com/fasterxml/jackson/dataformat/avro/ser/AvroWriteContext.java‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.math.BigDecimal;
55
import java.util.*;
66

7+
import org.apache.avro.LogicalTypes;
78
import org.apache.avro.Schema;
89
import org.apache.avro.Schema.Type;
910
import org.apache.avro.UnresolvedUnionException;
@@ -171,6 +172,7 @@ protected GenericRecord _createRecord(Schema schema, Object currValue) throws Js
171172
// couldn't find an exact match
172173
schema = _recordOrMapFromUnion(schema);
173174
}
175+
type = schema.getType();
174176
}
175177
if (type == Schema.Type.MAP) {
176178
throw new IllegalStateException("_createRecord should never be called for elements of type MAP");
@@ -189,6 +191,7 @@ protected GenericRecord _createRecord(Schema schema) throws JsonMappingException
189191
Type type = schema.getType();
190192
if (type == Schema.Type.UNION) {
191193
schema = _recordOrMapFromUnion(schema);
194+
type = schema.getType();
192195
}
193196
if (type == Schema.Type.MAP) {
194197
throw new IllegalStateException("_createRecord should never be called for elements of type MAP");
@@ -440,25 +443,41 @@ private static int _findNotNullIndex(List<Schema> types)
440443

441444
private static int _resolveBigDecimalIndex(Schema unionSchema, List<Schema> types,
442445
BigDecimal value) {
443-
int match = -1;
446+
// Branches are considered in order of how well they retain the value,
447+
// regardless of declaration order: "decimal" first, then String, then Double
448+
int stringMatch = -1;
449+
int doubleMatch = -1;
444450

445451
for (int i = 0, size = types.size(); i < size; ++i) {
446452
Schema schema = types.get(i);
447453
Schema.Type t = schema.getType();
448454

449-
if (t == Type.DOUBLE) {
450-
return i;
451-
}
452-
// BigDecimals can be shoved into a double, but optimally would be a String or byte[] with logical type information
453-
if (t == Type.DOUBLE) {
454-
match = i;
455-
continue;
455+
if (t == Type.BYTES || t == Type.FIXED) {
456+
// Best match: retains both scale and type.
457+
// NOTE: plain `bytes`/`fixed` must NOT be chosen, since conversion
458+
// requires the "decimal" logical type to exist
459+
if (schema.getLogicalType() instanceof LogicalTypes.Decimal) {
460+
return i;
461+
}
462+
} else if (t == Type.STRING) {
463+
// Second best: retains all digits, but reads back as String
464+
if (stringMatch < 0) {
465+
stringMatch = i;
466+
}
467+
} else if (t == Type.DOUBLE) {
468+
// Last resort: lossy
469+
if (doubleMatch < 0) {
470+
doubleMatch = i;
471+
}
456472
}
457473
}
458-
if (match < 0) {
459-
match = ReflectData.get().resolveUnion(unionSchema, value);
474+
if (stringMatch >= 0) {
475+
return stringMatch;
460476
}
461-
return match;
477+
if (doubleMatch >= 0) {
478+
return doubleMatch;
479+
}
480+
return ReflectData.get().resolveUnion(unionSchema, value);
462481
}
463482

464483
private static int _resolveMapIndex(Schema unionSchema, List<Schema> types,

‎avro/src/main/java/com/fasterxml/jackson/dataformat/avro/ser/RootContext.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,17 @@ public final AvroWriteContext createChildObjectContext(Object currValue) throws
6060
// verify that root type is record (or compatible)
6161
switch (_schema.getType()) {
6262
case RECORD:
63-
case UNION: // maybe
6463
{
6564
GenericRecord rec = _createRecord(_schema, currValue);
6665
_rootValue = rec;
6766
return new ObjectWriteContext(this, _generator, rec, currValue);
6867
}
68+
case UNION: // maybe: may resolve to either Record or Map
69+
{
70+
AvroWriteContext child = _createObjectContext(_schema, currValue);
71+
_rootValue = child.rawValue();
72+
return child;
73+
}
6974
case MAP: // used to not be supported
7075
{
7176
MapWriteContext ctxt = new MapWriteContext(this, _generator, _schema, currValue);

avro/src/test/java/com/fasterxml/jackson/dataformat/avro/BigDecimal_serialization_and_deserializationTest.java renamed to avro/src/test/java/com/fasterxml/jackson/dataformat/avro/BigDecimalSerializationAndDeserializationTest.java

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.fasterxml.jackson.dataformat.avro;
22

33
import java.math.BigDecimal;
4+
import java.util.LinkedHashMap;
5+
import java.util.Map;
46

57
import org.junit.jupiter.api.Test;
68

@@ -9,7 +11,7 @@
911

1012
import static org.assertj.core.api.Assertions.assertThat;
1113

12-
public class BigDecimal_serialization_and_deserializationTest extends AvroTestBase {
14+
public class BigDecimalSerializationAndDeserializationTest extends AvroTestBase {
1315
private static final AvroMapper MAPPER = new AvroMapper();
1416

1517
static class BigDecimalAndName {
@@ -177,4 +179,126 @@ public void testSerialization_toFixedWithLogicalTypeDecimal() throws Exception {
177179
assertThat(result.name).isEqualTo("peter");
178180
}
179181

182+
/*
183+
/**********************************************************************
184+
/* Tests for union branch selection
185+
/**********************************************************************
186+
*/
187+
188+
// Verify BigDecimal in a union with STRING and DOUBLE selects STRING (not DOUBLE)
189+
@Test
190+
public void testBigDecimalUnionPrefersStringOverDouble() throws Exception
191+
{
192+
AvroSchema schema = _unionSchema("'string','double'");
193+
194+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
195+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
196+
197+
Map<String, Object> result = MAPPER.readerFor(Map.class)
198+
.with(schema)
199+
.readValue(bytes);
200+
// Serialized as String, not double: retains full precision
201+
assertThat(result.get("value")).isInstanceOf(String.class);
202+
}
203+
204+
// Verify BigDecimal in a union with BYTES ("decimal" logical type) selects BYTES
205+
@Test
206+
public void testBigDecimalUnionPrefersBytesOverDouble() throws Exception
207+
{
208+
AvroSchema schema = _unionSchema(
209+
"{'type':'bytes','logicalType':'decimal','precision':20,'scale':6},'double'");
210+
211+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
212+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
213+
214+
Map<String, Object> result = MAPPER.readerFor(Map.class)
215+
.with(schema)
216+
.readValue(bytes);
217+
assertThat(result.get("value")).isInstanceOf(BigDecimal.class);
218+
assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
219+
}
220+
221+
// Verify that plain BYTES (without "decimal" logical type) is NOT chosen for
222+
// BigDecimal: conversion would fail, so DOUBLE must be used instead
223+
@Test
224+
public void testBigDecimalUnionSkipsPlainBytes() throws Exception
225+
{
226+
AvroSchema schema = _unionSchema("'bytes','double'");
227+
228+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
229+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
230+
231+
Map<String, Object> result = MAPPER.readerFor(Map.class)
232+
.with(schema)
233+
.readValue(bytes);
234+
assertThat(result.get("value")).isInstanceOf(Double.class);
235+
assertThat((Double) result.get("value")).isEqualTo(input.doubleValue());
236+
}
237+
238+
// Verify that "decimal" BYTES wins over STRING even when declared later:
239+
// preference is by type fidelity, not by declaration order
240+
@Test
241+
public void testBigDecimalUnionPrefersBytesOverString() throws Exception
242+
{
243+
AvroSchema schema = _unionSchema(
244+
"'string',{'type':'bytes','logicalType':'decimal','precision':20,'scale':6},'double'");
245+
246+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
247+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
248+
249+
Map<String, Object> result = MAPPER.readerFor(Map.class)
250+
.with(schema)
251+
.readValue(bytes);
252+
assertThat(result.get("value")).isInstanceOf(BigDecimal.class);
253+
assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
254+
}
255+
256+
// Verify that FIXED with "decimal" logical type is also usable for BigDecimal
257+
@Test
258+
public void testBigDecimalUnionPrefersFixedOverString() throws Exception
259+
{
260+
AvroSchema schema = _unionSchema(
261+
"'string',{'type':'fixed','name':'Dec','size':16,"
262+
+"'logicalType':'decimal','precision':20,'scale':6},'double'");
263+
264+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
265+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
266+
267+
Map<String, Object> result = MAPPER.readerFor(Map.class)
268+
.with(schema)
269+
.readValue(bytes);
270+
assertThat(result.get("value")).isInstanceOf(BigDecimal.class);
271+
assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
272+
}
273+
274+
// Verify that plain FIXED (without "decimal" logical type) is not chosen either
275+
@Test
276+
public void testBigDecimalUnionSkipsPlainFixed() throws Exception
277+
{
278+
AvroSchema schema = _unionSchema("{'type':'fixed','name':'Raw','size':16},'double'");
279+
280+
BigDecimal input = BigDecimal.valueOf(123456789, 6);
281+
byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(_value(input));
282+
283+
Map<String, Object> result = MAPPER.readerFor(Map.class)
284+
.with(schema)
285+
.readValue(bytes);
286+
assertThat(result.get("value")).isInstanceOf(Double.class);
287+
assertThat((Double) result.get("value")).isEqualTo(input.doubleValue());
288+
}
289+
290+
// Record with a single 'value' property, typed as a union of 'null' plus given branches
291+
private AvroSchema _unionSchema(String branches) throws Exception {
292+
return MAPPER.schemaFrom(aposToQuotes("{"
293+
+"'type':'record',"
294+
+"'name':'Test',"
295+
+"'fields':[{'name':'value', 'type':['null',"+branches+"]}]"
296+
+"}"));
297+
}
298+
299+
private Map<String,Object> _value(Object value) {
300+
Map<String,Object> map = new LinkedHashMap<>();
301+
map.put("value", value);
302+
return map;
303+
}
180304
}

‎avro/src/test/java/com/fasterxml/jackson/dataformat/avro/MapWithUnionTest.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,56 @@ public void testMapContainerWithNested() throws IOException
132132
assertEquals("bing", m.get("zap"));
133133
}
134134

135+
// Verify that a root-level union resolving to MAP is written as a Map,
136+
// and not passed to Record handling
137+
@Test
138+
public void testRootUnionResolvingToMapType() throws Exception
139+
{
140+
AvroSchema schema = MAPPER.schemaFrom(aposToQuotes(
141+
"['null',{'type':'map','values':'string'}]"));
142+
143+
Map<String,Object> input = _map("key1", "val1", "key2", "val2");
144+
byte[] avro = MAPPER.writer(schema).writeValueAsBytes(input);
145+
146+
Map<String,Object> result = MAPPER.readerFor(Map.class)
147+
.with(schema)
148+
.readValue(avro);
149+
assertEquals("val1", result.get("key1"));
150+
assertEquals("val2", result.get("key2"));
151+
}
152+
153+
// Verify that a union resolving to MAP is handled correctly for Record properties
154+
@Test
155+
public void testUnionResolvingToMapType() throws Exception
156+
{
157+
AvroSchema schema = MAPPER.schemaFrom(aposToQuotes("{"
158+
+"'type':'record',"
159+
+"'name':'Test',"
160+
+"'fields':["
161+
+" {'name':'data', 'type':['null',"
162+
+" {'type':'record','name':'Nested','fields':["
163+
+" {'name':'x','type':'int'}"
164+
+" ]},"
165+
+" {'type':'map','values':'string'}"
166+
+" ]}"
167+
+"]"
168+
+"}"));
169+
170+
// Write a record where 'data' is a map (union index 2)
171+
Map<String,Object> input = _map("data", _map("key1", "val1", "key2", "val2"));
172+
byte[] avro = MAPPER.writer(schema).writeValueAsBytes(input);
173+
174+
Map<String,Object> result = MAPPER.readerFor(Map.class)
175+
.with(schema)
176+
.readValue(avro);
177+
Object ob = result.get("data");
178+
assertTrue(ob instanceof Map<?,?>,
179+
"Expected Map but got "+((ob == null) ? "NULL" : ob.getClass().getSimpleName()));
180+
Map<?,?> dataMap = (Map<?,?>) ob;
181+
assertEquals("val1", dataMap.get("key1"));
182+
assertEquals("val2", dataMap.get("key2"));
183+
}
184+
135185
private Map<String,Object> _map(Object... args) {
136186
Map<String,Object> m = new LinkedHashMap<>();
137187
for (int i = 0; i < args.length; i += 2) {

‎avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schemaev/ComplexDefaultsTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.fasterxml.jackson.dataformat.avro.schemaev;
22

3+
import java.util.LinkedHashMap;
34
import java.util.List;
45
import java.util.Map;
56

@@ -193,4 +194,75 @@ public void testListDefaults() throws Exception
193194
assertEquals("Fo", result.data.get(0));
194195
assertEquals("obar", result.data.get(1));
195196
}
197+
198+
/*
199+
/**********************************************************************
200+
/* Tests for integer default precision (not stored as float)
201+
/**********************************************************************
202+
*/
203+
204+
// Verify that int default values preserve precision (not stored as float)
205+
@Test
206+
public void testIntDefaultValuePrecision() throws Exception
207+
{
208+
final AvroSchema srcSchema = MAPPER.schemaFrom(aposToQuotes("{"
209+
+"'type':'record','name':'RootType',"
210+
+"'fields':["
211+
+" {'name':'x','type':'int'},"
212+
+" {'name':'y','type':'int'}"
213+
+"]"
214+
+"}"));
215+
// Default value 20000001 exceeds float's 24-bit mantissa (max exact int: 16777216)
216+
final AvroSchema dstSchema = MAPPER.schemaFrom(aposToQuotes("{"
217+
+"'type':'record','name':'RootType',"
218+
+"'fields':["
219+
+" {'name':'x','type':'int'},"
220+
+" {'name':'largeInt','type':'int','default':20000001},"
221+
+" {'name':'y','type':'int'}"
222+
+"]"
223+
+"}"));
224+
225+
Map<String,Object> input = new LinkedHashMap<>();
226+
input.put("x", 1);
227+
input.put("y", 2);
228+
byte[] avro = MAPPER.writer(srcSchema).writeValueAsBytes(input);
229+
230+
Map<String,Object> result = MAPPER.readerFor(Map.class)
231+
.with(srcSchema.withReaderSchema(dstSchema))
232+
.readValue(avro);
233+
assertEquals(1, result.get("x"));
234+
assertEquals(2, result.get("y"));
235+
// The critical assertion: default 20000001 must survive float truncation
236+
assertEquals(20000001, ((Number) result.get("largeInt")).intValue(),
237+
"Integer default lost precision -- likely stored as float");
238+
}
239+
240+
// Verify that long default values preserve precision
241+
@Test
242+
public void testLongDefaultValuePrecision() throws Exception
243+
{
244+
final AvroSchema srcSchema = MAPPER.schemaFrom(aposToQuotes("{"
245+
+"'type':'record','name':'RootType',"
246+
+"'fields':[{'name':'x','type':'int'}]"
247+
+"}"));
248+
// Default value 9007199254740993 exceeds float AND double integer precision
249+
final AvroSchema dstSchema = MAPPER.schemaFrom(aposToQuotes("{"
250+
+"'type':'record','name':'RootType',"
251+
+"'fields':["
252+
+" {'name':'x','type':'int'},"
253+
+" {'name':'bigLong','type':'long','default':9007199254740993}"
254+
+"]"
255+
+"}"));
256+
257+
Map<String,Object> input = new LinkedHashMap<>();
258+
input.put("x", 1);
259+
byte[] avro = MAPPER.writer(srcSchema).writeValueAsBytes(input);
260+
261+
Map<String,Object> result = MAPPER.readerFor(Map.class)
262+
.with(srcSchema.withReaderSchema(dstSchema))
263+
.readValue(avro);
264+
assertEquals(1, result.get("x"));
265+
assertEquals(9007199254740993L, ((Number) result.get("bigLong")).longValue(),
266+
"Long default lost precision -- likely stored as float");
267+
}
196268
}

0 commit comments

Comments
 (0)