gradle-avro-plugin icon indicating copy to clipboard operation
gradle-avro-plugin copied to clipboard

customEncode will not be generated for logicalType decimal

Open mmessell opened this issue 3 years ago • 3 comments

If I define a field like this: { "name": "amount", "type": "bytes", "logicalType": "decimal", "precision": 21, "scale": 3, "doc": "The amount as a decimal number" }

I will get a warning like this: Ignored the amount.logicalType property ("decimal"). It should probably be nested inside the "type" for the field.

The result is that the field will become of type byte and not decimal. If you do as the plugin suggests and define the field inside a type like this: { "name": "amount", "type": { "type": "bytes", "logicalType": "decimal", "precision": 21, "scale": 3 }, "doc": "The amount as a decimal number" }

The warning disappears, but now the customEncode function in my object disappears.. Can be tested with this definition. Just comment/uncomment amount field. { "type": "record", "name": "TestRecord", "namespace": "org.test.common", "doc": "A purchase event happened.", "fields": [ { "name": "amountV1", "type": [ "null", { "type": "record", "name": "AmountV1", "fields": [ { "name": "amount", "type": { "type": "bytes", "logicalType": "decimal", "precision": 21, "scale": 3 }, "doc": "The amount as a decimal number" }, { "name": "currency", "type": { "type": "string", "avro.java.string": "String" }, "doc": "The legal currency (e.g. EUR, PLN, etc.)" } ] } ], "default": null, "doc": "Event Amount." } ] }

I have seen the same issue for logicalType timestamp-millis, so I am guessing it is a general issue..

mmessell avatar Aug 24 '21 08:08 mmessell

Thank you for your bug report. I'm going to focus on the reported decimal issue in this ticket; if you'd like to try to diagnose issues with timestamp-millis, please submit a separate ticket.

The warning you mention is generated by the Avro library (see here). The second form (with the nested object) is the correct form. An example of this form can be found in the plugin's test corpus here.

To reproduce correct behavior:

  • Clone this project's git repo locally
  • Copy src/test/resources/com/github/davidmc24/gradle/plugin/avro/user.avsc to test-project/src/main/avro
  • In test-project, run ./gradlew clean build
  • Check the contents of test-project/build/generated-main-avro-java/example/avro/User.java

When I do this, I see that the salary field is of type BigDecimal. If, in test-project/build.gradle, within the avro block, I add enableDecimalLogicalType = false and re-run the build, I now see that the salary field is of type ByteBuffer.

When I take your example schema, store it as test-project/src/main/avro/TestRecord.avsc, and run the build with enableDecimalLogicalType = true, I get an AmountV1 class with a java.math.BigDecimal amount field, which looks to me to be working as expected.

davidmc24 avatar Aug 24 '21 13:08 davidmc24

Hi David.. The field is, as you point out, generated correctly as a Decimal... but if you look in the TestRecord class you will not find the function customEncode.

If you however remove the amount field, and then look at the TestRecord again, now you will see the function customEncode.

mmessell avatar Aug 24 '21 13:08 mmessell

Why do you think there should be a customEncode function? This plugin doesn't have anything to do with such a function; that's all handled by the Apache Avro library.

davidmc24 avatar Aug 24 '21 15:08 davidmc24

I'm experiencing the same issue with UUID logical type. Without any logical type, plugin generates the customEncode method, but when I add the logical type, it does not generate the customEncode method.

According to @davidmc24 Avro library is making that decision, is that correct? I'm just looking for a confirmation I guess.

Also I tried enableUUIDLogicalType = true doesn't look like a valid config.

Edit: After research I discovered that types are case sensitive, I was using "logicalType": "uuid" and that didn't generate customEncode part, but when I cahhnged it to "logicalType": "UUID", it started generating customEncode method.

apocalypse32 avatar Aug 05 '22 13:08 apocalypse32

I am the author of the plugin. The plugin has never had any code that generates a customEncode method (or, for that matter, actually generates any of the generated Java code). It's all generated by making calls to the Avro library.

Please note that the original poster's problem was, as far as I can tell, that their Avro schema had the declaration of the logical type wrong. It should have been nested inside the "type" for the field, rather than at the top-level of the field. This was called out in a warning by the Avro compiler.

In the repo's test project, there is an example file that works properly with UUID logical encoding.

https://github.com/davidmc24/gradle-avro-plugin/blob/e8412e4c2a58483b4271c5eb6a7e792600289db8/test-project/src/main/avro/UUIDTestRecord.avsc

If you checkout the project, cd into the test-project directory, and run ./gradlew compileJava, you should be able to see that build/generated-main-avro-java/UUIDTestRecord.java has the field correctly generated as a UUID. From there, I recommend that you try to identify what's different between your use-case and that example.

davidmc24 avatar Aug 05 '22 13:08 davidmc24

Thanks for your prompt reply @davidmc24. I ran the way you mentioned and I can confirm depending on uuid vs UUID your test produces different results for UUIDTestRecord.java. And my setup is exactly the way you mentioned but having some issue with google pubsub (Invalid schema message: Message failed schema validation.). I'm just trying to identify the root cause, is it because the generated code or pubsub avro doesn't support uuid logical type.

My field def

{
      "name": "id",
      "type": {
        "type": "string",
        "logicalType": "UUID"
      }
    }

apocalypse32 avatar Aug 05 '22 14:08 apocalypse32

If you're talking about Google Cloud Pub/Sub or something similar, I wouldn't be at all surprised if they had spotty (or entirely lacking) support for logical types. That said, I don't have any direct experience with that.

davidmc24 avatar Aug 05 '22 14:08 davidmc24

That's ok, I'm not looking to solve google pub/sub issue here :). The connection here is the customEncode method, from google example it uses customEncode to publish avro encoded messages, so in your example, it doesn't generate(implement) that method because of the case sensitive nature of the type, as a result the code example throws UnsupportedOperationException exception from abstract method in org.apache.avro.specific.SpecificRecordBase

I believe that part should be addressed otherwise users will not be able to understand why that method is missing, and end up like me :)

apocalypse32 avatar Aug 05 '22 14:08 apocalypse32

Interesting. This is the first I've heard of why someone would care about the customEncode method. Sounds like this might be a good bug report to the Avro project.

davidmc24 avatar Aug 05 '22 14:08 davidmc24

Okay to close

mmessell avatar Aug 05 '22 14:08 mmessell