arcapos / luapgsql

Lua binding for PostgreSQL
108 stars 24 forks source link

Allow passing oids to :prepare #45

Open daurnimator opened 7 years ago

daurnimator commented 7 years ago

PQprepare takes a list of oids; I'd like to pass these directly. Currently in luapgsql you need to create an example piece of data for each type. e.g. conn:prepare("mystatement", [[insert into example(col1, col2) values ($1, $2)]], 1, "two")) where 1 and "two" are values invented just to get the types correct.

As part of this there should be a table exposed pgsql.oid with all known oids.

Possibly also a function getdefaultoid given an object.

mbalmer commented 7 years ago

For prepared statements, we must pass a list of types at the time we prepare the statement. This is a mapping of Lua types to corresponding PostgreSQL data types. This mapping is fixed. There are a lot less Lua data types than oids available. Passing the oids does not make sense, since you could then pass arbitrary and possibly wrong values. In addition, passing oids from a table which you have to dereference makes the code harder to read, same for passing strings. Passing example data like is done now yields the shortest code. No doubt db:prepare(... , 1.0, '', true) is shorter than db:prepare(..., pgsql.oids.numeric, pgsql.oids.text, pgsql.oids.boolean) or even db:prepare(..., 'numeric', 'string', 'boolean').

daurnimator commented 7 years ago

For prepared statements, we must pass a list of types at the time we prepare the statement. This is a mapping of Lua types to corresponding PostgreSQL data types.

This mapping is fixed.

Maybe it shouldn't be.

Passing the oids does not make sense, since you could then pass arbitrary and possibly wrong values.

It would end up being an error at execParams time.

In addition, passing oids from a table which you have to dereference makes the code harder to read, same for passing strings. Passing example data like is done now yields the shortest code. No doubt db:prepare(... , 1.0, '', true) is shorter than db:prepare(..., pgsql.oids.numeric, pgsql.oids.text, pgsql.oids.boolean) or even db:prepare(..., 'numeric', 'string', 'boolean').

I disagree that it's harder to read: making up arbitrary values doesn't make for good code. The length of the code matters little: it's how understandable and debuggable it is.

I think https://github.com/daurnimator/lua-pg/blob/43636e5a7ec7097e7887a7637a16f0c1085bb929/example.lua#L10 is much more readable.

mbalmer commented 7 years ago

Maybe the existing functions could be could be suffixed with „byExample“ and new functions be created that accept a string similar to string.pack() to denote the parameter types (see https://www.lua.org/manual/5.3/manual.html#6.4.2).

Am 13.08.2017 um 15:04 schrieb daurnimator notifications@github.com:

For prepared statements, we must pass a list of types at the time we prepare the statement. This is a mapping of Lua types to corresponding PostgreSQL data types.

This mapping is fixed.

Maybe it shouldn't be.

Passing the oids does not make sense, since you could then pass arbitrary and possibly wrong values.

It would end up being an error at execParams time.

In addition, passing oids from a table which you have to dereference makes the code harder to read, same for passing strings. Passing example data like is done now yields the shortest code. No doubt db:prepare(... , 1.0, '', true) is shorter than db:prepare(..., pgsql.oids.numeric, pgsql.oids.text, pgsql.oids.boolean) or even db:prepare(..., 'numeric', 'string', 'boolean').

I disagree that it's harder to read: making up arbitrary values doesn't make for good code. The length of the code matters little: it's how understandable and debuggable it is.

I think https://github.com/daurnimator/lua-pg/blob/43636e5a7ec7097e7887a7637a16f0c1085bb929/example.lua#L10 is much more readable.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

daurnimator commented 7 years ago

Maybe the existing functions could be could be suffixed with „byExample“

Sounds good.

and new functions be created that accept a string similar to string.pack() to denote the parameter types

Personally I'd prefer the call style I linked above: being able to pass in the oid explicitly. It's much closer to the libpq interface, and I can build on top of it. luapgsql is not a high-level library library; lacking this feature it's also not a low level library: it's somewhere in the less-than-useful middle.

mbalmer commented 7 years ago

Am 14.08.2017 um 02:45 schrieb daurnimator notifications@github.com:

Maybe the existing functions could be could be suffixed with „byExample“

Sounds good.

and new functions be created that accept a string similar to string.pack() to denote the parameter types

Personally I'd prefer the call style I linked above: being able to pass in the oid explicitly. It's much closer to the libpq interface, and I can build on top of it.

It makes no sense to pass OIDs of PostgreSQL data types. We pass Lua variables and luapgsql must map the Lua types to the corresponding PostgreSQL data types. So the question how do indicate which Lua types will be passed during :exec. Now it’s „by example“, but it could be a string where each character stands for one type. That would be consistent with other software we use in C (but which is not published anywhere). Because of that it’s my favourite solution atm should „by-example“ be replaced.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

mbalmer commented 7 years ago

It would be nearly trivial to implement the function this way: If the sole parameter after the SQL command is a string of length >0, it represents datatypes, one character per parameter. If after the SQL command follows an arbitrary number of integers > 0, then they are used as OIDs. Optionally, to stay compatible with the current API, example parameters could be used, but with a limitation: empty string, integer 0, decimal 0.0, nil.