Return ffi's NULL instead of nil from lua_mysql_pushresult()

[darkwrat]

Before patch:

tarantool> rows = conn:execute('select rand() as w, NULL as x')

---
...

tarantool> rows

---
- - w: 0.81680908595846
...

tarantool> json.encode(rows)

---
- '[{"w":0.81680908595846}]'
...

After patch:

tarantool> rows = conn:execute('select rand() as w, NULL as x')

---
...

tarantool> rows

---
- - x: null
    w: 0.28386442669096
...

tarantool> json.encode(rows)

---
- '[{"x":null,"w":0.28386442669096}]'
...

[darkwrat]

This function is actually a copy-paste from tarantool/src/lua/utils.h.

[bigbes]

luaL_pushnull must be imported to public API, you're right

[GeorgyKirichenko]

May be adding some sql meta info to returning tables (for example, field names with its native sql types) should be a better solution?

[Totktonada]

The proposed change may be considered as backward incompatible, however I don't mind to enable this behaviour under an option like keep_null.

May be adding some sql meta info to returning tables (for example, field names with its native sql types) should be a better solution?

The metadata is returned now when the use_numeric_result option is set (introduced in PR #25):

{
    { -- result set
        rows = {
            {r1c1val, r1c2val, ...}, -- row
            {r2c1val, r2c2val, ...}, -- row
            ...
        },
        metadata = {
            {type = 'long', name = 'col1'}, -- column meta
            {type = 'long', name = 'col2'}, -- column meta
            ...
        },
    },
    ...
}

[darkwrat]

Please excuse me for the terribly slow response, I think it's time for me to tinker with some notification settings :)

The proposed change may be considered as backward incompatible, however I don't mind to enable this behaviour under an option like keep_null.

Sounds great, I will update the changeset and README accordingly.

The metadata is returned now when the use_numeric_result option is set (introduced in PR #25):

In my opinion the metadata table introduces a complication for the particular case of simply packing the response and sending it over the wire. Some effort still must be put on the receiving end to actually discover that a value is NULL.

[darkwrat]

It looks like CI fails because Tarantool 1.6 (which this change was initally based on) had all non-static symbols visible to ffi (tarantool/tarantool#2971).
So extern int luaL_nil_ref shenanigans are not viable anymore, and as @bigbes pointed out luaL_pushnull must be exported to the public API first (as well as luaL_nil_ref?).

As I see this, it means that the change may not be introduced without breaking compatibility with all existing tarantool versions out there, and that is quite a bummer.

[Totktonada]

So extern int luaL_nil_ref shenanigans are not viable anymore, and as @bigbes pointed out luaL_pushnull must be exported to the public API first (as well as luaL_nil_ref?).

You can make the variable static and assign it within the module. See utils.c.

[Totktonada]

So extern int luaL_nil_ref shenanigans are not viable anymore, and as @bigbes pointed out luaL_pushnull must be exported to the public API first (as well as luaL_nil_ref?).

You can make the variable static and assign it within the module. See utils.c.

The code now uses CTID_P_VOID, but it is internal contant, which is not guaranteed to be the same in future. I'll look for other approach, which will lean only on public LuaJIT / Tarantool APIs.

[romanhabibov]

@darkwrat, if you are not going to make changes, then let me know I will finish this pull request.

[darkwrat]

@darkwrat, if you are not going to make changes, then let me know I will finish this pull request.

Please go ahead.

[Totktonada]

CI fails are expected. See #53 and #54.

[Totktonada]

LGTM.