arcapos / luapgsql

Lua binding for PostgreSQL
108 stars 24 forks source link

sendQueryParams broken in HEAD #18

Closed daurnimator closed 9 years ago

daurnimator commented 9 years ago
local pg = require "pgsql"

local conn = pg.connectdb("dbname=test")
assert(conn:status() == pg.CONNECTION_OK, conn:errorMessage())

-- execParams works fine
local res = conn:execParams("SELECT $1, $2, $3", "foo", 123809878, 0987788789);
assert(res:status() == pg.PGRES_TUPLES_OK, res:errorMessage())

-- sendQueryParams is broken
if conn:sendQueryParams("SELECT $1, $2, $3", "foo", 123809878, 0987788789) == 0 then
    error(conn:errorMessage())
end
local res = conn:getResult()
-- Have to read until nil
assert(conn:getResult() == nil)
assert(res:status() == pg.PGRES_TUPLES_OK, res:errorMessage())
EXEC    SELECT $1, $2, $3       foo     123809878       987788789
lua5.1: failure.lua:5: ERROR:  could not determine data type of parameter $2

stack traceback:
        [C]: in function 'assert'
        failure.lua:5: in main chunk
        [C]: ?
daurnimator commented 9 years ago

Staring at the code side by side; it might be the 3rd parameter to get_sql_params: Compare: https://github.com/mbalmer/luapgsql/blob/9ce83ea9ee1502a377125fba96cc03466f5044ba/luapgsql.c#L439 vs https://github.com/mbalmer/luapgsql/blob/9ce83ea9ee1502a377125fba96cc03466f5044ba/luapgsql.c#L734

It also appears that the NULL check at https://github.com/mbalmer/luapgsql/blob/9ce83ea9ee1502a377125fba96cc03466f5044ba/luapgsql.c#L744 should be removed?

mbalmer commented 9 years ago

Can you recheck now that I fixed a blocking issue?

mbalmer commented 9 years ago

The NULL check is there in case calloc fauils (OOM)

daurnimator commented 9 years ago

The NULL check is there in case calloc fauils (OOM)

Yes, I figured the lack of it in conn_execParams meant it was not required. Actually it seems that it's missing in conn_execParams.

daurnimator commented 9 years ago

In any case, d592d3f023efb298fde7c536305f4f6d3507f073 fixes the main issue reported here. Thanks @geoffleyland

mbalmer commented 9 years ago

Am 18.02.15 um 17:40 schrieb daurnimator:

The NULL check is there in case calloc fauils (OOM)

Yes, I figured the lack of it in |conn_execParams| meant it was not required. Actually it seems that it's /missing/ in |conn_execParams|.

— Reply to this email directly or view it on GitHub https://github.com/mbalmer/luapgsql/issues/18#issuecomment-74897497.

fixed.

daurnimator commented 9 years ago

fixed.

Thanks!