davebshow / goblin

A Python 3.5 rewrite of the TinkerPop 3 OGM Goblin
Other
93 stars 21 forks source link

64-bit Integer Properties #63

Closed rosenbrockc closed 7 years ago

rosenbrockc commented 7 years ago

goblin.Integer uses the built-in python int for validating whether a given value is integer (https://github.com/davebshow/goblin/blob/master/goblin/properties.py#L175). However, this causes problems with 64-bit integers. For example, list and set cardinality creates *VertexPropertyManager instances that call validate on each argument they are given.

In order to support correct GraphSON serialization to Int64, gremlin_python creates a long type in gremlin_python.statics. Developers should cast integers to that sub-class of int if they want them to be serialized correctly to the database. Sets of 64-bit integers, however, get cast back to the 32-bit int because of the validator.

I propose replacing the existing validate with:

                if not isinstance(val, long):
                    return int(val)
                else:
                    return val

where long is from gremlin_python.statics import long. Happy to pull request for this again.

davebshow commented 7 years ago

Good catch here...we still need to validate in the case of a long type:

if isinstance(val, long):
    return long(val)
return int(val)

I'd be happy to accept a PR.

rosenbrockc commented 7 years ago

To master or property_datatype_refactor?

davebshow commented 7 years ago

direct to the refactor branch.