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

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