influxdata / influxdb-java

Java client for InfluxDB
MIT License
1.18k stars 476 forks source link

Incorrect long values returned due to json to java conversion #276

Open gmegan opened 7 years ago

gmegan commented 7 years ago

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 commented 7 years ago

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.

Aleishus commented 7 years ago

I met the same problems

majst01 commented 7 years ago

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

Aleishus commented 7 years ago

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 (。•ˇ‸ˇ•。)
    }
...
majst01 commented 7 years ago

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

gmegan commented 7 years ago

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

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

majst01 commented 7 years ago

Any idea howto solve this ?

gmegan commented 7 years ago

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.

majst01 commented 7 years ago

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

majst01 commented 7 years ago

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 commented 7 years ago

Which is also tested:

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

echernyshev commented 7 years ago

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();
...
kkarski commented 7 years ago

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.

simon04 commented 7 years ago

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

majst01 commented 7 years ago

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

simon04 commented 7 years ago

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 commented 7 years ago

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.

larrywalker commented 6 years ago

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

Drimix20 commented 3 years ago

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.

Tancen commented 2 years ago

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);
...
majst01 commented 2 years ago

Would you please try with #837

majst01 commented 2 years ago

@Tancen any chance to try #837 ?

Tancen commented 2 years ago

@Tancen any chance to try #837 ?

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