[Tarantool-patches] [PATCH v2 07/16] port: add dump format and request type to port_sql
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Dec 4 01:51:15 MSK 2019
Thanks for the patch!
See 4 comments below.
On 20/11/2019 22:28, Nikita Pettik wrote:
> Dump formats of DQL and DML queries are different: the last one contains
> number of affected rows and optionally list of autoincremented ids; the
> first one comprises all meta-information including column names of
> resulting set and their types. What is more, dump format is going to be
> different for execute and prepare requests. So let's introduce separate
> member to struct port_sql responsible for dump format to be used.
>
> What is more, prepared statement finalization is required only for
> PREPARE-AND-EXECUTE requests. So let's keep request type in port as well.
1. I don't think it is a good idea to rely on certain
meta being returned by only certain requests. There is
a ticket, which will remove the border between different
request types:
https://github.com/tarantool/tarantool/issues/3649.
Yes, now this is either sql_info or metadata, but that
will change.
>
> Note that C standard specifies that enums are integers, but it does not
> specify the size. Hence, let's use simple uint8 - mentioned enums are
> small enough to fit into it.
2. Try this:
enum ... <member> : 8;
>
> Needed for #2592
> ---
> src/box/execute.c | 32 +++++++++++++++++++++++---------
> src/box/execute.h | 24 ++++++++++++++++++++++++
> src/box/lua/execute.c | 20 +++++++++++++-------
> 3 files changed, 60 insertions(+), 16 deletions(-)
>
> diff --git a/src/box/execute.c b/src/box/execute.c
> index 83680b70f..d2a999099 100644
> --- a/src/box/execute.c
> +++ b/src/box/execute.c
> @@ -100,7 +100,9 @@ static void
> port_sql_destroy(struct port *base)
> {
> port_tuple_vtab.destroy(base);
> - sql_finalize(((struct port_sql *)base)->stmt);
> + struct port_sql *port_sql = (struct port_sql *) base;
> + if (port_sql->request == PREPARE_AND_EXECUTE)
3. You could turn the types into a bitmask to drop the
dependency on a certain request type:
bool do_finalize : 1;
bool do_omit_data : 1;
> + sql_finalize(((struct port_sql *)base)->stmt);
> }
>
> const struct port_vtab port_sql_vtab = {
> @@ -395,6 +404,9 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
> mp_encode_int(buf, id_entry->id);
> }
> }
> + break;
> + }
> + default: unreachable();
4. Please, wrap on a new line, and include into {}. I know,
this is shorter, but this just does not match the code
style. In the other place too.
> }
> return 0;
> }
More information about the Tarantool-patches
mailing list