Skip to content

C# to Java type mapping #1643

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

Closed
niemyjski opened this issue Dec 1, 2015 · 5 comments
Closed

C# to Java type mapping #1643

niemyjski opened this issue Dec 1, 2015 · 5 comments

Comments

@niemyjski
Copy link
Contributor

I have a dictionary that I do dynamic mapping on in our app. I append a type suffix to the field name so I can share mappings (multi tenant). Currently I'm only handling decimal and double mappings in .net and this leads to some insert errors when I detect it incorrectly (obviously a bug on our part).. I think the solutions for us going forward is to have a typed field mapping based on the suffix that maps to the largest numeric type (sucks because you use more memory) or parse out the specific type (and use more processing time).

I wanted to look through your code to see if I could find a method where I could pass a type/instance or string (do a parse of the type for me) and return the name it would be mapped to (E.G., number). I came across this and this in master. To my knowledge, correct me if I'm wrong.. seem to incorrectly handle some data types incorrectly or not at all. For example it doesn't seem like some data types are handled like DateTimeOffset and possibly some numeric types. Secondly the java types as defined here don't match up with your .NET data type mappings. Example:

Java byte is -128 to 127 which is the same as a .NET SByte. In the code I saw you had it mapped to a .NET Byte as well which is 0-255. Which I believe the methods would return the incorrect mapping if the value is >127.

Would it be possible to expose some helper methods that make it easier to get the mapped types as well as update these mappings?

@russcam
Copy link
Contributor

russcam commented Dec 24, 2015

@niemyjski thanks for reporting this. I'm looking at this in the 2.0 branch, with the following C# to Elasticsearch data type mappings:

.NET Type Elasticsearch Type
System.Byte short
System.Int16 short
System.Int32 integer
System.Int64 long
System.Single float
System.Double double
System.Decimal double
System.SByte byte
System.UInt16 integer
System.UInt32 long
System.UInt64 double
System.Boolean boolean
System.Char string
System.String string
System.Guid string
System.DateTime date
System.DateTimeOffset date

/cc @gmarz @Mpdreamz, I think we should consider also handling System.TimeSpan by default, storing it as a double representing the TotalMilliseconds of the TimeSpan. Storing it as a string makes less sense to me as you may want to query on the field and this would be unituitive on a string field.

Within the 2.0 branch, you can use the PropertyWalker to get the mapped properties for a given type.

russcam added a commit that referenced this issue Dec 24, 2015
This fixes discrepancies between how C# types compare to Java types as documented in https://msdn.microsoft.com/en-us/library/ms228360(v=vs.90).aspx and https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html. Fixes issue #1643
Add additional unit tests for inferred types
russcam added a commit that referenced this issue Dec 24, 2015
…alker

This fixes discrepancies between how C# types compare to Java types as documented in https://msdn.microsoft.com/en-us/library/ms228360(v=vs.90).aspx and https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html. Fixes issue #1643
Add additional unit tests for inferred types
@Mpdreamz
Copy link
Member

Mpdreamz commented Jan 6, 2016

Closing this as we are doing the correct thing in 2.0 (unless i'm going crosseyed @niemyjski @russcam please reopen!)

@Mpdreamz Mpdreamz closed this as completed Jan 6, 2016
@russcam
Copy link
Contributor

russcam commented Jan 6, 2016

@Mpdreamz We adhere to the above mappings in 2.0 (and also have been backported to the next release of 1.x).

The only remaining consideration is handling TimeSpan by default - this would require a custom JsonConverter to handle serialization/deserialization to and from a double. I'll open as a separate issue for consideration.

@niemyjski
Copy link
Contributor Author

@russcam was this back ported to 1.8?

@russcam
Copy link
Contributor

russcam commented May 2, 2016

@niemyjski yes, it went into 1.7.2 in aa8c993

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

No branches or pull requests

3 participants