[patches] [PATCH 6/7] iproto: send SQL column meta on SELECT
Konstantin Osipov
kostja at tarantool.org
Thu Mar 29 17:45:07 MSK 2018
* Vladislav Shpilevoy <v.shpilevoy at tarantool.org> [18/02/28 22:41]:
> +/**
> + * Encode meta information about a non-table column represented in
> + * a SELECT column list by a constant, or by a complex expression.
> + * It does not have any meta except an alias.
> + * @param out Output buffer.
> + * @param column Column meta.
> + *
> + * @retval 0 Success.
> + * @retval -1 Memory error.
> + */
> +static inline int
> +sql_ephemeral_column_meta_encode(struct obuf *out,
> + const struct sql_column_meta *column)
Either I don't understand the comment or the purpose of this
function.
Why do yo need to encode metadata for constants separately?
Constants are not different from any other column. You can use
string representation of the constant as its name/alias, if there
is no explicit name, and all other properties are the same as for
a table column - nullability is false for everything but NULL,
character set and collation are known, the data type is very well
known as well.
Seems like the whole purpose of having sql_column_meta was to
avoid treating constants separately, so I'm completely confused.
> +{
> + assert(column->alias != NULL);
> + assert(column->name == NULL);
> + int alias_len = strlen(column->alias);
> + size_t size = mp_sizeof_map(1) + mp_sizeof_uint(IPROTO_FIELD_NAME) +
> + mp_sizeof_str(alias_len);
> + char *pos = (char *) obuf_alloc(out, size);
> + if (pos == NULL) {
> + diag_set(OutOfMemory, size, "obuf_alloc", "pos");
> + return -1;
> + }
> + char *begin = pos;
> + pos = mp_encode_map(pos, 1);
> + pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);
> + pos = mp_encode_str(pos, column->alias, alias_len);
> + assert(pos == begin + size);
> + return 0;
> +}
> +
> +/**
> + * Encode meta information about a table column specified in a
> + * SELECT column list.
> + * @param out Output buffer.
> + * @param column Column meta.
> + *
> + * @retval 0 Success.
> + * @retval -1 Memory error.
> + */
> +static inline int
> +sql_table_column_meta_encode(struct obuf *out,
> + const struct sql_column_meta *column)
> +{
> + assert(column->alias != NULL);
> + assert(column->name != NULL);
> + int alias_len = strlen(column->alias);
> + int name_len = strlen(column->name);
> + bool is_alias_eq_name =
> + alias_len == name_len &&
> + memcmp(column->name, column->alias, name_len) == 0;
> + struct iproto_sql_column_meta_bin header = iproto_sql_column_meta_bin;
> + size_t size = sizeof(header) + mp_sizeof_uint(IPROTO_FIELD_COLUMN) +
> + mp_sizeof_str(name_len);
> + if (! is_alias_eq_name) {
> + size += mp_sizeof_uint(IPROTO_FIELD_NAME) +
> + mp_sizeof_str(alias_len);
> + /* @Sa mp_encode_map(). */
> + header.m_header |= 3;
> + } else {
> + header.m_header |= 2;
> + }
> + header.m_flags = column->flags;
> +
> + char *pos = (char *) obuf_alloc(out, size);
> + if (pos == NULL) {
> + diag_set(OutOfMemory, size, "obuf_alloc", "pos");
> + return -1;
> + }
> + char *begin = pos;
> + memcpy(pos, &header, sizeof(header));
> + pos += sizeof(header);
> + if (! is_alias_eq_name) {
> + pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);
> + pos = mp_encode_str(pos, column->alias, alias_len);
> + }
> + pos = mp_encode_uint(pos, IPROTO_FIELD_COLUMN);
> + pos = mp_encode_str(pos, column->name, name_len);
> + assert(pos == begin + size);
> + return 0;
> +}
> +
> /**
> * Serialize a description of the prepared statement.
> * @param stmt Prepared statement.
> @@ -524,24 +627,12 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
> return -1;
>
> for (int i = 0; i < column_count; ++i) {
> - size_t size = mp_sizeof_map(1) +
> - mp_sizeof_uint(IPROTO_FIELD_NAME);
> - const char *name = sqlite3_column_name(stmt, i);
> - /*
> - * Can not fail, since all column names are
> - * preallocated during prepare phase and the
> - * column_name simply returns them.
> - */
> - assert(name != NULL);
> - size += mp_sizeof_str(strlen(name));
> - char *pos = (char *) obuf_alloc(out, size);
> - if (pos == NULL) {
> - diag_set(OutOfMemory, size, "obuf_alloc", "pos");
> - return -1;
> - }
> - pos = mp_encode_map(pos, 1);
> - pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);
> - pos = mp_encode_str(pos, name, strlen(name));
> + const struct sql_column_meta *column =
> + sqlite3_column_meta(stmt, i);
> + if (column->name != NULL)
> + sql_table_column_meta_encode(out, column);
> + else
> + sql_ephemeral_column_meta_encode(out, column);
> }
> return 0;
> }
> diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
> index c06092386..6cbf701ea 100644
> --- a/src/box/iproto_constants.h
> +++ b/src/box/iproto_constants.h
> @@ -121,8 +121,31 @@ enum iproto_key {
> */
> enum iproto_metadata_key {
> IPROTO_FIELD_NAME = 0,
> + IPROTO_FIELD_COLUMN = 1,
> + IPROTO_FIELD_FLAGS = 2,
Please avoid having flags. enum iproto_metadata_key is already a
flags enum, let's not have flags nested inside other flags.
> + IPROTO_METADATA_KEY_MAX,
> };
>
> +/** Bit members of IPROTO_FIELD_FLAGS. */
> +enum iproto_field_flag {
> + IPROTO_FIELD_IS_NULLABLE = 1,
> + IPROTO_FIELD_IS_PRIMARY_PART = 2,
> + IPROTO_FIELD_IS_AUTOINCREMENT = 4,
> + IPROTO_FIELD_IS_CASE_SENSITIVE = 8,
> + IPROTO_FIELD_FLAG_MAX,
> +};
Please merge the two, it doesn't cost anything but is
way easier to dissect.
> +
> +/**
> + * If the assert fails, then update
> + * struct iproto_sql_column_meta_bin.
> + */
> +static_assert(IPROTO_METADATA_KEY_MAX <= 0x7f,
> + "IPROTO_METADATA fields must fit in one MessagePack byte");
> +static_assert(IPROTO_METADATA_KEY_MAX <= 15,
> + "IPROTO_METADATA MessagePack map header must fit in one byte");
> +static_assert(IPROTO_FIELD_FLAG_MAX - 1 <= 0x7f,
> + "IPROTO_FIELD_FLAGS must fit in one MessagePack byte");
> +
> #define bit(c) (1ULL<<IPROTO_##c)
>
> #define IPROTO_HEAD_BMAP (bit(REQUEST_TYPE) | bit(SYNC) | bit(REPLICA_ID) |\
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 18d1ce083..4fef4cf41 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -39,11 +39,23 @@ local IPROTO_SCHEMA_VERSION_KEY = 0x05
> local IPROTO_METADATA_KEY = 0x32
> local IPROTO_SQL_INFO_KEY = 0x43
> local IPROTO_SQL_ROW_COUNT_KEY = 0x44
> -local IPROTO_FIELD_NAME_KEY = 0
> local IPROTO_DATA_KEY = 0x30
> local IPROTO_ERROR_KEY = 0x31
> local IPROTO_GREETING_SIZE = 128
>
> +local iproto_metadata_keys = {
> + name = 0,
> + column = 1,
> + flags = 2,
> +}
> +
> +local iproto_field_flags = {
> + is_nullable = 1,
> + is_primary_part = 2,
> + is_autoincrement = 4,
> + is_case_sensitive = 8,
> +}
> +
> -- select errors from box.error
> local E_UNKNOWN = box.error.UNKNOWN
> local E_NO_CONNECTION = box.error.NO_CONNECTION
> @@ -850,8 +862,19 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
> end
> -- Set readable names for the metadata fields.
> for i, field_meta in pairs(metadata) do
> - field_meta["name"] = field_meta[IPROTO_FIELD_NAME_KEY]
> - field_meta[IPROTO_FIELD_NAME_KEY] = nil
> + for key, code in pairs(iproto_metadata_keys) do
> + field_meta[key] = field_meta[code]
> + field_meta[code] = nil
> + end
> + if not field_meta.name and field_meta.column then
> + field_meta.name = field_meta.column
> + end
> + if field_meta.flags then
> + for flag, value in pairs(iproto_field_flags) do
> + field_meta[flag] = bit.band(field_meta.flags, value) ~= 0
> + end
> + field_meta.flags = nil
> + end
> end
> setmetatable(res, sequence_mt)
> return {metadata = metadata, rows = res}
> diff --git a/test/sql/iproto.result b/test/sql/iproto.result
> index f7749cb05..ce8d06c06 100644
> --- a/test/sql/iproto.result
> +++ b/test/sql/iproto.result
> @@ -41,7 +41,11 @@ cn:ping()
> -- Simple select.
> cn:execute('select * from test')
> ---
> -- metadata: [{'name': ID}, {'name': 'A'}, {'name': 'B'}]
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'ID', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'A',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'A', 'is_autoincrement': false},
> + {'is_case_sensitive': true, 'column': 'B', 'is_nullable': true, 'is_primary_part': false,
> + 'name': 'B', 'is_autoincrement': false}]
> rows:
> - [1, 2, '3']
> - [4, 5, '6']
> @@ -76,7 +80,8 @@ cn:execute('insert qwerty gjsdjq q qwd qmq;; q;qwd;')
> -- Empty result.
> cn:execute('select id as identifier from test where a = 5;')
> ---
> -- metadata: [{'name': IDENTIFIER}]
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'IDENTIFIER', 'is_autoincrement': false}]
> rows: []
> ...
> -- netbox API errors.
> @@ -98,11 +103,40 @@ cn:execute(' ;')
> - error: 'Failed to execute SQL statement: syntax error: empty request'
> ...
> --
> +-- gh-2620: return column metadata on SELECT.
> +--
> +cn:execute('create table test2 (id integer primary key autoincrement, ci collate "unicode_ci" not null)')
> +---
> +- rowcount: 1
> +...
> +cn:reload_schema()
> +---
> +...
> +cn:execute('select id as identifier, ci from test2')
> +---
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'IDENTIFIER', 'is_autoincrement': true}, {'is_case_sensitive': false,
> + 'column': 'CI', 'is_nullable': false, 'is_primary_part': false, 'name': 'CI',
> + 'is_autoincrement': false}]
> + rows: []
> +...
> +cn:execute('drop table test2')
> +---
> +- rowcount: 1
> +...
> +cn:reload_schema()
> +---
> +...
> +--
> -- Parmaeters bindig.
> --
> cn:execute('select * from test where id = ?', {1})
> ---
> -- metadata: [{'name': ID}, {'name': 'A'}, {'name': 'B'}]
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'ID', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'A',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'A', 'is_autoincrement': false},
> + {'is_case_sensitive': true, 'column': 'B', 'is_nullable': true, 'is_primary_part': false,
> + 'name': 'B', 'is_autoincrement': false}]
> rows:
> - [1, 2, '3']
> ...
> @@ -117,7 +151,11 @@ parameters[1][':value'] = 1
> ...
> cn:execute('select * from test where id = :value', parameters)
> ---
> -- metadata: [{'name': ID}, {'name': 'A'}, {'name': 'B'}]
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'ID', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'A',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'A', 'is_autoincrement': false},
> + {'is_case_sensitive': true, 'column': 'B', 'is_nullable': true, 'is_primary_part': false,
> + 'name': 'B', 'is_autoincrement': false}]
> rows:
> - [1, 2, '3']
> ...
> @@ -299,7 +337,12 @@ cn:execute('insert into test2 values (1, 1, 1, 1)')
> ...
> cn:execute('select * from test2')
> ---
> -- metadata: [{'name': ID}, {'name': 'A'}, {'name': 'B'}, {'name': 'C'}]
> +- metadata: [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'ID', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'A',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'A', 'is_autoincrement': false},
> + {'is_case_sensitive': true, 'column': 'B', 'is_nullable': true, 'is_primary_part': false,
> + 'name': 'B', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'C',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'C', 'is_autoincrement': false}]
is_primary_part -> is_part_of_primary_key
is_autoincrement -> is_sequence, perhaps? We don't use the word
"autoincrement" elsewhere, it's an sqlite thing.
> rows:
> - [1, 1, 1, 1]
> ...
> @@ -469,7 +512,11 @@ res = cn:execute('select * from test')
> ...
> res.metadata
> ---
> -- [{'name': ID}, {'name': 'A'}, {'name': 'B'}]
> +- [{'is_case_sensitive': true, column: 'ID', 'is_nullable': false, 'is_primary_part': true,
> + 'name': 'ID', 'is_autoincrement': false}, {'is_case_sensitive': true, 'column': 'A',
> + 'is_nullable': true, 'is_primary_part': false, 'name': 'A', 'is_autoincrement': false},
> + {'is_case_sensitive': true, 'column': 'B', 'is_nullable': true, 'is_primary_part': false,
> + 'name': 'B', 'is_autoincrement': false}]
> ...
> cn:close()
> ---
> diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
> index 64c0a56fe..d18d00d70 100644
> --- a/test/sql/iproto.test.lua
> +++ b/test/sql/iproto.test.lua
> @@ -38,6 +38,15 @@ cn:execute('select 1', nil, {dry_run = true})
> cn:execute('')
> cn:execute(' ;')
>
> +--
> +-- gh-2620: return column metadata on SELECT.
> +--
> +cn:execute('create table test2 (id integer primary key autoincrement, ci collate "unicode_ci" not null)')
> +cn:reload_schema()
> +cn:execute('select id as identifier, ci from test2')
> +cn:execute('drop table test2')
> +cn:reload_schema()
> +
> --
> -- Parmaeters bindig.
> --
> --
> 2.14.3 (Apple Git-98)
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
More information about the Tarantool-patches
mailing list