[Tarantool-patches] [PATCH] sql: extend result set with span
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Dec 29 19:34:58 MSK 2019
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 at 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)
More information about the Tarantool-patches
mailing list