Hello, Sergei! Looks fine for me. So if you both prefer it more, let's use this variant. Regards, Nikita On Wed, Jul 28, 2021 at 12:08 PM Sergei Golubchik <serg@mariadb.org> wrote:
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
-- Nikita Malyavin, Software Engineer MariaDB Corporation