PQgetCopyData result

[daurnimator]

PQgetCopyData returns:

• an integer >0 when a row is available (the integer is the row's length) + a row's data. This integer should be used as the length rather than using the row as a null terminated string
• 0 if data is not ready yet
• -1 if it's time to call PQgetResult to get the result code.

At the moment luapgsql only doesn't allow distinguishing 0 from -1, Also, it doesn't use the length when pushing the row into lua.

[daurnimator]

Additionally, PQgetCopyData can potentially leak memory (same style of issue as #26).

[mbalmer]

Am 12.05.2016 um 04:11 schrieb daurnimator notifications@github.com:

PQgetCopyData returns:

an integer >0 when a row is available (the integer is the row's length) + a row's data. This integer should be used as the length rather than using the row as a null terminated string 0 if data is not ready yet -1 if it's time to call PQgetResult to get the result code. At the moment luapgsql only doesn't allow distinguishing 0 from -1, Also, it doesn't use the length when pushing the row into lua.

it only return 0 if in async mode, which is not enabled (yet). -1 as you noted, .2 in case of an error.

[daurnimator]

it only return 0 if in async mode, which is not enabled (yet).

Huh? I use :setnonblocking with luapgsql all the time.

[mbalmer]

Am 12.05.2016 um 08:34 schrieb daurnimator notifications@github.com:

it only return 0 if in async mode, which is not enabled (yet).

Huh? I use :setnonblocking with luapgsql all the time.

It's a flag in the function call, see http://www.postgresql.org/docs/9.5/static/libpq-copy.html#LIBPQ-COPY-RECEIVE http://www.postgresql.org/docs/9.5/static/libpq-copy.html#LIBPQ-COPY-RECEIVE

[daurnimator]

Ah. I missed that. I thought the docs where just talking about non-blocking mode.

In that case:

• Please add support for the async flag.
• Handle the 0 return value
• Use the returned length with lua_pushlstring instead of lua_pushstring

[mbalmer]

Am 12.05.2016 um 08:40 schrieb daurnimator notifications@github.com:

Ah. I missed that. I thought the docs where just talking about non-blocking mode.

In that case:

Please add support for the async flag. Handle the 0 return value Use the returned length with lua_pushlstring instead of lua_pushstring I suggest these return values:

• a string if data is returned
• true if the copy is done
• false if no data is present (only in async mode)
• nil if an error occurs

[daurnimator]

I suggest these return values:

• a string if data is returned
• true if the copy is done
• false if no data is present (only in async mode)
• nil if an error occurs

The 2nd option you mention "copy is done" is the same as the first. There are 3 possible outcomes:

• got a row: return it as a string (already handled)
• need to call PQgetResult: return nil (already handled)
• need to call consumeInput and try again (new possibility. I guess returning false works.)

[mbalmer]

Am 12.05.2016 um 08:53 schrieb daurnimator notifications@github.com:

I suggest these return values:

a string if data is returned true if the copy is done false if no data is present (only in async mode) nil if an error occurs The 2nd option you mention "copy is done" is the same as the first. There are 3 possible outcomes:

got a row: return it as a string (already handled) need to call PQgetResult: return nil (already handled) need to call consumeInput and try again (new possibility. I guess returning false works.) It should handle all cases now. Rechecking the docs...

[daurnimator]

Ah ha. I actually did miss the -1 vs -2 distinction. The result pushing code in be222cd9b3b5fd8ef05d3bde1348b6655cb03abc looks good :)

However, make sure you do all argument checking before calling gcmalloc.

Also, I'd consider adding in a luaL_argcheck that the type of async is boolean or nil.

[daurnimator]

Fixed now.