Dgraph: JSON String mutations are now type inferred

Created on 8 Nov 2018  路  8Comments  路  Source: dgraph-io/dgraph

  • What version of Dgraph are you using?
    1.0.10

  • Have you tried reproducing the issue with latest release?
    was not present before 1.0.10

  • What is the hardware spec (RAM, OS)?
    macbook pro 2016

  • Expected behaviour and actual result.
    When saving a predicate with JSON, the JSON type should be respected.

In #2627 , this code block is added
https://github.com/dgraph-io/dgraph/pull/2627/files#diff-5c560e7723a3bc51e26d863bfa4257bdR136

It _seems_ that this alters the default behavior for parsing json string predicates. In the past strings would be stored as string type predicates, and now it seems that some type guessing is applied on the first mutation.

The comment in that block seem to indicate that this is _not_ the expected behavior for JSON strings.

Most helpful comment

Thanks for reporting this and explaining your point. And to be clear I was not being disrespectful, I was merely explaining how the behavior changed. This was not an oversight, it was a feature we thought it would help our users, hence why it was being tested. But you make valid points and I will err on the side of caution in case more users don't use schema and need this behavior.

All 8 comments

Please can you let the issue more clear? or move this to discuss?

  • Expected behaviour and actual result.

Cheers.

Yes, the JSON behavior to save as string has changed. If you want to disable the type-hinting, you must define a schema. That's what the comment is describing.

// Try to guess a storage type. The schema type is preferred though.

Using a schema is the best practice to have predictable performance.

@srfrog This doesn't make sense for JSON. If I put a json int or float, I would expect the schema to be inferred as a int or float. But if I put a string, I expect a string.

Requiring a schema to take a string as a string seems like a regression to me.

Requiring a schema to take a string as a string seems like a regression to me.

JSON numberic values work as expected. But unfortunately, JSON has no distinction about numbers as string, where "1" == 1. The old behavior would have stored this as string, as you noted, but is this correct only 50% of the time. JSON's weak types handling created complexity in Dgraph so we needed to add precedence. The change made it possible to have a single type precedence for JSON and RDF. But now you must use a schema if you want 100% control of the values. We are slowly working toward a type system and this is a first step.

Btw, you can find the change details here: https://discuss.dgraph.io/t/testing-mutation-type-changes/3352

Let's move the discussion there, as this is not an issue atm.

@manishrjain This is a breaking change for us, and I'm pretty disappointed in how such an oversight for JSON mutations could be introduced without even being mentioned in the release notes (or docs) :\

@srfrog While "1" == 1 is true in javascript, "1" === 1 is false, which is how you should check equality with types considered.

It seems to me that if you WANT a type conversion from string to something else THAT would be a case where defining a schema makes sense. Defining a schema to keep a string a string is backwards.

Lets say you have a field zip_code (just one of our cases where things break). We take care to make sure all zip codes are string types, but now if the first zip code is a US zip code, the schema is broken. Our application has an evolving schema, and this now breaks the ability to evolve the schema with user added fields.

I apologize for my curt tone, but it is incredibly frustrating to have such a drastic change added to a major release without mention, and such a flippant response as "just add a schema."

At the very least it seems like this should be a configurable option.

Thanks for reporting this and explaining your point. And to be clear I was not being disrespectful, I was merely explaining how the behavior changed. This was not an oversight, it was a feature we thought it would help our users, hence why it was being tested. But you make valid points and I will err on the side of caution in case more users don't use schema and need this behavior.

@srfrog Just wanted to say I appreciate you guys taking this concern seriously and the quick remediation!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jerodsanto picture jerodsanto  路  3Comments

ShawnMilo picture ShawnMilo  路  4Comments

captain-me0w picture captain-me0w  路  4Comments

fritzblue picture fritzblue  路  5Comments

mbudge picture mbudge  路  3Comments