influxdb-java icon indicating copy to clipboard operation
influxdb-java copied to clipboard

Incorrect long values returned due to json to java conversion

Open gmegan opened this issue 8 years ago • 23 comments

I am having a problem with bad long values being returned due to the conversion to double that happens when all JSON numeric values are stored as Doubles.

This came up first when trying to get back time values as nanoseconds instead of rfc3339 string values. But we need to store very large long values in other fields as well. The application here is scientific instruments, so numerical errors are pretty high priority problems when we are looking at data storage.

From looking around, it does not look like there is a good way to intercept the JSON and convert to Long instead of Double... but maybe I am missing something? I would like to find some way to get these values returned as Long instead of Double. Maybe a different converter? I would appreciate any suggestions or workarounds.

Here is a snippet that recreates the problem:

        long lval = 1485370052974000001L;

        BatchPoints batchPoints = BatchPoints
                .database(dbName)
                .tag("async", "true")
                .retentionPolicy("autogen")
                .consistency(ConsistencyLevel.ALL)
                .build();
        Point point = Point.measurement("testProblems")
                .time(innano, TimeUnit.NANOSECONDS)
                .addField("double", 3.14)
                .addField("long", lval)
                .build();

        batchPoints.point(point);
        influxDB.write(batchPoints);

        Series series0 = result.getResults().get(0).getSeries().get(0);
        List<String> cols = series0.getColumns();
        List<Object> val0 = series0.getValues().get(0);

        Object outlval_obj = val0.get(cols.indexOf("long"));
        if (outlval_obj instanceof Double)
        {
            long outlval = ((Double)outlval_obj).longValue();
            if (outlval != lval)
            {
                System.err.println("Got bad lval back as double [" + (outlval_obj) + "] -> " + outlval + " != " + lval);
            }
        }

Here is a gist with more code and outputs: https://gist.github.com/ghmegan/4ed652c8c8dbaf18976c6f5f4c0c6b55

gmegan avatar Jan 27 '17 16:01 gmegan

As a note, when I use the influx client and query the database directly, the long field shows the correct value. So it is stored correctly. The JSON contains the correct value in response to a curl query. This is why I am thinking it is the JSON to java conversion and not some other step in the process.

gmegan avatar Jan 27 '17 16:01 gmegan

I met the same problems

Aleishus avatar Feb 07 '17 08:02 Aleishus

Ok, please write a unit-test which shows the problem and raise a PR therefore, so anybody can see what exact problem you face.

Unit Test go here: https://github.com/influxdata/influxdb-java/blob/master/src/test/java/org/influxdb/TicketTests.java

majst01 avatar Feb 07 '17 08:02 majst01

I thought the json library moshi cause this problem ,just look at the method

com.squareup.moshi.JsonUtf8Reader#nextDouble

 int p = peeked;
    if (p == PEEKED_NONE) {
      p = doPeek();
    }

    if (p == PEEKED_LONG) {
      peeked = PEEKED_NONE;
      pathIndices[stackSize - 1]++;
      return (double) peekedLong;  //  force convert (。•ˇ‸ˇ•。)
    }
...

Aleishus avatar Feb 07 '17 09:02 Aleishus

But just looking at the code gives an idea, showing the error in a unit-test is believing !

majst01 avatar Feb 07 '17 11:02 majst01

I added the unit tests and made a pull request from my fork of this library.

https://github.com/influxdata/influxdb-java/pull/283

gmegan avatar Feb 08 '17 17:02 gmegan

Any idea howto solve this ?

majst01 avatar Mar 12 '17 09:03 majst01

I am either going to have to try to switch the converter, since this is a moshi problem... or, if that doesn't work, write my own query functions and translate the json by hand. I will have to find some fix, since we are working with large numerical data, so this problem is a deal breaker.

I have just been working along on other things, waiting to see if there are other people with similar problem who might have other suggestions... But at this point, I will probably need to be doing a fix in my fork in the next couple of weeks since I will need to start pushing our product out for testing.

gmegan avatar Mar 12 '17 17:03 gmegan

Maybe we should link this issue to a new moshi issue, the square people are very helpful from my experience.

majst01 avatar Mar 13 '17 06:03 majst01

But reading the moshi code gives me:

https://github.com/square/moshi/blob/master/moshi/src/main/java/com/squareup/moshi/JsonReader.java


  /**
   * Returns the {@linkplain Token#NUMBER long} value of the next token, consuming it. If the next
   * token is a string, this method will attempt to parse it as a long. If the next token's numeric
   * value cannot be exactly represented by a Java {@code long}, this method throws.
   *
   * @throws JsonDataException if the next token is not a literal value, if the next literal value
   *     cannot be parsed as a number, or exactly represented as a long.
   */
  public abstract long nextLong() throws IOException;

So in principal, moshi is able to deserialize Long values. Probably the problem is somewhere up the stack.

majst01 avatar Mar 13 '17 06:03 majst01

Which is also tested:

https://github.com/square/moshi/blob/master/moshi/src/test/java/com/squareup/moshi/JsonReaderTest.java#L358

majst01 avatar Mar 13 '17 06:03 majst01

Hi! Problem in org.influxdb.dto.QueryResult.Series class.

...
 public static class Series {
...
    private List<List<Object>> values;
...

A simple test reproducing an error:

public class MoshiTest{
    public static class Bean{
        private List<List<Object>> list;
        private List<Object> objectList;
        public List<List<Object>> getList(){
            return list;
        }

        public List<Object> getObjectList(){
            return objectList;
        }

        public void setList(List<List<Object>> list){
            this.list = list;
        }

        public void setObjectList(List<Object> objectList){
            this.objectList = objectList;
        }
    }

    @Test
    public void test() throws IOException{
        String json = "{\"list\":[[22, 11000]],\"objectList\":[22, 11]}";

        Moshi moshi = new Moshi.Builder().build();
        JsonAdapter<Bean> jsonAdapter = moshi.adapter(Bean.class);

        Bean bean = jsonAdapter.fromJson(json);
        Assert.assertEquals(bean.getList().get(0).get(0).getClass(), Double.class);
        Assert.assertEquals(bean.getList().get(0).get(1).getClass(), Double.class);
        Assert.assertEquals(bean.getObjectList().get(0).getClass(), Double.class);
        Assert.assertEquals(bean.getObjectList().get(1).getClass(), Double.class);
    }
}

The method com.squareup.moshi.StandardJsonAdapters.ObjectJsonAdapter.fromJson(JsonReader) always calls the function com.squareup.moshi.JsonReader.nextDouble():

...
    case NUMBER:
          return reader.nextDouble();
...

echernyshev avatar Apr 11 '17 14:04 echernyshev

Having the same issue. Was considering storing the datatypes along with the values but essentially came to the same conclusion that this is a client limitation, not DB.

kkarski avatar May 08 '17 23:05 kkarski

I dug into the moshi sources and submitted a PR against moshi. Let's see.

simon04 avatar May 25 '17 21:05 simon04

@simon04 Thanks for that, i was always pretty sure its moshi´s fault.

majst01 avatar May 26 '17 05:05 majst01

The JSON standard is pretty lose on the precision on numbers. It specifically notes that everything outside IEEE 754 double precision might cause interoperability problems.

simon04 avatar May 26 '17 07:05 simon04

The Moshi PR has been closed as wont-fix (to not alter the "Double is default type for numbers" behaviour).

To address this on the influxdb-java side, @swankjesse provided a CustomNumericTypes JSON adapter which needs https://github.com/square/moshi/commit/aee3216ca1d17971457e63495e44a56694053099.

simon04 avatar Jun 13 '17 07:06 simon04

We are seeing this also. It is correct in the db when viewed via the cli(influx client) or when retrieved directly via http, but loses precision when read via influxdb-java.

Value in when seen in DB = 1504718625918164992 Value from influxdb-java = 1.50471862591816499E18 when put to back to a long = 1504718625918164990

larrywalker avatar Sep 06 '17 18:09 larrywalker

It looks like the issue in the moshi is already resolved https://github.com/square/moshi/pull/314. Long type is already handled in StandardJsonAdapters. For more details see https://github.com/square/moshi/blob/9ac54dd33faa6d4865dfc6d807cf20daa78b27a9/moshi/src/main/java/com/squareup/moshi/StandardJsonAdapters.java#L54.

Drimix20 avatar Mar 05 '21 14:03 Drimix20

It looks like the issue in the moshi is already resolved square/moshi#314. Long type is already handled in StandardJsonAdapters. For more details see https://github.com/square/moshi/blob/9ac54dd33faa6d4865dfc6d807cf20daa78b27a9/moshi/src/main/java/com/squareup/moshi/StandardJsonAdapters.java#L54.

In Influxdb-Java 2.2.2 (dependent on Moshi 1.8.0), The bug hasn't been fixed yet. I substituted Moshi with Fastjson by reflection. Now, it works well.

    static private boolean replaceObjectMember(Object obj, Class<?> srcClass, String memberName, Object newMember)
    {
        try
        {
            Field field = srcClass.getDeclaredField(memberName);
            field.setAccessible(true);
            field.set(obj, newMember);
        }catch (Exception e)
        {
            GlobalVariable.gLogger.error("InfluxDB - replaceObjectMember failed: ", e);
            return false;
        }
        return true;
    }

    static private boolean replaceObjectMember(Object obj, String className, String memberName, Object newMember)
    {
        try
        {
            return replaceObjectMember(obj, Class.forName(className), memberName, newMember);
        }catch (Exception e)
        {
            GlobalVariable.gLogger.error("InfluxDB - replaceObjectMember failed: ", e);
            return false;
        }
    }

    static private Object getObjectMember(Object obj, Class<?> srcClass, String memberName)
    {
        try
        {
            Field field = srcClass.getDeclaredField(memberName);
            field.setAccessible(true);
            return field.get(obj);
        }catch (Exception e)
        {
            GlobalVariable.gLogger.error("InfluxDB - getObjectMember failed: ", e);
            return null;
        }
    }

    static private Object getObjectMember(Object obj, String className, String memberName)
    {
        try
        {
            return getObjectMember(obj, Class.forName(className), memberName);
        }catch (Exception e)
        {
            GlobalVariable.gLogger.error("InfluxDB - getObjectMember failed: ", e);
            return null;
        }
    }

    static private Object constructObject(String className, Class<?>[] argsClass,  Object[] args)
    {
        try
        {
            Class<?> aClass = Class.forName(className);
            Constructor<?> constructor;
            if (argsClass.length > 0)
                constructor = aClass.getDeclaredConstructor(argsClass);
            else
                constructor = aClass.getDeclaredConstructor();
            constructor.setAccessible(true);
            Object ret;
            if (argsClass.length > 0)
                ret = constructor.newInstance(args);
            else
                ret = constructor.newInstance();
            return ret;
        }catch (Exception e)
        {
            GlobalVariable.gLogger.error("InfluxDB - constructObject failed: ", e);
            return null;
        }
    }

    static class MyJsonAdapter<T> extends JsonAdapter<T>
    {
        JsonAdapter<T> proxy;

        public MyJsonAdapter(JsonAdapter<T> proxy)
        {
            this.proxy = proxy;
        }

        @Nullable
        @Override
        public T fromJson(JsonReader jsonReader) throws IOException
        {
            BufferedSource source = (BufferedSource)getObjectMember(jsonReader, "com.squareup.moshi.JsonUtf8Reader", "source");
            if (source == null)
                return null;

            ParameterizedType pt = (ParameterizedType) this.getClass().getGenericSuperclass();
            Class<T> clazz = (Class<T>) pt.getActualTypeArguments()[0];

            String s = source.readString(StandardCharsets.UTF_8);
            T ret = JSON.parseObject(s, clazz);
            return ret;
        }

        @Override
        public void toJson(JsonWriter jsonWriter, @Nullable T o) throws IOException
        {
            proxy.toJson(jsonWriter, o);
        }
    }

    private static boolean hackInfluxDBClient(org.influxdb.InfluxDB client)
    {
        Moshi moshi = new Moshi.Builder().build();
        JsonAdapter<QueryResult> adapter = new MyJsonAdapter<>(moshi.adapter(QueryResult.class));
        Class<?>[] argsClass = new Class[]{InfluxDBImpl.class, JsonAdapter.class};
        Object[] args = new Object[]{client, adapter};
        Object chunkProccesor = constructObject("org.influxdb.impl.InfluxDBImpl$JSONChunkProccesor", argsClass, args);
        if (chunkProccesor == null)
            return false;

        if (!replaceObjectMember(client, InfluxDBImpl.class, "chunkProccesor", chunkProccesor))
            return false;

        Object retrofit = getObjectMember(client, InfluxDBImpl.class, "retrofit");
        if (retrofit == null)
            return false;
        List<Converter.Factory> converterFactories = new ArrayList<>();
        converterFactories.add(new Retrofit2ConverterFactory());
        if(!replaceObjectMember(retrofit, "retrofit2.Retrofit", "converterFactories", converterFactories))
            return false;
        return true;
    }

...
org.influxdb.InfluxDB client = InfluxDBFactory.connect(url, username, password);
hackInfluxDBClient(client);
...

Tancen avatar Jun 01 '22 01:06 Tancen

Would you please try with #837

majst01 avatar Jun 01 '22 05:06 majst01

@Tancen any chance to try #837 ?

majst01 avatar Jun 07 '22 07:06 majst01

@Tancen any chance to try #837 ?

Sorry, We haven't more free time to try it now.

Tancen avatar Jun 08 '22 00:06 Tancen