Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gmegan opened this issue Jan 27, 2017 · 23 comments
Open

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

gmegan opened this issue Jan 27, 2017 · 23 comments

Comments

@gmegan
Copy link

gmegan commented Jan 27, 2017

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
Copy link
Author

gmegan commented Jan 27, 2017

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
Copy link

Aleishus commented Feb 7, 2017

I met the same problems

@majst01
Copy link
Collaborator

majst01 commented Feb 7, 2017

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
Copy link

Aleishus commented Feb 7, 2017

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
Copy link
Collaborator

majst01 commented Feb 7, 2017

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

@gmegan
Copy link
Author

gmegan commented Feb 8, 2017

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

#283

@majst01
Copy link
Collaborator

majst01 commented Mar 12, 2017

Any idea howto solve this ?

@gmegan
Copy link
Author

gmegan commented Mar 12, 2017

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
Copy link
Collaborator

majst01 commented Mar 13, 2017

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

@majst01
Copy link
Collaborator

majst01 commented Mar 13, 2017

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
Copy link
Collaborator

majst01 commented Mar 13, 2017

@echernyshev
Copy link

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
Copy link

kkarski commented May 8, 2017

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
Copy link
Contributor

simon04 commented May 25, 2017

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

@majst01
Copy link
Collaborator

majst01 commented May 26, 2017

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

@simon04
Copy link
Contributor

simon04 commented May 26, 2017

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
Copy link
Contributor

simon04 commented Jun 13, 2017

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 square/moshi@aee3216.

@larrywalker
Copy link

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

@fmachado fmachado added this to the 3.x milestone Jan 12, 2020
@Drimix20
Copy link

Drimix20 commented Mar 5, 2021

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.

@Tancen
Copy link

Tancen commented Jun 1, 2022

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
Copy link
Collaborator

majst01 commented Jun 1, 2022

Would you please try with #837

@majst01
Copy link
Collaborator

majst01 commented Jun 7, 2022

@Tancen any chance to try #837 ?

@Tancen
Copy link

Tancen commented Jun 8, 2022

@Tancen any chance to try #837 ?

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants