ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vyacheslav Daradur <daradu...@gmail.com>
Subject Re: IGNITE-3196 - ready for review
Date Mon, 06 Feb 2017 08:21:32 GMT
Pavel, Igor

Please, review it again.

https://github.com/apache/ignite/pull/1473/files

All tests
<http://ci.ignite.apache.org/viewLog.html?buildId=444231&tab=buildResultsDiv&buildTypeId=IgniteTests_RunAll>
.NET tests
<http://ci.ignite.apache.org/viewLog.html?buildId=443439&tab=buildResultsDiv&buildTypeId=IgniteTests_IgnitePlatformNet>

How about this solution?

2017-02-03 13:59 GMT+03:00 Vyacheslav Daradur <daradurvs@gmail.com>:

> 1. On my first question
> I think up, if we serialize only positive numbers, we can write sign in
> first byte, because it is positive always.
> I will try to make this decision
>
> 2017-02-03 12:48 GMT+03:00 Pavel Tupitsyn <ptupitsyn@apache.org>:
>
>> Vyacheslav,
>>
>> I see the problem now. Yes, negative scale is not supported in .NET.
>>
>> I don't think we should do the multiplication. As you described, this will
>> break equality on Java side. SQL queries might be broken, etc.
>> I think we should throw an exception in .NET when encountering negative
>> decimal scale.
>>
>> Vladimir O, any thoughts?
>>
>> Pavel
>>
>> On Fri, Feb 3, 2017 at 12:01 PM, Vyacheslav Daradur <daradurvs@gmail.com>
>> wrote:
>>
>> > Hello.
>> >
>> > I looked and understood the code of methods ReadDecimal and WriteDecimal
>> > on .NET platform.
>> >
>> > 1. At the moment remaking of this methods for my Java-decimal-fix is
>> very
>> > difficult, it needs to write new methods for
>> serialization/deserialization
>> > of negative decimals.
>> >
>> > I can make it, but there is simpler decision: to add additional byte for
>> > sign.
>> >
>> > I need advice: difficult solution (new methods .net) Versus : simple
>> > solutions (additional byte)?
>> >
>> > *I don't know yet, what changes are necessary on ะก++ platform.
>> >
>> > 2. I see a problem with the negative scale on .NET platform.
>> >
>> > Now negative scale is forbidden.
>> >
>> > We can make:
>> > if (scale < 0) return Decimal.Multiply(new decimal(lo, mid, hi, neg, 0),
>> > new decimal(Math.Pow(10, -scale)));
>> >
>> > But there is the problem:
>> > * 1 Serialize in Java; number=123456789, scale=-4
>> > * 2 Deserialize in .NET; number=1234567890000, scale=0
>> > * 3 Serialize in .NET; number=1234567890000, scale=0
>> > * 4 Deserialize in Java; number=1234567890000, scale=0
>> >
>> > Logically: (1) 123456789 * 10^4 == (2)  1234567890000
>> >
>> > In Java (1) not equal (2), because scales are different.
>> >
>> > Any thougths?
>> >
>> > 2017-01-31 14:08 GMT+03:00 Pavel Tupitsyn <ptupitsyn@apache.org>:
>> >
>> >> Vyacheslav,
>> >>
>> >> I'm not sure I understand the code you attached.
>> >>
>> >> If you know how to fix the .NET part, can you just do it in your PR and
>> >> run "Platform .NET" on TeamCity to verify?
>> >> http://ci.ignite.apache.org/viewType.html?buildTypeId=Ignite
>> >> Tests_IgnitePlatformNet
>> >>
>> >> Thanks,
>> >>
>> >> Pavel
>> >>
>> >> On Tue, Jan 31, 2017 at 1:35 PM, Vyacheslav Daradur <
>> daradurvs@gmail.com>
>> >> wrote:
>> >>
>> >>> Pavel, I see that you are the main contributor of Ignite.NET.
>> >>>
>> >>> We should repair BinaryUtils#ReadDecimal and BinaryUtils#WriteDecimal.
>> >>>
>> >>> *ReadDecimal:
>> >>> byte[] mag = ReadByteArray(stream); // including at least one sign
>> bit,
>> >>> which is (ceil((this.bitLength() + 1)/8))
>> >>> bool neg = (mag[0] < 0);
>> >>> if (scale < 0)
>> >>> // TODO: a scale of -3 means the unscaled value is multiplied by 1000
>> >>>
>> >>> *WriteDecimal:
>> >>> int sign = vals[3] < 0 ? -1 : 0;
>> >>> stream.WriteInt(sign);
>> >>>
>> >>> Can you help with this task?
>> >>>
>> >>>
>> >>> 2017-01-31 12:46 GMT+03:00 Igor Sapego <isapego@gridgain.com>:
>> >>>
>> >>>> Vyacheslav,
>> >>>>
>> >>>> I had a look at your PR and left some comments in Jira.
>> >>>>
>> >>>> Best Regards,
>> >>>> Igor
>> >>>>
>> >>>> On Mon, Jan 30, 2017 at 12:52 PM, Vyacheslav Daradur <
>> >>>> daradurvs@gmail.com>
>> >>>> wrote:
>> >>>>
>> >>>> > Hello. I fixed it. Please, review.
>> >>>> >
>> >>>> > https://issues.apache.org/jira/browse/IGNITE-3196 - Marshaling
>> works
>> >>>> wrong
>> >>>> > for the BigDecimals that have negative scale
>> >>>> >
>> >>>>
>> >>>
>> >>>
>> >>
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message