From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 8277A46970E for ; Mon, 30 Dec 2019 01:32:17 +0300 (MSK) References: <06ed9bc1dade3011b573ae1105e39e372e1408f9.1577573718.git.korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <01f239af-b54d-9f21-c60b-99eb722c00e0@tarantool.org> Date: Mon, 30 Dec 2019 01:32:13 +0300 MIME-Version: 1.0 In-Reply-To: <06ed9bc1dade3011b573ae1105e39e372e1408f9.1577573718.git.korablev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] sql: extend result set with span List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.tarantool.org, Kirill Yukhin LGTM. On 29/12/2019 01:56, Nikita Pettik wrote: > Each column of result set features its name span (in full metadata > mode). For instance: > > SELECT x + 1 AS add FROM ...; > > In this case real name (span) of resulting set column is "x + 1" > meanwhile "add" is its alias. This patch extends metadata with > member which corresponds to column's original expression. > It is worth mentioning that in most cases span coincides with name, so > to avoid overhead and sending the same string twice, we follow the rule > that if span is encoded as MP_NIL then its value is the same as name. > Also note that span is always presented in full metadata mode. > > Closes #4407 > > @TarantoolBot document > Title: extended SQL metadata > > Before this patch metadata for SQL DQL contained only two fields: > name and type of each column of result set. Now it may contain > following properties: > - collation (in case type of resulting set column is string and > collation is different from default "none"); > is encoded with IPROTO_FIELD_COLL (0x2) key in IPROTO_METADATA map; > in msgpack is encoded as string and held with MP_STR type; > - is_nullable (in case column of result set corresponds to space's > field; for expressions like x+1 for the sake of > simplicity nullability is omitted); > is encoded with IPROTO_FIELD_IS_NULLABLE key (0x3) in IPROTO_METADATA; > in msgpack is encoded as boolean and held with MP_BOOL type; > note that absence of this field implies that nullability is unknown; > - is_autoincrement (is set only for autoincrement column in result > set); > is encoded with IPROTO_FIELD_IS_AUNTOINCREMENT (0x4) key in IPROTO_METADATA; > in msgpack is encoded as boolean and held with MP_BOOL type; > - span (is always set in full metadata mode; it is an original > expression forming result set column. For instance: > SELECT a + 1 AS x; -- x is a name, meanwhile a + 1 is a span); > is encoded with IPROTO_FIELD_SPAN (0x5) key in IPROTO_METADATA map; > in msgpack is encoded as string and held with MP_STR type OR > as NIL with MP_NIL type. The latter case indicates that span > coincides with name. This simple optimization allows to avoid > sending the same string twice. > > This extended metadata is send only when PRAGMA full_metadata is > enabled. Otherwise, only basic (name and type) metadata is processed. > --- > Branch: https://github.com/tarantool/tarantool/tree/np/gh-4407-extend-sql-metadata-v2 > Issue: https://github.com/tarantool/tarantool/issues/4407 > > src/box/execute.c | 33 ++++++++++++-- > src/box/iproto_constants.h | 1 + > src/box/lua/execute.c | 10 +++++ > src/box/lua/net_box.c | 34 ++++++++++---- > src/box/sql/select.c | 10 +++++ > src/box/sql/sqlInt.h | 6 +++ > src/box/sql/vdbe.h | 3 ++ > src/box/sql/vdbeInt.h | 6 +++ > src/box/sql/vdbeapi.c | 15 +++++++ > src/box/sql/vdbeaux.c | 19 ++++++++ > test/sql/full_metadata.result | 99 ++++++++++++++++++++++++++++++++++++++--- > test/sql/full_metadata.test.lua | 11 +++++ > 12 files changed, 230 insertions(+), 17 deletions(-) > > diff --git a/src/box/execute.c b/src/box/execute.c > index c70935d01..3034cee86 100644 > --- a/src/box/execute.c > +++ b/src/box/execute.c > @@ -270,7 +270,7 @@ error: > > static inline size_t > metadata_map_sizeof(const char *name, const char *type, const char *coll, > - int nullable, bool is_autoincrement) > + const char *span, int nullable, bool is_autoincrement) > { > uint32_t members_count = 2; > size_t map_size = 0; > @@ -289,6 +289,12 @@ metadata_map_sizeof(const char *name, const char *type, const char *coll, > map_size += mp_sizeof_uint(IPROTO_FIELD_IS_AUTOINCREMENT); > map_size += mp_sizeof_bool(true); > } > + if (sql_metadata_is_full()) { > + members_count++; > + map_size += mp_sizeof_uint(IPROTO_FIELD_SPAN); > + map_size += span != NULL ? mp_sizeof_str(strlen(span)) : > + mp_sizeof_nil(); > + } > map_size += mp_sizeof_uint(IPROTO_FIELD_NAME); > map_size += mp_sizeof_uint(IPROTO_FIELD_TYPE); > map_size += mp_sizeof_str(strlen(name)); > @@ -299,10 +305,13 @@ metadata_map_sizeof(const char *name, const char *type, const char *coll, > > static inline void > metadata_map_encode(char *buf, const char *name, const char *type, > - const char *coll, int nullable, bool is_autoincrement) > + const char *coll, const char *span, int nullable, > + bool is_autoincrement) > { > uint32_t map_sz = 2 + (coll != NULL) + (nullable != -1) + > is_autoincrement; > + if (sql_metadata_is_full()) > + map_sz++; > buf = mp_encode_map(buf, map_sz); > buf = mp_encode_uint(buf, IPROTO_FIELD_NAME); > buf = mp_encode_str(buf, name, strlen(name)); > @@ -320,6 +329,21 @@ metadata_map_encode(char *buf, const char *name, const char *type, > buf = mp_encode_uint(buf, IPROTO_FIELD_IS_AUTOINCREMENT); > buf = mp_encode_bool(buf, true); > } > + if (sql_metadata_is_full()) { > + /* > + * Span is an original expression that forms > + * result set column. In most cases it is the > + * same as column name. So to avoid sending > + * the same string twice simply encode it as > + * a nil and account this behaviour on client > + * side (see decode_metadata_optional()). > + */ > + buf = mp_encode_uint(buf, IPROTO_FIELD_SPAN); > + if (span != NULL) > + buf = mp_encode_str(buf, span, strlen(span)); > + else > + buf = mp_encode_nil(buf); > + } > } > > /** > @@ -348,6 +372,7 @@ sql_get_metadata(struct sql_stmt *stmt, struct obuf *out, int column_count) > const char *coll = sql_column_coll(stmt, i); > const char *name = sql_column_name(stmt, i); > const char *type = sql_column_datatype(stmt, i); > + const char *span = sql_column_span(stmt, i); > int nullable = sql_column_nullable(stmt, i); > bool is_autoincrement = sql_column_is_autoincrement(stmt, i); > /* > @@ -357,14 +382,14 @@ sql_get_metadata(struct sql_stmt *stmt, struct obuf *out, int column_count) > */ > assert(name != NULL); > assert(type != NULL); > - size = metadata_map_sizeof(name, type, coll, nullable, > + size = metadata_map_sizeof(name, type, coll, span, nullable, > is_autoincrement); > char *pos = (char *) obuf_alloc(out, size); > if (pos == NULL) { > diag_set(OutOfMemory, size, "obuf_alloc", "pos"); > return -1; > } > - metadata_map_encode(pos, name, type, coll, nullable, > + metadata_map_encode(pos, name, type, coll, span, nullable, > is_autoincrement); > } > return 0; > diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h > index 30d1af4cb..3808c6f28 100644 > --- a/src/box/iproto_constants.h > +++ b/src/box/iproto_constants.h > @@ -134,6 +134,7 @@ enum iproto_metadata_key { > IPROTO_FIELD_COLL = 2, > IPROTO_FIELD_IS_NULLABLE = 3, > IPROTO_FIELD_IS_AUTOINCREMENT = 4, > + IPROTO_FIELD_SPAN = 5, > }; > > enum iproto_ballot_key { > diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c > index e8e3e2a9f..5ed7b9bd3 100644 > --- a/src/box/lua/execute.c > +++ b/src/box/lua/execute.c > @@ -23,10 +23,13 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L, > const char *coll = sql_column_coll(stmt, i); > const char *name = sql_column_name(stmt, i); > const char *type = sql_column_datatype(stmt, i); > + const char *span = sql_column_span(stmt, i); > int nullable = sql_column_nullable(stmt, i); > bool is_autoincrement = sql_column_is_autoincrement(stmt, i); > size_t table_sz = 2 + (coll != NULL) + (nullable != -1) + > is_autoincrement; > + if (sql_metadata_is_full()) > + table_sz++; > lua_createtable(L, 0, table_sz); > /* > * Can not fail, since all column names are > @@ -51,6 +54,13 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L, > lua_pushboolean(L, true); > lua_setfield(L, -2, "is_autoincrement"); > } > + if (sql_metadata_is_full()) { > + if (span != NULL) > + lua_pushstring(L, span); > + else > + lua_pushstring(L, name); > + lua_setfield(L, -2, "span"); > + } > lua_rawseti(L, -2, i + 1); > } > } > diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c > index c22848874..6db506b57 100644 > --- a/src/box/lua/net_box.c > +++ b/src/box/lua/net_box.c > @@ -641,7 +641,7 @@ netbox_decode_select(struct lua_State *L) > /** Decode optional (i.e. may be present in response) metadata fields. */ > static void > decode_metadata_optional(struct lua_State *L, const char **data, > - uint32_t map_size) > + uint32_t map_size, const char *name, uint32_t name_len) > { > /* 2 is default metadata map size (field name + field size). */ > while (map_size-- > 2) { > @@ -655,6 +655,24 @@ decode_metadata_optional(struct lua_State *L, const char **data, > bool is_nullable = mp_decode_bool(data); > lua_pushboolean(L, is_nullable); > lua_setfield(L, -2, "is_nullable"); > + } else if (key == IPROTO_FIELD_SPAN) { > + /* > + * There's an agreement: if span is not > + * presented in metadata (encoded as NIL), > + * then it is the same as name. It allows > + * avoid sending the same string twice. > + */ > + const char *span = NULL; > + if (mp_typeof(**data) == MP_STR) { > + span = mp_decode_str(data, &len); > + } else { > + assert(mp_typeof(**data) == MP_NIL); > + mp_decode_nil(data); > + span = name; > + len = name_len; > + } > + lua_pushlstring(L, span, len); > + lua_setfield(L, -2, "span"); > } else { > assert(key == IPROTO_FIELD_IS_AUTOINCREMENT); > bool is_autoincrement = mp_decode_bool(data); > @@ -676,21 +694,21 @@ netbox_decode_metadata(struct lua_State *L, const char **data) > lua_createtable(L, count, 0); > for (uint32_t i = 0; i < count; ++i) { > uint32_t map_size = mp_decode_map(data); > - assert(map_size >= 2 && map_size <= 5); > + assert(map_size >= 2 && map_size <= 6); > uint32_t key = mp_decode_uint(data); > assert(key == IPROTO_FIELD_NAME); > (void) key; > lua_createtable(L, 0, map_size); > - uint32_t len; > - const char *str = mp_decode_str(data, &len); > - lua_pushlstring(L, str, len); > + uint32_t name_len, type_len; > + const char *str = mp_decode_str(data, &name_len); > + lua_pushlstring(L, str, name_len); > lua_setfield(L, -2, "name"); > key = mp_decode_uint(data); > assert(key == IPROTO_FIELD_TYPE); > - const char *type = mp_decode_str(data, &len); > - lua_pushlstring(L, type, len); > + const char *type = mp_decode_str(data, &type_len); > + lua_pushlstring(L, type, type_len); > lua_setfield(L, -2, "type"); > - decode_metadata_optional(L, data, map_size); > + decode_metadata_optional(L, data, map_size, str, name_len); > lua_rawseti(L, -2, i + 1); > } > } > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index a19494ed9..b4182b88a 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1849,6 +1849,10 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > if (space->sequence != NULL && > space->sequence_fieldno == (uint32_t) iCol) > vdbe_metadata_set_col_autoincrement(v, i); > + if (pEList->a[i].zName != NULL) { > + vdbe_metadata_set_col_span(v, i, > + pEList->a[i].zSpan); > + } > } > } else { > const char *z = NULL; > @@ -1859,6 +1863,12 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > else > z = tt_sprintf("column%d", i + 1); > vdbe_metadata_set_col_name(v, i, z); > + if (is_full_meta) { > + if (pEList->a[i].zName != NULL) { > + vdbe_metadata_set_col_span(v, i, > + pEList->a[i].zSpan); > + } > + } > } > } > if (var_count == 0) > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > index e248bc673..250f6a2cd 100644 > --- a/src/box/sql/sqlInt.h > +++ b/src/box/sql/sqlInt.h > @@ -536,6 +536,9 @@ sql_finalize(sql_stmt * pStmt); > int > sql_reset(struct sql_stmt *stmt); > > +bool > +sql_metadata_is_full(); > + > int > sql_exec(sql *, /* An open database */ > const char *sql, /* SQL to be evaluated */ > @@ -585,6 +588,9 @@ sql_column_nullable(sql_stmt *stmt, int n); > bool > sql_column_is_autoincrement(sql_stmt *stmt, int n); > > +const char * > +sql_column_span(sql_stmt *stmt, int n); > + > int > sql_initialize(void); > > diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h > index 1e585d89d..79fe6f95d 100644 > --- a/src/box/sql/vdbe.h > +++ b/src/box/sql/vdbe.h > @@ -263,6 +263,9 @@ vdbe_metadata_set_col_nullability(struct Vdbe *p, int idx, int nullable); > void > vdbe_metadata_set_col_autoincrement(struct Vdbe *p, int idx); > > +int > +vdbe_metadata_set_col_span(struct Vdbe *p, int idx, const char *span); > + > void sqlVdbeCountChanges(Vdbe *); > sql *sqlVdbeDb(Vdbe *); > void sqlVdbeSetSql(Vdbe *, const char *z, int n, int); > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index fcdcd9b70..1022ec040 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -357,6 +357,12 @@ struct sql_column_metadata { > int8_t nullable; > /** True if column features autoincrement property. */ > bool is_actoincrement; > + /** > + * Span is an original expression forming result set > + * column. In most cases it is the same as name; it is > + * different only in case of presence of AS clause. > + */ > + char *span; > }; > > /* > diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c > index 5b423c9df..559251140 100644 > --- a/src/box/sql/vdbeapi.c > +++ b/src/box/sql/vdbeapi.c > @@ -36,6 +36,7 @@ > */ > #include "sqlInt.h" > #include "vdbeInt.h" > +#include "box/session.h" > > /* > * Invoke the profile callback. This routine is only called if we already > @@ -115,6 +116,12 @@ sql_clear_bindings(sql_stmt * pStmt) > return rc; > } > > +bool > +sql_metadata_is_full() > +{ > + return current_session()->sql_flags & SQL_FullMetadata; > +} > + > /**************************** sql_value_ ****************************** > * The following routines extract information from a Mem or sql_value > * structure. > @@ -769,6 +776,14 @@ sql_column_is_autoincrement(sql_stmt *stmt, int n) > return p->metadata[n].is_actoincrement; > } > > +const char * > +sql_column_span(sql_stmt *stmt, int n) > +{ > + struct Vdbe *p = (struct Vdbe *) stmt; > + assert(n < sql_column_count(stmt) && n >= 0); > + return p->metadata[n].span; > +} > + > /******************************* sql_bind_ ************************** > * > * Routines used to attach values to wildcards in a compiled SQL statement. > diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c > index 5b4bc0182..d4db6aca2 100644 > --- a/src/box/sql/vdbeaux.c > +++ b/src/box/sql/vdbeaux.c > @@ -1835,6 +1835,7 @@ vdbe_metadata_delete(struct Vdbe *v) > free(v->metadata[i].name); > free(v->metadata[i].type); > free(v->metadata[i].collation); > + free(v->metadata[i].span); > } > free(v->metadata); > } > @@ -1921,6 +1922,24 @@ vdbe_metadata_set_col_autoincrement(struct Vdbe *p, int idx) > p->metadata[idx].is_actoincrement = true; > } > > +int > +vdbe_metadata_set_col_span(struct Vdbe *p, int idx, const char *span) > +{ > + assert(idx < p->nResColumn); > + if (p->metadata[idx].span != NULL) > + free((void *)p->metadata[idx].span); > + if (span == NULL) { > + p->metadata[idx].span = NULL; > + return 0; > + } > + p->metadata[idx].span = strdup(span); > + if (p->metadata[idx].span == NULL) { > + diag_set(OutOfMemory, strlen(span) + 1, "strdup", "span"); > + return -1; > + } > + return 0; > +} > + > /* > * This routine checks that the sql.nVdbeActive count variable > * matches the number of vdbe's in the list sql.pVdbe that are > diff --git a/test/sql/full_metadata.result b/test/sql/full_metadata.result > index 463108001..ca69f4145 100644 > --- a/test/sql/full_metadata.result > +++ b/test/sql/full_metadata.result > @@ -56,6 +56,7 @@ execute("SELECT 'aSd' COLLATE \"unicode_ci\";") > | --- > | - metadata: > | - type: string > + | span: '''aSd'' COLLATE "unicode_ci"' > | name: '''aSd'' COLLATE "unicode_ci"' > | collation: unicode_ci > | rows: > @@ -64,7 +65,8 @@ execute("SELECT 'aSd' COLLATE \"unicode_ci\";") > execute("SELECT c FROM t;") > | --- > | - metadata: > - | - type: string > + | - span: C > + | type: string > | is_nullable: true > | name: C > | collation: unicode_ci > @@ -75,6 +77,7 @@ execute("SELECT c COLLATE \"unicode\" FROM t;") > | --- > | - metadata: > | - type: string > + | span: c COLLATE "unicode" > | name: c COLLATE "unicode" > | collation: unicode > | rows: > @@ -86,14 +89,17 @@ execute("SELECT c COLLATE \"unicode\" FROM t;") > execute("SELECT id, a, c FROM t;") > | --- > | - metadata: > - | - type: integer > + | - span: ID > + | type: integer > | is_autoincrement: true > | name: ID > | is_nullable: false > | - type: integer > + | span: A > | name: A > | is_nullable: false > - | - type: string > + | - span: C > + | type: string > | is_nullable: true > | name: C > | collation: unicode_ci > @@ -103,14 +109,17 @@ execute("SELECT id, a, c FROM t;") > execute("SELECT * FROM t;") > | --- > | - metadata: > - | - type: integer > + | - span: ID > + | type: integer > | is_autoincrement: true > | name: ID > | is_nullable: false > | - type: integer > + | span: A > | name: A > | is_nullable: false > - | - type: string > + | - span: C > + | type: string > | is_nullable: true > | name: C > | collation: unicode_ci > @@ -118,6 +127,83 @@ execute("SELECT * FROM t;") > | - [1, 1, 'aSd'] > | ... > > +-- Span is always set in extended metadata. Span is an original > +-- expression forming result set column. > +-- > +execute("SELECT 1 AS x;") > + | --- > + | - metadata: > + | - type: integer > + | span: '1' > + | name: X > + | rows: > + | - [1] > + | ... > +execute("SELECT *, id + 1 AS x, a AS y, c || 'abc' FROM t;") > + | --- > + | - metadata: > + | - span: ID > + | type: integer > + | is_autoincrement: true > + | name: ID > + | is_nullable: false > + | - type: integer > + | span: A > + | name: A > + | is_nullable: false > + | - span: C > + | type: string > + | is_nullable: true > + | name: C > + | collation: unicode_ci > + | - type: integer > + | span: id + 1 > + | name: X > + | - type: integer > + | span: a > + | name: Y > + | is_nullable: false > + | - type: string > + | span: c || 'abc' > + | name: c || 'abc' > + | rows: > + | - [1, 1, 'aSd', 2, 1, 'aSdabc'] > + | ... > + > +box.execute("CREATE VIEW v AS SELECT id + 1 AS x, a AS y, c || 'abc' FROM t;") > + | --- > + | - row_count: 1 > + | ... > +execute("SELECT * FROM v;") > + | --- > + | - metadata: > + | - type: integer > + | span: X > + | name: X > + | - type: integer > + | span: Y > + | name: Y > + | is_nullable: false > + | - type: string > + | span: c || 'abc' > + | name: c || 'abc' > + | rows: > + | - [2, 1, 'aSdabc'] > + | ... > +execute("SELECT x, y FROM v;") > + | --- > + | - metadata: > + | - type: integer > + | span: x > + | name: X > + | - type: integer > + | span: y > + | name: Y > + | is_nullable: false > + | rows: > + | - [2, 1] > + | ... > + > execute("PRAGMA full_metadata = false;") > | --- > | - row_count: 0 > @@ -139,6 +225,9 @@ test_run:cmd("setopt delimiter ''"); > | - true > | ... > > +box.space.V:drop() > + | --- > + | ... > box.space.T:drop() > | --- > | ... > diff --git a/test/sql/full_metadata.test.lua b/test/sql/full_metadata.test.lua > index e3b30f7e7..576c49ad3 100644 > --- a/test/sql/full_metadata.test.lua > +++ b/test/sql/full_metadata.test.lua > @@ -35,6 +35,16 @@ execute("SELECT c COLLATE \"unicode\" FROM t;") > execute("SELECT id, a, c FROM t;") > execute("SELECT * FROM t;") > > +-- Span is always set in extended metadata. Span is an original > +-- expression forming result set column. > +-- > +execute("SELECT 1 AS x;") > +execute("SELECT *, id + 1 AS x, a AS y, c || 'abc' FROM t;") > + > +box.execute("CREATE VIEW v AS SELECT id + 1 AS x, a AS y, c || 'abc' FROM t;") > +execute("SELECT * FROM v;") > +execute("SELECT x, y FROM v;") > + > execute("PRAGMA full_metadata = false;") > > test_run:cmd("setopt delimiter ';'") > @@ -45,4 +55,5 @@ if remote then > end; > test_run:cmd("setopt delimiter ''"); > > +box.space.V:drop() > box.space.T:drop() >