[
https://issues.apache.org/jira/browse/AVRO-3408?focusedWorklogId=740194&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-740194
]
ASF GitHub Bot logged work on AVRO-3408:
----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Mar/22 18:11
Start Date: 11/Mar/22 18:11
Worklog Time Spent: 10m
Work Description: opwvhk commented on a change in pull request #1584:
URL: https://github.com/apache/avro/pull/1584#discussion_r824956689
##########
File path:
lang/java/ipc/src/test/java/org/apache/avro/message/TestBigDecimalEvolution.java
##########
@@ -0,0 +1,141 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to you under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * <p>https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * <p>Unless required by applicable law or agreed to in writing, software
distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.message;
+
+import org.apache.avro.Protocol;
+import org.apache.avro.Schema;
+import org.apache.avro.io.BinaryEncoder;
+import org.apache.avro.io.DecoderFactory;
+import org.apache.avro.io.EncoderFactory;
+import org.apache.avro.ipc.Transceiver;
+import org.apache.avro.ipc.reflect.ReflectRequestor;
+import org.apache.avro.ipc.reflect.ReflectResponder;
+
+import org.apache.avro.message.protocol.BDRequest;
+import org.apache.avro.message.protocol.BDResponse;
+import org.apache.avro.message.protocol.BigDecimalConversionUpgrade;
+import org.apache.avro.message.protocol.BigDecimalConversionOld;
+import org.apache.avro.message.protocol.BigDecimalProtocol;
+import org.apache.avro.reflect.ReflectData;
+import org.junit.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestBigDecimalEvolution {
+
+ public static final double DELTA = 0.00000001;
+ private final MockTransceiver transceiver = new MockTransceiver();
+
+ @Test
+ public void bigDecimalEvolutionThroughLogicalType() throws Exception {
+ ReflectData oldApp = new ReflectData();
+ oldApp.addLogicalTypeConversion(new BigDecimalConversionOld());
+ ReflectData upgradeApp = new ReflectData();
+ upgradeApp.addLogicalTypeConversion(new BigDecimalConversionUpgrade());
+
+ verify(oldApp, oldApp);
+ verify(upgradeApp, upgradeApp);
+ verify(upgradeApp, oldApp);
+ verify(oldApp, upgradeApp);
+ }
+
+ private void verify(ReflectData clientData, ReflectData serverData) throws
IOException {
+ Protocol clientProtocol = serverData.getProtocol(BigDecimalProtocol.class);
+ ReflectRequestor clientRequestor = new ReflectRequestor(clientProtocol,
transceiver, serverData);
+ ReflectResponder clientResponder = new ReflectResponder(clientProtocol,
null, serverData);
+ Schema clientRequestSchema = serverData.getSchema(BDRequest.class);
+ Schema clientResponseSchema = serverData.getSchema(BDResponse.class);
+
+ Protocol serverProtocol = clientData.getProtocol(BigDecimalProtocol.class);
+ ReflectRequestor serverRequestor = new ReflectRequestor(serverProtocol,
transceiver, clientData);
+ ReflectResponder serverResponder = new ReflectResponder(serverProtocol,
null, clientData);
+ Schema serverRequestSchema = clientData.getSchema(BDRequest.class);
+ Schema serverResponseSchema = clientData.getSchema(BDResponse.class);
+
+ BDRequest originalRequest = new BDRequest();
+ originalRequest.setValue(new BigDecimal("123.321"));
+
+ BDRequest request = verifyRequest(originalRequest, serverRequestSchema,
clientRequestSchema, serverProtocol,
+ serverRequestor, clientResponder);
+
+ BDResponse originalResponse = new BDResponse(request.getValue());
+ verifyResponse(originalResponse, serverResponseSchema,
clientResponseSchema, serverResponder, clientRequestor);
+ }
+
+ private BDRequest verifyRequest(BDRequest originalRequest, Schema
writerSchema, Schema readerSchema,
+ Protocol writerProtocol, ReflectRequestor requestor, ReflectResponder
responder) throws IOException {
+
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(out, null);
+
+
requestor.writeRequest(writerProtocol.getMessages().get("test").getRequest(),
new Object[] { originalRequest },
+ encoder);
+
+ encoder.flush();
+
+ Object obj = responder.readRequest(writerSchema, readerSchema,
+ DecoderFactory.get().binaryDecoder(out.toByteArray(), null));
+ assertTrue(obj instanceof BDRequest);
+ BDRequest request = (BDRequest) obj;
+
+ assertEquals(originalRequest.getValue().doubleValue(),
request.getValue().doubleValue(), DELTA);
Review comment:
Actually, to test the logical type being read from the binary Avro
format you _also_ need to test the scale. The BigDecimal equality check helps
here.
For user input (not here), you can use BigDecimal.compareTo() == 0, as that
method ignores scale.
--
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-unsubscr...@avro.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 740194)
Time Spent: 2h (was: 1h 50m)
> Schema evolution with logical types
> ------------------------------------
>
> Key: AVRO-3408
> URL: https://issues.apache.org/jira/browse/AVRO-3408
> Project: Apache Avro
> Issue Type: Improvement
> Components: java
> Affects Versions: 1.11.0
> Reporter: Ivan Zemlyanskiy
> Priority: Major
> Labels: pull-request-available
> Time Spent: 2h
> Remaining Estimate: 0h
>
> Hello!
> First of all, thank you for this project. I love Avro encoding from both
> technology and code culture points of view. (y)
> I know you recommend migrating schema by adding a new field and removing the
> old one in the future, but please-please-please consider my case as well.
> In my company, we have some DTOs, and it's about 200+ fields in total that we
> encode with Avro and send over the network. About a third of them have type
> `java.math.BigDecimal`. At some point, we discovered we send them with a
> schema like
> {code:json}
> {
> "name":"performancePrice",
> "type":{
> "type":"string",
> "java-class":"java.math.BigDecimal"
> }
> }
> {code}
> That's a kind of disaster for us cos we have pretty much a high load with ~2
> million RPS.
> So we start to think about migrating to something lighter than strings (no
> blame for choosing it as a default, I know BigDecimal has a lot of pitfalls,
> and string is the easiest way for encoding/decoding).
> It was fine to make a standard precision for all such fields, so we found
> `Conversions.DecimalConversion` and decided at the end of the day we were
> going to use this logical type with a recommended schema like
> {code:java}
> @Override
> public Schema getRecommendedSchema() {
> Schema schema = Schema.create(Schema.Type.BYTES);
> LogicalTypes.Decimal decimalType =
> LogicalTypes.decimal(MathContext.DECIMAL32.getPrecision(),
> DecimalUtils.MONEY_ROUNDING_SCALE);
> decimalType.addToSchema(schema);
> return schema;
> }
> {code}
> (we use `org.apache.avro.reflect.ReflectData`)
> It all looks good and promising, but the question is how to migrate to such
> schema?
> As I said, we have a lot of such fields, and migrating all of them with
> duplication fields with future removal might be painful and would cost us a
> considerable overhead.
> I made some tests and found out if two applications register the same
> `BigDecimalConversion` but for one application the `getRecommendedSchema()`
> is like the method above and for another application the
> `getRecommendedSchema()` is
> {code:java}
> @Override
> public Schema getRecommendedSchema() {
> Schema schema = Schema.create(Schema.Type.STRING);
> schema.addProp(SpecificData.CLASS_PROP, BigDecimal.class.getName());
> return schema;
> }
> {code}
> so they can easily read each other messages using _SERVER_ schema.
> So, I made two applications and wired them up with `ProtocolRepository`,
> `ReflectResponder` and all that stuff, I found out it doesn't work. Because
> `org.apache.avro.io.ResolvingDecoder` totally ignores logical types for some
> reason.
> So as a result, one application specifically told "I encode this field as a
> byte array which supposed to be a logical type 'decimal' with precision N",
> but another application just tries to convert those bytes to a string and
> make a BigDecimal based on the result string. As a result, we got
> {code:java}
> java.lang.NumberFormatException: Character ' is neither a decimal digit
> number, decimal point, nor "e" notation exponential mark.
> {code}
> In my humble opinion, `org.apache.avro.io.ResolvingDecoder` should respect
> logical types in _SERVER_ (_ACTUAL_) schema and use a corresponding
> conversion instance for reading values. In my example, I'd say it might be
> {code}
> ResolvingDecoder#readString() -> read the actual logical type -> find
> BigDecimalConversion instance ->
> conversion.fromBytes(readValueWithActualSchema()) ->
> conversion.toCharSequence(readValueWithConversion)
> {code}
> I'd love to read your opinion on all of that.
> Thank you in advance for your time, and sorry for the long issue description.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
