Re: Problem retrieving a numeric(38,0) value as SQL_NUMERIC_STRUCT if value needs to use all 16 SQLCHAR elements of the val array - Mailing list pgsql-odbc

Thanks for your investigation.

I don't object to the change.
Probably few people used SQL_C_NUMERIC extensively.

regards,
Hiroshi Inoue

(2014/06/10 17:34), Heikki Linnakangas wrote:
> Ugh, the decimal->binary conversion algorithm is really ugly even before
> this patch, and the patch doesn't make it any prettier. It took me quite
> a while to understand how it works: it repeatedly divides the base-10
> representation by two, and outputs the reminder bit. There are much
> better and faster ways to convert from decimal to binary, and they're
> not really any more complicated either.
>
> The binary->decimal conversion in ResolveNumericParam is even more ugly.
> To be honest, I still don't understand the algorithm behind it, but it
> looks complicated and slow. There's a fast-path for small numbers that
> fit in an UInt4, which doesn't make it any simpler.
>
> Aside from ugly and slow, the binary->decimal conversion is also buggy:
>
> 1.The following input creates a core dump with "Floating point exception":
>
> precision=3, scale=50, val=0x02 0x03
>
> 2. Building the final output string is broken if scale is large, e.g.
> this produces garbage output:
>
> precision=25, scale=80
>
> ERROR: invalid input syntax for type numeric:
> "p.000`000H0000000000000000000000aT000000000000000000000000000000000000000000000770";
>
>
> 3. The param_string buffer allocated for the result of the
> binary->decimal conversion is too small. The buffer is 128 bytes, but a
> numeric can have a scale as large as 127. That won't fit, as the sign
> and decimal dot use two more bytes.
>
> I propose the attached patch to fix all of the above, rewriting the
> binary->decimal and decimal->binary conversions altogether. It fixes all
> of the known issues, the ones listed above and the one Walter reported.
> It also adds a regression test for all of them.
>
> Can anyone come up with some more special cases that this patched or the
> old conversion might handle incorrectly, for adding to the regression
> suite?
>
>
> One more thing: The precision works in a funny way, if you pass a "val"
> with more bits than fit in precision. I filled the val-bytes with 0x9F
> 0x86 0x01, which corresponds to 99999 in decimal. I passed that as a
> param with different precisions:
>
> sign 1 prec 1 scale 0 val 9F8601:
>      159
> sign 1 prec 2 scale 0 val 9F8601:
>      159
> sign 1 prec 3 scale 0 val 9F8601:
>      34463
> sign 1 prec 4 scale 0 val 9F8601:
>      34463
> sign 1 prec 5 scale 0 val 9F8601:
>      99999
> sign 1 prec 6 scale 0 val 9F8601:
>      99999
>
> The conversion algorithm seems to assume that there are no extra bits in
> the val-array that don't fit within the precision. Perhaps that's a
> reasonable assumption, but I think it would be more robust to ignore any
> extra bits if they are there, rather than sometimes take some of them
> into account. (Or, always parse all the bits, ignoring the precision.) I
> did that in the attached patch.
>
> However, according to this article I found from Microsoft:
> https://support.microsoft.com/kb/222831
>
> "The precision and scale fields of the numeric structure are never used
> for input from an application, only for output from the driver to the
> application."
>
> So we probably should be ignoring them completely, and get the precision
> and scale from elsewhere.
>
>
> PS. ResolveNumericParam always returns TRUE. That's fortunate, because
> the caller would incorrectly fall through the switch-case if it didn't..
> When the code was added, it fell through to the "default" case, but the
> interval-cases were later added in between, breaking it. But it's not a
> live bug, since ResolveNumericParam always returns TRUE. Fixed that too
> in the attached patch.
>
> - Heikki
>