Incorrect handling of infinitys

[daurnimator]

asprintf(&paramValues[n], "%f", v) incorrectly renders inf for math.huge. It should render Infinity

Issue is at https://github.com/mbalmer/luapgsql/blob/054741811a92cfd8173be9d41a2b59f778309da4/luapgsql.c#L410. Postgres docs: http://www.postgresql.org/docs/current/static/datatype-numeric.html

[mbalmer]

Yes, math.huge is incorrectly handled here. Did you try if rendering 'Infinity' here actually works? At least using pgsql I can not use Infinity and I find no docs as what PQexecParams() expects for Infinity. Need to dig into this.

[mbalmer]

Lua numbers are mapped to numeric values in PostgreSQL, and for numeric values, Infinity and -Infinity are not defined, only NaN. So math.huge can not be converted to a numeric value at all. It remains to be discussed whether the mapping to numeric is the best way.

[daurnimator]

Lua numbers are mapped to numeric values in PostgreSQL, and for numeric values, Infinity and -Infinity are not defined, only NaN. So math.huge can not be converted to a numeric value at all. It remains to be discussed whether the mapping to numeric is the best way.

As lua numbers (at least <5.3) are always doubles, why not use FLOAT8OID?

[daurnimator]

Furthermore, asprintf with "%f" is likely resulting in a loss of precision for numeric arguments.

From man page (emphasis mine):

f, F

The double argument is rounded and converted to decimal notation in the style [-]ddd.ddd, where the number of digits after the decimal-point character is equal to the precision specification. If the precision is missing, it is taken as 6; if the precision is explicitly zero, no decimal-point character appears. If a decimal point appears, at least one digit appears before it.

[mbalmer]

How would lua_tostring() convert a number? With the same precision implications?

[mbalmer]

Actually Lua seems to use 12 digits after the decimal point for conversions, so I guess we should go with that, too. "%.12f" instead of "%f".

[daurnimator]

Actually Lua seems to use 12 digits after the decimal point for conversions, so I guess we should go with that, too. "%.12f" instead of "%f".

Only for the default tostring. You should be using the full precision available. e.g. %.17f or %a, but that's C99 only.

Why not use FLOAT8OID in binary format? It seems much less error prone.

[daurnimator]

Actually Lua seems to use 12 digits after the decimal point for conversions, so I guess we should go with that, too. "%.12f" instead of "%f".

btw; not sure where you got the 12 from; lua uses %.14g itself; see http://www.lua.org/source/5.2/luaconf.h.html#LUA_NUMBER_FMT

[mbalmer]

NUMERICOID or FLOAT8OID, does it really matter? Imo, both are just fine. If we switch to binary fomats, then we likely need to include server headers again: "Values passed in binary format require knowledge of the internal representation expected by the backend.". Need to check this.

[mbalmer]

on my system, Lua outputs numbers with 12 digits, that is why I assumed .12f. No problem going to .14f, though.

[mbalmer]

btw, I just removed asprintf().

[daurnimator]

NUMERICOID or FLOAT8OID, does it really matter? Imo, both are just fine. If we switch to binary fomats, then we likely need to include server headers again: "Values passed in binary format require knowledge of the internal representation expected by the backend.". Need to check this.

I was under the impression that FLOAT8OID was actually just a double in standard IEEE754 format... i.e. you could just assign it in.

[daurnimator]

I was under the impression that FLOAT8OID was actually just a double in standard IEEE754 format... i.e. you could just assign it in.

I checked the source; and it is:

• http://doxygen.postgresql.org/float_8c_source.html#l00562
• http://doxygen.postgresql.org/pqformat_8c_source.html#l00529
• http://doxygen.postgresql.org/c_8h_source.html#l00334

[mbalmer]

So I implemented it and it seems to work. The double value has to be sent in big endian format.

[mbalmer]

Fixed in pgsql-1.4.2.

[daurnimator]

Looks good; though may have raised some issues around portability of 'endian.h'