Hi, Nikita!

On Jul 27, Nikita Malyavin wrote:
> revision-id: ce289840131 (mariadb-10.3.30-31-gce289840131)
> parent(s): b50ea900636
> author: Nikita Malyavin
> committer: Nikita Malyavin
> timestamp: 2021-07-22 22:38:31 +0300
> message:
>
> MDEV-24511 null field is created with CREATE..SELECT
>
> The regression has been initially appeared by MDEV-12588 patch (441349aa).
>
> The reason CREATE... SELECT NULL does not create null field is that
> it calls handle_select(), where the field is created directly from Item,
> by calling type_handler->type_handler_for_tmp_table(), instead of plain
> type_handler.
>
> In UNION case, we'll already have Item_field's containing UNION'ed tmp
> table fields to the moment of select_create::prepare() call.
>
> There, item's type_handler is used directly in
> st_select_lex_unit::join_union_item_types(), instead of
> type_handler_for_tmp_table() or type_handler_for_union().
>
> Using those two involve too many changes in CREATE...SELECT...UNION
> behavior, which may be reasonably undesired to be pushed in 10.3
>
> However, patching Field_null::type_handler() directly seems to be accurate
> enough.
>
> diff --git a/mysql-test/main/union.result b/mysql-test/main/union.result
> index a892f6c9e40..b19427948a6 100644
> --- a/mysql-test/main/union.result
> +++ b/mysql-test/main/union.result
> @@ -1609,7 +1609,7 @@ NULL    binary(0)       YES             NULL
>  CREATE TABLE t5 SELECT NULL UNION SELECT NULL;
>  DESC t5;
>  Field        Type    Null    Key     Default Extra
> -NULL null    YES             NULL   
> +NULL binary(0)       YES             NULL   
>  CREATE TABLE t6
>  SELECT * FROM (SELECT * FROM (SELECT NULL)a) b UNION SELECT a FROM t1;
>  DESC t6;
> diff --git a/sql/field.h b/sql/field.h
> index a3385736c0a..2af5346bea9 100644
> --- a/sql/field.h
> +++ b/sql/field.h
> @@ -2526,7 +2526,7 @@ class Field_null :public Field_str {
>      :Field_str(ptr_arg, len_arg, null, 1,
>              unireg_check_arg, field_name_arg, collation)
>      {}
> -  const Type_handler *type_handler() const { return &type_handler_null; }
> +  const Type_handler *type_handler() const { return &type_handler_string; }
>    Information_schema_character_attributes
>      information_schema_character_attributes() const
>    {

This looks suspiciously risky to me. The problem was specifically in
UNION, but you change the very basic idea of what type the Field_null
is.

I cannot clearly see what else might this possibly affect, field's type
is used everywhere.

It would be safer and more logical to fix specifically field creation
for UNION.

I've discussed it with Bar, who implemented this whole Type_handler
hierarchy, he suggested to use Type_handler::union_element_finalize()
for that. I've attached the patch that does that.

union_element_finalize() is for 10.4, so I also backported it to 10.3,
it's very straightforward though.

What do you think?

Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org