fmiguelez opened a new issue, #15858:
URL: https://github.com/apache/pulsar/issues/15858
**Describe the bug**
When JSR310 is enabled in `SchemaDefintionBuilder` the
`TimestampMillisConversion` is registered for `java.time.Instant` (using
logical type `timestamp-millis`), however that conversion is never applied as
`TimestampMicrosConversion` is also registered first (i.e. with higher
priority) inside `AvroSchema` class of Pulsar client.
E.g. take this POJO definition:
```
public class TestObject
{
private Instant startTimestamp;
public Instant getStartTimestamp() { return startTimestamp; }
public void setStartTimestamp(Instant startTimestamp) {
this.startTimestamp = startTimestamp; }
}
```
Consider these two ways of obtaining AVRO schema from POJO:
```
Schema<TestObject> schema = Schema.AVRO(TestObject.class);
Schema<TestObject> schema =
AvroSchema.of(SchemaDefinition.<T>builder().withPojo(TestObject.class).withJSR310ConversionEnabled(true).build());
```
These two methods produce same AVRO type for field `startTimestamp` (`union`
with `null` has been omited for clarity):
```
{"type":"long","logicalType":"timestamp-micros"}
```
Second method should however produce following type (**`timestamp-millis`**
instead of **`timestamp-micros`**):
```
{"type":"long","logicalType":"timestamp-millis"}
```
If we look at `AvroSchema.addLogicalTypeConversions()` we can spot the BUG:
```
public static void addLogicalTypeConversions(ReflectData reflectData,
boolean jsr310ConversionEnabled, boolean decimalConversionEnabled) {
if (decimalConversionEnabled) {
reflectData.addLogicalTypeConversion(new DecimalConversion());
}
reflectData.addLogicalTypeConversion(new DateConversion());
reflectData.addLogicalTypeConversion(new TimeMillisConversion());
reflectData.addLogicalTypeConversion(new TimeMicrosConversion());
reflectData.addLogicalTypeConversion(new
TimestampMicrosConversion());
if (jsr310ConversionEnabled) {
// This registration has no effect as TimestampMicrosConversion
is
// registered first and always takes precedence
reflectData.addLogicalTypeConversion(new
TimestampMillisConversion());
} else {
try {
Class.forName("org.joda.time.DateTime");
reflectData.addLogicalTypeConversion(new
AvroSchema.TimestampConversion());
} catch (ClassNotFoundException var4) {
}
}
reflectData.addLogicalTypeConversion(new UUIDConversion());
}
```
One fix could be the following:
```
public static void addLogicalTypeConversions(ReflectData reflectData,
boolean jsr310ConversionEnabled, boolean decimalConversionEnabled) {
if (decimalConversionEnabled) {
reflectData.addLogicalTypeConversion(new DecimalConversion());
}
reflectData.addLogicalTypeConversion(new DateConversion());
reflectData.addLogicalTypeConversion(new TimeMillisConversion());
reflectData.addLogicalTypeConversion(new TimeMicrosConversion());
if (jsr310ConversionEnabled) {
reflectData.addLogicalTypeConversion(new
TimestampMillisConversion());
} else {
// Register alternative conversion to micros if JSR310 is not
enabled
reflectData.addLogicalTypeConversion(new
TimestampMicrosConversion());
try {
Class.forName("org.joda.time.DateTime");
reflectData.addLogicalTypeConversion(new
AvroSchema.TimestampConversion());
} catch (ClassNotFoundException var4) {
}
}
reflectData.addLogicalTypeConversion(new UUIDConversion());
}
```
**To Reproduce**
Already indicated in the description.
**Expected behavior**
When JSR310 flag is enabled interpret use logical type `timestamp-millis`
instead of `timestamp-micros` in AVRO Schema for `java.time.Instant` types.
**Screenshots**
None
**Desktop (please complete the following information):**
NA
**Additional context**
Affects all versions since **2.5.1**
--
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]
