Hi, Alexander! On Nov 27, Alexander Barkov wrote:
Hi Serg,
Please review a patch for MDEV-8092.
I didn't review a lot of it, sorry. Because my first two comments - if we agree on them - might cause quite a few changes (search/replace kind of changes, but still). So I'd better look at the whole patch after that.
diff --git a/sql/field.h b/sql/field.h index 0591914..a40e307 100644 --- a/sql/field.h +++ b/sql/field.h @@ -537,6 +537,115 @@ inline bool is_temporal_type_with_time(enum_field_types type) }
+/** + @class Name + An arbitrary SQL name, consisting of a NULL-terminated string + and its length. + Later it can be reused for table and schema names. +*/ +class Name +{ + const char *m_str; + uint m_length;
Sorry, we have too many "string" classes already. Either use LEX_STRING or create a Lex_string which is inherited from LEX_STRING, binary compatible with it and only adds C++ methods (and can be casted back and forth automatically). So that one could use Lex_string where a LEX_STRING is expected and vice versa.
@@ -602,7 +711,7 @@ class Virtual_column_info: public Sql_alloc } };
-class Field: public Value_source +class Field: public Value_source, public Column_name
I don't think so, this is logically wrong. A field *has* a name, that's right. But a field *is not* a name. The Field class should include the name, not be inherited from it.
{ Field(const Item &); /* Prevent use of these */ void operator=(Field &);
Regards, Sergei Chief Architect MariaDB and security@mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/maria... https://www.percona.com/live/data-performance-conference-2016/sessions/maria...