Looks good to me. Best Regards, Igor
On Tue, Feb 7, 2017 at 2:55 PM, Vyacheslav Daradur <daradu...@gmail.com> wrote: > Ok, thanks for explanations. > > What about this task? > > 2017-02-07 13:57 GMT+03:00 Igor Sapego <isap...@gridgain.com>: > >> But that's Ok. Since we use int8_t for bytes in C++ as well I guess >> your -0x80 may have more sense than 0x80. >> >> Best Regards, >> Igor >> >> On Tue, Feb 7, 2017 at 1:54 PM, Igor Sapego <isap...@gridgain.com> wrote: >> >>> I was just curious. >>> >>> In C++ both constants 0x80 and -0x80 are of type 'int' and have the same >>> lower byte, so they give the same result. Though first number is actually >>> 0x00000080 when the second one is 0xFFFFFF80. >>> >>> So it's just made a minus sign look a little redundant and pointless to >>> me >>> in C++ code. >>> >>> Best Regards, >>> Igor >>> >>> On Mon, Feb 6, 2017 at 10:15 PM, Vyacheslav Daradur <daradu...@gmail.com >>> > wrote: >>> >>>> Byte.MIN_VALUE = -128 = -0x80 >>>> Byte.MAX_VALUE = 127 = 0x7F >>>> >>>> It is just more evident for me. >>>> >>>> Maybe, I just have the Java programming style. >>>> >>>> In Java: >>>> byte a = 100 | -0x80; // compiled >>>> byte b = 100 | 0x80; // doesn't compile, explicit type casting is >>>> neccessary (byte)(100 | 0x80) >>>> System.out.println(a | -0x80); // -28 >>>> System.out.println(a | 0x80); // 228 - cast to int >>>> >>>> Is it bad style? >>>> >>>> 2017-02-06 20:04 GMT+03:00 Igor Sapego <isap...@gridgain.com>: >>>> >>>>> Vyacheslav, >>>>> >>>>> Overall looks good. But why do you use -0x80 instead of 0x80? >>>>> >>>>> Best Regards, >>>>> Igor >>>>> >>>>> On Mon, Feb 6, 2017 at 5:36 PM, Vyacheslav Daradur < >>>>> daradu...@gmail.com> wrote: >>>>> >>>>>> Igor, >>>>>> >>>>>> I didn't change the CPP code before approval approach. >>>>>> I shall write directly, sorry. >>>>>> >>>>>> But I made CPP changes already. >>>>>> >>>>>> > TestEscConvertFunctionFloat >>>>>> > TestEscConvertFunctionDouble. >>>>>> These tests were passed >>>>>> <http://ci.ignite.apache.org/viewQueued.html?itemId=445824> >>>>>> >>>>>> >>>>>> >>>>>> 2017-02-06 13:20 GMT+03:00 Pavel Tupitsyn <ptupit...@apache.org>: >>>>>> >>>>>>> .NET changes look good to me. >>>>>>> >>>>>>> Pavel >>>>>>> >>>>>>> On Mon, Feb 6, 2017 at 1:10 PM, Igor Sapego <isap...@gridgain.com> >>>>>>> wrote: >>>>>>> >>>>>>> > Vyacheslav, I can see two ODBC tests fail in C++ test suits that >>>>>>> should >>>>>>> > not: >>>>>>> > - TestEscConvertFunctionFloat >>>>>>> > <http://ci.ignite.apache.org/viewLog.html?buildId=444207&tab >>>>>>> =buildResultsDiv&buildTypeId=IgniteTests_IgnitePlatformCppLi >>>>>>> nux#testNameId-9178617718508801660> >>>>>>> > - TestEscConvertFunctionDouble >>>>>>> > <http://ci.ignite.apache.org/viewLog.html?buildId=444207&tab >>>>>>> =buildResultsDiv&buildTypeId=IgniteTests_IgnitePlatformCppLi >>>>>>> nux#testNameId5432107083822590090> >>>>>>> > . >>>>>>> > >>>>>>> > I believe, this is because I can't see any changes in C++ Decimal >>>>>>> > marshaling code. >>>>>>> > Please, pay attention to file ignite\modules\platforms\cpp\ >>>>>>> > odbc\src\utility.cpp, >>>>>>> > functions ReadDecimal and WriteDecimal. >>>>>>> > >>>>>>> > Best Regards, >>>>>>> > Igor >>>>>>> > >>>>>>> > On Mon, Feb 6, 2017 at 11:21 AM, Vyacheslav Daradur < >>>>>>> daradu...@gmail.com> >>>>>>> > wrote: >>>>>>> > >>>>>>> >> 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 < >>>>>>> daradu...@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 <ptupit...@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 < >>>>>>> >>>> daradu...@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 < >>>>>>> ptupit...@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 < >>>>>>> >>>> daradu...@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 < >>>>>>> isap...@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 < >>>>>>> >>>> >>>> daradu...@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 >>>>>>> >>>> >>>> > >>>>>>> >>>> >>>> >>>>>>> >>>> >>> >>>>>>> >>>> >>> >>>>>>> >>>> >> >>>>>>> >>>> > >>>>>>> >>>> >>>>>>> >>> >>>>>>> >>> >>>>>>> >> >>>>>>> > >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >
