maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx
-- 
Vote for my Percona Live 2016 talks:
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-connectors-fast-and-smart-new-protocol-optimizations#community-voting
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-101-security-validation-authentication-encryption#community-voting