keith-turner commented on a change in pull request #2535: URL: https://github.com/apache/accumulo/pull/2535#discussion_r821018344
########## File path: core/src/main/java/org/apache/accumulo/core/client/lexicoder/BigDecimalLexicoder.java ########## @@ -0,0 +1,88 @@ +/* + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.accumulo.core.client.lexicoder; + +import java.io.ByteArrayInputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.math.BigDecimal; +import java.math.BigInteger; + +import org.apache.accumulo.core.clientImpl.lexicoder.FixedByteArrayOutputStream; + +/** + * A lexicoder to encode/decode a BigDecimal to/from bytes that maintain its native Java sort order. + * + * @since 2.1.0 + */ +public class BigDecimalLexicoder extends AbstractLexicoder<BigDecimal> { + + private final BigIntegerLexicoder bigIntegerLexicoder = new BigIntegerLexicoder(); + + @Override + public BigDecimal decode(byte[] b) { + // This concrete implementation is provided for binary compatibility, since the corresponding + // superclass method has type-erased return type Object. See ACCUMULO-3789 and #1285. + return super.decode(b); + } + + @Override + public byte[] encode(BigDecimal bd) { + // To encode we separate out the scale and the unscaled value + // serialize each value individually and store them + int scale = bd.scale(); + BigInteger bigInt = bd.unscaledValue(); + byte[] encodedbigInt = bigIntegerLexicoder.encode(bigInt); + // Length is set to size of encoded BigInteger + length of the scale value + byte[] ret = new byte[4 + encodedbigInt.length]; + try (DataOutputStream dos = new DataOutputStream(new FixedByteArrayOutputStream(ret))) { + scale = scale ^ 0x80000000; + dos.write(encodedbigInt); + dos.writeInt(scale); Review comment: I tried to rewrite this locally to use an exponent and it mostly works. Below is what I was experimenting with. ```java public class BigDecimalLexicoder extends AbstractLexicoder<BigDecimal> { private final BigIntegerLexicoder bigIntegerLexicoder = new BigIntegerLexicoder(); @Override public BigDecimal decode(byte[] b) { // This concrete implementation is provided for binary compatibility, since the corresponding // superclass method has type-erased return type Object. See ACCUMULO-3789 and #1285. return super.decode(b); } private static int getExponent(BigDecimal bd){ return -1*bd.scale()+bd.precision()-1; } /** * inverse of {@link #getExponent(BigDecimal)} */ private static int getScale(int exponent, int precision) { return (exponent+1-precision) * -1; } private byte[] encodeBigInt(BigInteger bigInt) { byte[] bytes = bigInt.toByteArray(); // flip the sign bit bytes[0] = (byte) (0x80 ^ bytes[0]); return bytes; } private BigInteger decodeBigInt(byte[] data, int off, int len) { // unflip the sign bit data[off] = (byte) (0x80 ^ data[off]); return new BigInteger(data, off, len); } @Override public byte[] encode(BigDecimal bd) { BigInteger bigInt = bd.unscaledValue(); byte[] encodedbigInt = encodeBigInt(bigInt); // Length is set to size of encoded BigInteger + length of the scale value byte[] ret = new byte[5 + encodedbigInt.length]; try (DataOutputStream dos = new DataOutputStream(new FixedByteArrayOutputStream(ret))) { // sort first on the sign, make negative numbers come first dos.write((byte)bd.signum() ^ 0x80); // sort second on the exponent int exponent = getExponent(bd); if(bd.signum() < 0) { // when the number if negative the exponents need to sort in reverse order exponent = exponent * -1; } dos.writeInt(exponent ^ 0x80000000); // sort third on the unscaled value. dos.write(encodedbigInt); return ret; } catch (IOException e) { throw new UncheckedIOException(e); } } @Override protected BigDecimal decodeUnchecked(byte[] b, int offset, int origLen) throws IllegalArgumentException { try { DataInputStream dis = new DataInputStream(new ByteArrayInputStream(b, offset, origLen)); int signum = (byte)(dis.read() ^ 0x80); int exponent = dis.readInt() ^ 0x80000000; if(signum < 0) exponent = exponent * -1; BigInteger bigInt = decodeBigInt(b,5,b.length - 5); // TODO is there a better way to get this int precision = bigInt.abs().toString().length(); return new BigDecimal(bigInt, getScale(exponent, precision)); } catch (IOException ioe) { throw new UncheckedIOException(ioe); } } } ``` I modified the test function to use the following test data. ```java @Test public void testSortOrder() { var list = Stream.of("4.2E+10","4.2E+100","4.1E+100","4.3E+100","4.2E-10","4.2E-100", "-4.2E+10","-4.2E+100","-4.2E-10","-4.2E-100","-4.1E-100","-4.3E-100","4.2","-4.2","2.1","2.2", "2.0", "2.00", "2.000", "-3.000", "-2.00", "0.0000", "0.1", "0.10", "-65537.000", "-65537.123", "-65537.00", "-65537.0", "4.1E-10","4.3E-10").map(BigDecimal::new).collect(Collectors.toList()); assertSortOrder(new BigDecimalLexicoder(), list); } ``` And it fails with the following difference in sort order. ``` Expected :[-4.2E+100, -4.2E+10, -65537.123, -65537.000, -65537.00, -65537.0, -4.2, -3.000, -2.00, -4.2E-10, -4.3E-100, -4.2E-100, -4.1E-100, 0.0000, 4.2E-100, 4.1E-10, 4.2E-10, 4.3E-10, 0.1, 0.10, 2.0, 2.00, 2.000, 2.1, 2.2, 4.2, 4.2E+10, 4.1E+100, 4.2E+100, 4.3E+100] Actual :[-4.2E+100, -4.2E+10, -65537.00, -65537.0, -65537.123, -65537.000, -4.2, -3.000, -2.00, -4.2E-10, -4.3E-100, -4.2E-100, -4.1E-100, 0.0000, 4.2E-100, 4.1E-10, 4.2E-10, 4.3E-10, 0.1, 0.10, 2.00, 2.000, 2.0, 2.1, 2.2, 4.2, 4.2E+10, 4.1E+100, 4.2E+100, 4.3E+100] ``` So everything in the test data is sorting fine. The only problem is `-65537.123` is sorting after `-65537.0`. I understand why its doing this, not sure what to do about it for now though. -- 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]
