* [Tarantool-patches] [PATCH] sql: extend result set with span
@ 2019-12-28 22:56 Nikita Pettik
2019-12-29 16:34 ` Vladislav Shpilevoy
2019-12-29 22:32 ` Vladislav Shpilevoy
0 siblings, 2 replies; 5+ messages in thread
From: Nikita Pettik @ 2019-12-28 22:56 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy
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()
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] sql: extend result set with span
2019-12-28 22:56 [Tarantool-patches] [PATCH] sql: extend result set with span Nikita Pettik
@ 2019-12-29 16:34 ` Vladislav Shpilevoy
2019-12-29 20:42 ` Nikita Pettik
2019-12-29 22:32 ` Vladislav Shpilevoy
1 sibling, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-29 16:34 UTC (permalink / raw)
To: Nikita Pettik, tarantool-patches
Hi! Thanks for the patch!
I've pushed my review fixes on top of this commit. See it below
and on the branch. If you agree, then squash. Otherwise lets
discuss. In my commit you can find inlined explanations about some
changes.
> 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
Are you sure this is correct? This is not a pure span. The
original expression was 'c', but here it is 'C'.
================================================================================
commit 057d42f1784f592d10d5d74303ee6cf7c555dcda
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Sun Dec 29 19:30:15 2019 +0300
Review fixes
diff --git a/src/box/execute.c b/src/box/execute.c
index 3034cee86..552504365 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -274,6 +274,8 @@ metadata_map_sizeof(const char *name, const char *type, const char *coll,
{
uint32_t members_count = 2;
size_t map_size = 0;
+ if (! sql_metadata_is_full())
+ goto short_meta;
================================================================================
Seeing at how fat the full meta has become, I decided it is
worth skipping it all at once in case we know for sure, that
it is empty. It will be so in most of the cases.
================================================================================
if (coll != NULL) {
members_count++;
map_size += mp_sizeof_uint(IPROTO_FIELD_COLL);
@@ -289,12 +291,11 @@ 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();
- }
+ members_count++;
+ map_size += mp_sizeof_uint(IPROTO_FIELD_SPAN);
+ map_size += span != NULL ? mp_sizeof_str(strlen(span)) :
+ mp_sizeof_nil();
+short_meta:
map_size += mp_sizeof_uint(IPROTO_FIELD_NAME);
map_size += mp_sizeof_uint(IPROTO_FIELD_TYPE);
map_size += mp_sizeof_str(strlen(name));
@@ -308,15 +309,16 @@ metadata_map_encode(char *buf, const char *name, const char *type,
const char *coll, const char *span, int nullable,
bool is_autoincrement)
{
+ bool is_full = sql_metadata_is_full();
uint32_t map_sz = 2 + (coll != NULL) + (nullable != -1) +
- is_autoincrement;
- if (sql_metadata_is_full())
- map_sz++;
+ is_autoincrement + is_full;
buf = mp_encode_map(buf, map_sz);
buf = mp_encode_uint(buf, IPROTO_FIELD_NAME);
buf = mp_encode_str(buf, name, strlen(name));
buf = mp_encode_uint(buf, IPROTO_FIELD_TYPE);
buf = mp_encode_str(buf, type, strlen(type));
+ if (! is_full)
+ return;
if (coll != NULL) {
buf = mp_encode_uint(buf, IPROTO_FIELD_COLL);
buf = mp_encode_str(buf, coll, strlen(coll));
@@ -329,21 +331,18 @@ 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);
- }
+ /*
+ * 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);
}
/**
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 5ed7b9bd3..d019e69e2 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -26,10 +26,9 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
const char *span = sql_column_span(stmt, i);
int nullable = sql_column_nullable(stmt, i);
bool is_autoincrement = sql_column_is_autoincrement(stmt, i);
+ bool is_full = sql_metadata_is_full();
size_t table_sz = 2 + (coll != NULL) + (nullable != -1) +
- is_autoincrement;
- if (sql_metadata_is_full())
- table_sz++;
+ is_autoincrement + is_full;
lua_createtable(L, 0, table_sz);
/*
* Can not fail, since all column names are
@@ -42,6 +41,8 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
lua_setfield(L, -2, "name");
lua_pushstring(L, type);
lua_setfield(L, -2, "type");
+ if (!is_full)
+ goto set_column;
if (coll != NULL) {
lua_pushstring(L, coll);
lua_setfield(L, -2, "collation");
@@ -54,13 +55,12 @@ 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");
- }
+ if (span != NULL)
+ lua_pushstring(L, span);
+ else
+ lua_pushstring(L, name);
+ lua_setfield(L, -2, "span");
+ set_column:
lua_rawseti(L, -2, i + 1);
}
}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index b4182b88a..ae7e9fd68 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1814,6 +1814,8 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
}
}
vdbe_metadata_set_col_nullability(v, i, -1);
+ const char *colname = pEList->a[i].zName;
+ const char *span = pEList->a[i].zSpan;
if (p->op == TK_COLUMN || p->op == TK_AGG_COLUMN) {
char *zCol;
int iCol = p->iColumn;
@@ -1826,12 +1828,10 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
struct space_def *space_def = space->def;
assert(iCol >= 0 && iCol < (int)space_def->field_count);
zCol = space_def->fields[iCol].name;
- const char *name = NULL;
- if (pEList->a[i].zName != NULL) {
- name = pEList->a[i].zName;
- } else {
+ const char *name = colname;
+ if (name == NULL) {
if (!shortNames && !fullNames) {
- name = pEList->a[i].zSpan;
+ name = span;
} else if (fullNames) {
name = tt_sprintf("%s.%s",
space_def->name,
@@ -1849,26 +1849,20 @@ 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);
- }
+ if (colname != NULL)
+ vdbe_metadata_set_col_span(v, i, span);
}
} else {
const char *z = NULL;
- if (pEList->a[i].zName != NULL)
- z = pEList->a[i].zName;
- else if (pEList->a[i].zSpan != NULL)
- z = pEList->a[i].zSpan;
+ if (colname != NULL)
+ z = colname;
+ else if (span != NULL)
+ z = span;
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 (is_full_meta && colname != NULL)
+ vdbe_metadata_set_col_span(v, i, span);
}
}
if (var_count == 0)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] sql: extend result set with span
2019-12-29 16:34 ` Vladislav Shpilevoy
@ 2019-12-29 20:42 ` Nikita Pettik
0 siblings, 0 replies; 5+ messages in thread
From: Nikita Pettik @ 2019-12-29 20:42 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 29 Dec 19:34, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> I've pushed my review fixes on top of this commit. See it below
> and on the branch. If you agree, then squash. Otherwise lets
> discuss. In my commit you can find inlined explanations about some
> changes.
>
> > 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
>
> Are you sure this is correct? This is not a pure span. The
> original expression was 'c', but here it is 'C'.
It can be fixed with ease:
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index ae7e9fd68..4d69dbc40 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1849,8 +1849,7 @@ 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 (colname != NULL)
- vdbe_metadata_set_col_span(v, i, span);
+ vdbe_metadata_set_col_span(v, i, span);
}
I added this behaviour on purpose. Otherwise, span will be different
from column name and sent as a separate member. Nevetheless, it is
the same column. Probably, we shoudn't return uppercased names in
metadata at all (see https://github.com/tarantool/tarantool/issues/4653).
> ================================================================================
>
> commit 057d42f1784f592d10d5d74303ee6cf7c555dcda
> Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> Date: Sun Dec 29 19:30:15 2019 +0300
>
> Review fixes
>
> diff --git a/src/box/execute.c b/src/box/execute.c
> index 3034cee86..552504365 100644
> --- a/src/box/execute.c
> +++ b/src/box/execute.c
> @@ -274,6 +274,8 @@ metadata_map_sizeof(const char *name, const char *type, const char *coll,
> {
> uint32_t members_count = 2;
> size_t map_size = 0;
> + if (! sql_metadata_is_full())
> + goto short_meta;
> ================================================================================
>
> Seeing at how fat the full meta has become, I decided it is
> worth skipping it all at once in case we know for sure, that
> it is empty. It will be so in most of the cases.
I've apllied your fixes for select.c, but neglect two of them for
box/lua execute.c where you add goto's. This refactoring doesn't
give any performance gain (speaking of skipping code) but adds
unnecessary gotos making code harder to read and understand
(even to me).
That's what I applied:
diff --git a/src/box/execute.c b/src/box/execute.c
index 3034cee86..c7f5ff24a 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -308,10 +308,9 @@ metadata_map_encode(char *buf, const char *name, const char *type,
const char *coll, const char *span, int nullable,
bool is_autoincrement)
{
+ bool is_full = sql_metadata_is_full();
uint32_t map_sz = 2 + (coll != NULL) + (nullable != -1) +
- is_autoincrement;
- if (sql_metadata_is_full())
- map_sz++;
+ is_autoincrement + is_full;
buf = mp_encode_map(buf, map_sz);
buf = mp_encode_uint(buf, IPROTO_FIELD_NAME);
buf = mp_encode_str(buf, name, strlen(name));
@@ -329,21 +328,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);
- }
+ if (! is_full)
+ return;
+ /*
+ * 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);
}
/**
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 5ed7b9bd3..8442f610d 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -26,10 +26,9 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
const char *span = sql_column_span(stmt, i);
int nullable = sql_column_nullable(stmt, i);
bool is_autoincrement = sql_column_is_autoincrement(stmt, i);
+ bool is_full = sql_metadata_is_full();
size_t table_sz = 2 + (coll != NULL) + (nullable != -1) +
- is_autoincrement;
- if (sql_metadata_is_full())
- table_sz++;
+ is_autoincrement + is_full;
lua_createtable(L, 0, table_sz);
> ================================================================================
> if (coll != NULL) {
> members_count++;
> map_size += mp_sizeof_uint(IPROTO_FIELD_COLL);
> @@ -289,12 +291,11 @@ 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();
> - }
> + members_count++;
> + map_size += mp_sizeof_uint(IPROTO_FIELD_SPAN);
> + map_size += span != NULL ? mp_sizeof_str(strlen(span)) :
> + mp_sizeof_nil();
> +short_meta:
> map_size += mp_sizeof_uint(IPROTO_FIELD_NAME);
> map_size += mp_sizeof_uint(IPROTO_FIELD_TYPE);
> map_size += mp_sizeof_str(strlen(name));
> @@ -308,15 +309,16 @@ metadata_map_encode(char *buf, const char *name, const char *type,
> const char *coll, const char *span, int nullable,
> bool is_autoincrement)
> {
> + bool is_full = sql_metadata_is_full();
> uint32_t map_sz = 2 + (coll != NULL) + (nullable != -1) +
> - is_autoincrement;
> - if (sql_metadata_is_full())
> - map_sz++;
> + is_autoincrement + is_full;
> buf = mp_encode_map(buf, map_sz);
> buf = mp_encode_uint(buf, IPROTO_FIELD_NAME);
> buf = mp_encode_str(buf, name, strlen(name));
> buf = mp_encode_uint(buf, IPROTO_FIELD_TYPE);
> buf = mp_encode_str(buf, type, strlen(type));
> + if (! is_full)
> + return;
> if (coll != NULL) {
> buf = mp_encode_uint(buf, IPROTO_FIELD_COLL);
> buf = mp_encode_str(buf, coll, strlen(coll));
> @@ -329,21 +331,18 @@ 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);
> - }
> + /*
> + * 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);
> }
>
> /**
> diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
> index 5ed7b9bd3..d019e69e2 100644
> --- a/src/box/lua/execute.c
> +++ b/src/box/lua/execute.c
> @@ -26,10 +26,9 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
> const char *span = sql_column_span(stmt, i);
> int nullable = sql_column_nullable(stmt, i);
> bool is_autoincrement = sql_column_is_autoincrement(stmt, i);
> + bool is_full = sql_metadata_is_full();
> size_t table_sz = 2 + (coll != NULL) + (nullable != -1) +
> - is_autoincrement;
> - if (sql_metadata_is_full())
> - table_sz++;
> + is_autoincrement + is_full;
> lua_createtable(L, 0, table_sz);
> /*
> * Can not fail, since all column names are
> @@ -42,6 +41,8 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
> lua_setfield(L, -2, "name");
> lua_pushstring(L, type);
> lua_setfield(L, -2, "type");
> + if (!is_full)
> + goto set_column;
> if (coll != NULL) {
> lua_pushstring(L, coll);
> lua_setfield(L, -2, "collation");
> @@ -54,13 +55,12 @@ 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");
> - }
> + if (span != NULL)
> + lua_pushstring(L, span);
> + else
> + lua_pushstring(L, name);
> + lua_setfield(L, -2, "span");
> + set_column:
> lua_rawseti(L, -2, i + 1);
> }
> }
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index b4182b88a..ae7e9fd68 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -1814,6 +1814,8 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
> }
> }
> vdbe_metadata_set_col_nullability(v, i, -1);
> + const char *colname = pEList->a[i].zName;
> + const char *span = pEList->a[i].zSpan;
> if (p->op == TK_COLUMN || p->op == TK_AGG_COLUMN) {
> char *zCol;
> int iCol = p->iColumn;
> @@ -1826,12 +1828,10 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
> struct space_def *space_def = space->def;
> assert(iCol >= 0 && iCol < (int)space_def->field_count);
> zCol = space_def->fields[iCol].name;
> - const char *name = NULL;
> - if (pEList->a[i].zName != NULL) {
> - name = pEList->a[i].zName;
> - } else {
> + const char *name = colname;
> + if (name == NULL) {
> if (!shortNames && !fullNames) {
> - name = pEList->a[i].zSpan;
> + name = span;
> } else if (fullNames) {
> name = tt_sprintf("%s.%s",
> space_def->name,
> @@ -1849,26 +1849,20 @@ 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);
> - }
> + if (colname != NULL)
> + vdbe_metadata_set_col_span(v, i, span);
> }
> } else {
> const char *z = NULL;
> - if (pEList->a[i].zName != NULL)
> - z = pEList->a[i].zName;
> - else if (pEList->a[i].zSpan != NULL)
> - z = pEList->a[i].zSpan;
> + if (colname != NULL)
> + z = colname;
> + else if (span != NULL)
> + z = span;
> 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 (is_full_meta && colname != NULL)
> + vdbe_metadata_set_col_span(v, i, span);
> }
> }
> if (var_count == 0)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] sql: extend result set with span
2019-12-28 22:56 [Tarantool-patches] [PATCH] sql: extend result set with span Nikita Pettik
2019-12-29 16:34 ` Vladislav Shpilevoy
@ 2019-12-29 22:32 ` Vladislav Shpilevoy
2019-12-29 23:59 ` Nikita Pettik
1 sibling, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-29 22:32 UTC (permalink / raw)
To: Nikita Pettik, tarantool-patches, 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()
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH] sql: extend result set with span
2019-12-29 22:32 ` Vladislav Shpilevoy
@ 2019-12-29 23:59 ` Nikita Pettik
0 siblings, 0 replies; 5+ messages in thread
From: Nikita Pettik @ 2019-12-29 23:59 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 30 Dec 01:32, Vladislav Shpilevoy wrote:
> LGTM.
Pushed to master.
> 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()
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-29 23:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-28 22:56 [Tarantool-patches] [PATCH] sql: extend result set with span Nikita Pettik
2019-12-29 16:34 ` Vladislav Shpilevoy
2019-12-29 20:42 ` Nikita Pettik
2019-12-29 22:32 ` Vladislav Shpilevoy
2019-12-29 23:59 ` Nikita Pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox