You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "RyanSkraba (via GitHub)" <gi...@apache.org> on 2026/08/03 16:40:18 UTC
[PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
RyanSkraba opened a new pull request, #3917:
URL: https://github.com/apache/avro/pull/3917
## What is the purpose of the change
When the fastread function was set to be the default in Avro 1.12.1, the behaviour around create GenericData objects changed.
In the past, the GenericData returned standard and reliable "generic" data types that didn't involve casting or reflection, while SpecificData allowed the user to create "specific" java classes (including the `SpecificData.CLASS_PROP` and `SpecificData.KEY_CLASS_PROP` to control the representation of a STRING datum.
Currently, fastread changed this behaviour to apply those properties to the GenericData model, which causes tests to fail in Parquet. This restores the original behaviour.
## Verifying this change
This change added tests and can be verified by running the `TestFastReaderBuilderJavaClassTest`.
## Documentation
- Does this pull request introduce a new feature? **no**
- If yes, how is the feature documented? **no**
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba commented on PR #3917:
URL: https://github.com/apache/avro/pull/3917#issuecomment-5181697675
I changed my mind about starting a strategy of "testing both" with parameters here - GitHub Actions and ./build.sh test will catch if the behaviours diverge. In order for that to work, we _shouldn't_ setFastReadEnabled(...) at all and use it from the global properties.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on PR #3917:
URL: https://github.com/apache/avro/pull/3917#issuecomment-5179730990
@copilot fix the issues you reported
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba merged PR #3917:
URL: https://github.com/apache/avro/pull/3917
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3917:
URL: https://github.com/apache/avro/pull/3917#discussion_r3707522827
##########
lang/java/avro/src/test/java/org/apache/avro/io/FastReaderBuilderJavaClassTest.java:
##########
@@ -18,99 +18,190 @@
package org.apache.avro.io;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.math.BigDecimal;
+import java.util.Map;
import org.apache.avro.Schema;
+import org.apache.avro.SchemaBuilder;
import org.apache.avro.generic.GenericData;
import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.generic.GenericDatumWriter;
import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.GenericRecordBuilder;
+import org.apache.avro.specific.SpecificData;
+import org.apache.avro.util.Utf8;
import org.junit.jupiter.api.Test;
/**
- * Tests for FastReaderBuilder behavior with schemas containing "java-class"
- * attributes.
+ * Tests for FastReaderBuilder behavior with schemas containing
+ * {@link SpecificData#CLASS_PROP} and {@link SpecificData#KEY_CLASS_PROP}
+ * attributes. Note that {@link SpecificData#ELEMENT_PROP} isn't tested because
+ * it is only used by ReflectData.
*/
public class FastReaderBuilderJavaClassTest {
+ private static final Schema SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP = SchemaBuilder.record("NullableStringRecord")
+ .fields().requiredString("id").name("price").type().unionOf().nullType().and()
+ .type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP, "java.math.BigDecimal").endString())
+ .endUnion().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_NULLABLE_CLASS_PROP = new GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP).set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_CLASS_PROP = SchemaBuilder.record("StringRecord").fields()
+ .requiredString("id").name("price")
+ .type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP, "java.math.BigDecimal").endString())
+ .noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_CLASS_PROP = new GenericRecordBuilder(SCHEMA_RECORD_WITH_CLASS_PROP)
+ .set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP = SchemaBuilder.record("MapRecord").fields()
+ .requiredString("id").name("prices").type().map().prop(SpecificData.KEY_CLASS_PROP, "java.math.BigDecimal")
+ .values().stringType().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_MAP_KEY_CLASS_PROP = new GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP).set("id", "123")
+ .set("prices", Map.of("-0.0002", "cheap", "12345.678", "expensive")).build();
+
+ /**
+ * Reusable round-trip logic for a record, using the given model.
+ */
+ public static GenericRecord roundTrip(GenericRecord record, GenericData model) throws IOException {
+ byte[] serialized;
+
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ GenericDatumWriter<GenericRecord> writer = new GenericDatumWriter<>(record.getSchema());
+ BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(baos, null);
+ writer.write(record, encoder);
+ encoder.flush();
+ serialized = baos.toByteArray();
+ }
+
+ GenericDatumReader<GenericRecord> reader = new GenericDatumReader<>(record.getSchema(), record.getSchema(), model);
+ BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized, null);
+ return reader.read(null, decoder);
+ }
+
/**
- * Tests that GenericDatumReader can deserialize records with string fields that
- * have a "java-class" attribute (e.g., BigDecimal).
- *
- * This test reproduces a bug where
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a string field inside a union,
+ * matching the classic (non fast-reader) behavior of GenericData.
+ * <p>
+ * This test also reproduces a bug (AVRO-4225) where
* FastReaderBuilder.getTransformingStringReader() casts the result of
- * stringReader.read() directly to String, but in GenericData mode the reader
- * returns Utf8, causing a ClassCastException.
+ * stringReader.read() directly to String, but GenericData returns Utf8, causing
+ * a ClassCastException
*/
@Test
- void genericDatumReaderWithJavaClassAttribute() throws IOException {
- // Schema with a string field that has "java-class": "java.math.BigDecimal"
- // This is a common pattern for representing decimal values as strings
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\": \"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"id\", \"type\": \"string\"},\n" + " {\"name\": \"price\", \"type\": [\"null\", {\n"
- + " \"type\": \"string\",\n" + " \"java-class\": \"java.math.BigDecimal\"\n" + " }]}\n" + " ]\n"
- + "}";
-
- Schema schema = new Schema.Parser().parse(schemaJson);
-
- GenericRecord record = new GenericData.Record(schema);
- record.put("id", "123");
- record.put("price", "-0.0002");
-
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- GenericDatumWriter<GenericRecord> writer = new GenericDatumWriter<>(schema);
- BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(out, null);
- writer.write(record, encoder);
- encoder.flush();
-
- byte[] serialized = out.toByteArray();
-
- // Deserialize using GenericDatumReader (which uses FastReaderBuilder by
- // default)
- GenericDatumReader<GenericRecord> reader = new GenericDatumReader<>(schema);
- BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized, null);
+ void genericDataModelIgnoresJavaClassPropWithStringUnion() throws IOException {
+ // This round trip shouldn't cause a ClassCastException (AVRO-4225)
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP, GenericData.get());
Review Comment:
This test relies on the global default for fast-reader enablement (system property + GenericData singleton). If fast-reader is disabled in the test JVM, this won’t exercise FastReaderBuilder and may not catch regressions like AVRO-4225. Prefer using a fresh model instance with fast-reader explicitly enabled.
This issue also appears in the following locations of the same file:
- line 120
- line 169
##########
lang/java/avro/src/test/java/org/apache/avro/io/FastReaderBuilderJavaClassTest.java:
##########
@@ -18,99 +18,190 @@
package org.apache.avro.io;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.math.BigDecimal;
+import java.util.Map;
import org.apache.avro.Schema;
+import org.apache.avro.SchemaBuilder;
import org.apache.avro.generic.GenericData;
import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.generic.GenericDatumWriter;
import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.GenericRecordBuilder;
+import org.apache.avro.specific.SpecificData;
+import org.apache.avro.util.Utf8;
import org.junit.jupiter.api.Test;
/**
- * Tests for FastReaderBuilder behavior with schemas containing "java-class"
- * attributes.
+ * Tests for FastReaderBuilder behavior with schemas containing
+ * {@link SpecificData#CLASS_PROP} and {@link SpecificData#KEY_CLASS_PROP}
+ * attributes. Note that {@link SpecificData#ELEMENT_PROP} isn't tested because
+ * it is only used by ReflectData.
*/
public class FastReaderBuilderJavaClassTest {
+ private static final Schema SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP = SchemaBuilder.record("NullableStringRecord")
+ .fields().requiredString("id").name("price").type().unionOf().nullType().and()
+ .type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP, "java.math.BigDecimal").endString())
+ .endUnion().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_NULLABLE_CLASS_PROP = new GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP).set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_CLASS_PROP = SchemaBuilder.record("StringRecord").fields()
+ .requiredString("id").name("price")
+ .type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP, "java.math.BigDecimal").endString())
+ .noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_CLASS_PROP = new GenericRecordBuilder(SCHEMA_RECORD_WITH_CLASS_PROP)
+ .set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP = SchemaBuilder.record("MapRecord").fields()
+ .requiredString("id").name("prices").type().map().prop(SpecificData.KEY_CLASS_PROP, "java.math.BigDecimal")
+ .values().stringType().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_MAP_KEY_CLASS_PROP = new GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP).set("id", "123")
+ .set("prices", Map.of("-0.0002", "cheap", "12345.678", "expensive")).build();
+
+ /**
+ * Reusable round-trip logic for a record, using the given model.
+ */
+ public static GenericRecord roundTrip(GenericRecord record, GenericData model) throws IOException {
+ byte[] serialized;
+
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ GenericDatumWriter<GenericRecord> writer = new GenericDatumWriter<>(record.getSchema());
+ BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(baos, null);
+ writer.write(record, encoder);
+ encoder.flush();
+ serialized = baos.toByteArray();
+ }
+
+ GenericDatumReader<GenericRecord> reader = new GenericDatumReader<>(record.getSchema(), record.getSchema(), model);
+ BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized, null);
+ return reader.read(null, decoder);
+ }
+
/**
- * Tests that GenericDatumReader can deserialize records with string fields that
- * have a "java-class" attribute (e.g., BigDecimal).
- *
- * This test reproduces a bug where
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a string field inside a union,
+ * matching the classic (non fast-reader) behavior of GenericData.
+ * <p>
+ * This test also reproduces a bug (AVRO-4225) where
* FastReaderBuilder.getTransformingStringReader() casts the result of
- * stringReader.read() directly to String, but in GenericData mode the reader
- * returns Utf8, causing a ClassCastException.
+ * stringReader.read() directly to String, but GenericData returns Utf8, causing
+ * a ClassCastException
*/
@Test
- void genericDatumReaderWithJavaClassAttribute() throws IOException {
- // Schema with a string field that has "java-class": "java.math.BigDecimal"
- // This is a common pattern for representing decimal values as strings
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\": \"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"id\", \"type\": \"string\"},\n" + " {\"name\": \"price\", \"type\": [\"null\", {\n"
- + " \"type\": \"string\",\n" + " \"java-class\": \"java.math.BigDecimal\"\n" + " }]}\n" + " ]\n"
- + "}";
-
- Schema schema = new Schema.Parser().parse(schemaJson);
-
- GenericRecord record = new GenericData.Record(schema);
- record.put("id", "123");
- record.put("price", "-0.0002");
-
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- GenericDatumWriter<GenericRecord> writer = new GenericDatumWriter<>(schema);
- BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(out, null);
- writer.write(record, encoder);
- encoder.flush();
-
- byte[] serialized = out.toByteArray();
-
- // Deserialize using GenericDatumReader (which uses FastReaderBuilder by
- // default)
- GenericDatumReader<GenericRecord> reader = new GenericDatumReader<>(schema);
- BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized, null);
+ void genericDataModelIgnoresJavaClassPropWithStringUnion() throws IOException {
+ // This round trip shouldn't cause a ClassCastException (AVRO-4225)
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP, GenericData.get());
+
+ assertNotNull(result);
+ assertInstanceOf(Utf8.class, result.get("id"));
+ assertEquals("123", result.get("id").toString());
+ assertInstanceOf(Utf8.class, result.get("price"), "GenericData should ignore 'java-class'");
+ assertEquals("-0.0002", result.get("price").toString());
+ }
- // AVRO-4225 this should not throw ClassCastException: Utf8 cannot be cast
- // to String
- GenericRecord result = reader.read(null, decoder);
+ /**
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a direct (non-union) string
+ * field.
+ */
+ @Test
+ void genericDataModelIgnoresJavaClassPropWithString() throws IOException {
+ GenericRecord result = roundTrip(RECORD_WITH_CLASS_PROP, GenericData.get());
assertNotNull(result);
+ assertInstanceOf(Utf8.class, result.get("id"));
assertEquals("123", result.get("id").toString());
+ assertInstanceOf(Utf8.class, result.get("price"), "GenericData should ignore 'java-class'");
assertEquals("-0.0002", result.get("price").toString());
}
/**
- * Tests that GenericDatumReader can deserialize records with a direct string
- * field (not in a union) that has a "java-class" attribute.
+ * Tests that a GenericDatumReader built on the SpecificData model uses
+ * {@link SpecificData#CLASS_PROP} on a string field inside a union,
+ * transforming it into the named class without throwing ClassCastException.
*/
@Test
- void genericDatumReaderWithDirectJavaClassString() throws IOException {
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\": \"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"amount\", \"type\": {\n" + " \"type\": \"string\",\n"
- + " \"java-class\": \"java.math.BigDecimal\"\n" + " }}\n" + " ]\n" + "}";
+ void specificDataModelUsesJavaClassProp() throws IOException {
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP, SpecificData.get());
Review Comment:
This test should explicitly enable fast-reader on a fresh SpecificData instance to ensure it validates the FastReaderBuilder path regardless of JVM/system-property configuration.
This issue also appears in the following locations of the same file:
- line 152
- line 192
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba commented on PR #3917:
URL: https://github.com/apache/avro/pull/3917#issuecomment-5189968906
Cherry-picked to [branch-1.12](https://github.com/apache/avro/commit/be1443838fe84dd4d3b7032568c6c4d8ef6028d6).
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4322: [Java] Only apply java-class annotations to SpecificData models [avro]
Posted by "RyanSkraba (via GitHub)" <gi...@apache.org>.
RyanSkraba commented on PR #3917:
URL: https://github.com/apache/avro/pull/3917#issuecomment-5181494252
Copilot isn't wrong (except that we are pretty much guaranteed to run with fast-read and classic read because of the pom wrangling we do). I'm going to update this class to test both behaviours to ensure they remain consistent.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org